Repository navigation
Check the callback URL as the specification's six rules say - #77
Draft
jwrosewell wants to merge 5 commits into
Draft
jwrosewell wants to merge 5 commits into
jwrosewell wants to merge 5 commits into
Conversation
Adds the refused, accepted and logged cases from the JavaScript builder specification's callback URL section, and removes from the accepted hosts every host the section now refuses. The new cases fail until the check follows the rules.
The host value is checked as written. A host holding '?', '#' or '@' is refused, as is an IPv6 zone identifier and a name whose last label is a number but which is not an IPv4 address in RFC 3986 form. The URL kept for the log shows *** in place of everything before the host value's last '@'.
Adds example.com:, example.com:/path, [::1]:, https://example.com and http://example.com to the refused hosts, and the log case for the last protocol-in-host mistake, and removes example.com: from the accepted hosts.
A host such as https://example.com otherwise reads as the host https with an empty port, and gives https://https://example.com/... with nothing logged. RFC 3986 section 3.2.3 allows an empty port in the syntax, and section 6.2.3 notes that a URI with an empty port is equivalent to one with none, so the host is written without the colon.
A trailing line break after a host, a port or a bracketed address is refused, and 1:2:3:4:5:6:7:8 and ::1.2.3.4 are accepted whilst 1::2::3 and 12345::1 are refused.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The JavaScript builder's callback URL check does not follow the merged callback URL rules (specifications#74). This brings
pipeline/pipeline.javascriptbuilder/src/main/java/fiftyone/pipeline/javascriptbuilder/flowelements/CallbackUrl.javainto line with the page, tests first. The page is the authority.The rules and what changed
httporhttps, ASCII case only. Already held.?or#. Before,example.com?xandexample.com#xwere accepted and written into the script.@anywhere. Before, user information such asuser:pw@example.com,u%41@example.com,@example.comand:@example.comwas accepted and encoded.example.com/a@banduser:12/ss@example.com(read as hostuser, port 12) were accepted too.%inside the brackets is refused, so[fe80::1%25eth0],[fe80::1%25eth0]:8080and the other zone identifiers were accepted before. A name whose last label is digits, or0xand hex digits, must be an IPv4 address in RFC 3986 form. Before,example.123,example.0x1f,example.0X,1.2.3.999,1.2.3,127.1,0x7f.0.0.1,01.2.3.4,example.123.,1.2.3.4.,1.2.3.4.5,1,300.1.1.1and0x7f.1were accepted.:then digits up to 65535. A port that is present but empty is refused. RFC 3986 section 3.2.3 allows an empty port, so the rule refuses one deliberately (see also section 6.2.3), as specifications#112, merged, adds to rule 5. Soexample.com:,example.com:/pathand[::1]:fail, and so dohttps://example.comandhttp://example.com. Before,https://example.comread as the hosthttpswith an empty port and gavehttps://https://example.com/51dpipeline/jsonwith nothing logged.***in place of everything in the host value before its last@, souser:secret@example.com,a@b@example.com,u@[::1]:80x,user:pa/ss@example.comanduser:pa?ss@example.comnever write the password to the log. Before, the whole built URL was logged.Rules 7 and 8 are unchanged apart from the user information, which no longer exists. The encoding case is now
example.com/"^{|}with endpoint/j"^{|}.Tests
CallbackUrlTests.javaholds every refuse, accept and log case from the page and from the shared case list, with the newly refused hosts removed from the accepted list.JavaScriptUrlTests.javarenders the script through the element and checks the logged text, each case with its own element because a refusal is logged once per instance. Also added: configured protocols of white space only are refused.CallbackUrlTests, 7 inJavaScriptUrlTests). The failures are refused hosts accepted, and the logged URL carrying the password. An earlier run, 42579134449, failed only because the temporary workflow did not quote a Maven option.Empty port, added after the first runs: red run 42584863653 (new cases only fail), green run 42584954823 passed.
Full build
Run 42620856814 at head b20b4e2, where
Setting 'PullRequestSha'equals the head. All Windows and Ubuntu jobs and MacOS_Java_11 passed. The remaining macOS jobs were still queued or running for runners when this was written.The documentation links in the code comments point at the specifications
mainbranch, as the other repositories' do, and the callback URL section reachesmainwith the next specifications release.Release note
The callback URL check now follows RFC 3986 and the JavaScript builder specification.
Outstanding
Nothing known. No version change and no
+semverdirective.