Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upNetwork Security : Implement StrictOrigin and StrictOriginWhenCrossOr… #14059
+244
−256
Conversation
…igin Referer policy strict-origin and strict-origin-when-cross-origin changes have been implemented. Relevant unit test cases have been added. Enum for RefererPolicy has been added to hyper codebase and v 0.9.11 of hyper contains these changes. This commit also contains changes related to upgrade of hyper from v0.9.10 to v0.9.11. Other dependencies changed are rayon, utils, num_cpus.
|
|
|
Some nits. |
components/net/http_loader.rs
Outdated
| if referrer_url.scheme() == "https" && url.scheme() != "https" { | ||
| return None; | ||
| } | ||
| return strip_url(referrer_url, true); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
nmvk
Nov 4, 2016
Author
Contributor
Can you please elaborate why return is not required?
Thanks,
Raghav
This comment has been minimized.
This comment has been minimized.
nox
Nov 4, 2016
Member
It's the last expression of the function, and the last expression in a function is the function's return value.
This comment has been minimized.
This comment has been minimized.
components/net/http_loader.rs
Outdated
| if cross_origin { | ||
| return strip_url(referrer_url, true); | ||
| } | ||
| return strip_url(referrer_url, false); |
This comment has been minimized.
This comment has been minimized.
nox
Nov 4, 2016
Member
Nit: return is useless here and doing strip_url(referrer_url, cross_origin) is shorter.
components/net/http_loader.rs
Outdated
|
|
||
| /// https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-strict-origin-when-cross-origin | ||
| fn strict_origin_when_cross_origin(referrer_url: Url, url: Url) -> Option<Url> { | ||
| let cross_origin = referrer_url.origin() != url.origin(); |
This comment has been minimized.
This comment has been minimized.
components/net/http_loader.rs
Outdated
| let cross_origin = referrer_url.origin() != url.origin(); | ||
| if referrer_url.scheme() == "https" && url.scheme() != "https" { | ||
| return None; | ||
| } else { |
This comment has been minimized.
This comment has been minimized.
|
@bors-servo try |
|
|
Incorporated code review comments in components/net/http_loader.rs Resolved merge conflicts in cargo.lock file. Updated ReferrerPolicy in lib.rs
|
|
Since last execution statement is the value which is returned, return keyword has been removed from method strict_origin and strict_origin_when_cross_origin. Merge conflicts in Cargo.lock has been handled
|
@bors-servo: try |
bors-servo
added a commit
that referenced
this pull request
Nov 4, 2016
Network Security : Implement StrictOrigin and StrictOriginWhenCrossOr… This pull request contains commit implementing initial steps for Improving Network Security project. As part of initial steps referer policy enums for strict-origin and strict-origin-when-cross-origin have been added to hyper([hyperium/hyper#943]). Unit tests and additional logic has been added to handle these policies. Since enum changes are available on hyper version 0.9.11. We had to update hyper version to 0.9.11. Hyper 0.9.11 depends on num_cpus 1.1.0. To avoid different version of num_cpus. We have updated rayon version from 0.4.0 to 0.4.3. Cargo.toml of util, style, geckolib, stylo component has been updated to use num_cpus version 1.1.0 instead of 0.2.2. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [ X] `./mach build -d` does not report any errors - [ X] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> …igin Referer policy strict-origin and strict-origin-when-cross-origin changes have been implemented. Relevant unit test cases have been added. Enum for RefererPolicy has been added to hyper codebase and v 0.9.11 of hyper contains these changes. This commit also contains changes related to upgrade of hyper from v0.9.10 to v0.9.11. Other dependencies changed are rayon, utils, num_cpus. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14059) <!-- Reviewable:end -->
|
|
highfive
commented
Nov 4, 2016
|
|
Look at all those newly-passing tests! The timeout is #14067 and can be ignored, but the rest need to have their expected results updated. |
|
Also, it looks like there was a rebase error in cef/Cargo.lock: https://github.com/servo/servo/pull/14059/files#diff-f58e90618d14a4b6a043be78f4a4af21R2882 |
…er policy Fixed rebase issue in ports/cef/Cargo.lock Deleted PASSing testcases which were failing before implementation of referrer policies
gurudarshan266
commented
Nov 5, 2016
|
@jdm All the .ini files corresponding to PASSing testcases have been removed for strict-origin and strict-origin-when-cross-origin referrer policies |
|
@bors-servo r+ |
|
|
bors-servo
added a commit
that referenced
this pull request
Nov 7, 2016
Network Security : Implement StrictOrigin and StrictOriginWhenCrossOr… This pull request contains commit implementing initial steps for Improving Network Security project. As part of initial steps referer policy enums for strict-origin and strict-origin-when-cross-origin have been added to [hyper](hyperium/hyper#943). Unit tests and additional logic has been added to handle these policies. Since enum changes are available on hyper version 0.9.11. We had to update hyper version to 0.9.11. Hyper 0.9.11 depends on num_cpus 1.1.0. To avoid different version of num_cpus. We have updated rayon version from 0.4.0 to 0.4.3. Cargo.toml of util, style, geckolib, stylo component has been updated to use num_cpus version 1.1.0 instead of 0.2.2. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [ X] `./mach build -d` does not report any errors - [ X] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> …igin Referer policy strict-origin and strict-origin-when-cross-origin changes have been implemented. Relevant unit test cases have been added. Enum for RefererPolicy has been added to hyper codebase and v 0.9.11 of hyper contains these changes. This commit also contains changes related to upgrade of hyper from v0.9.10 to v0.9.11. Other dependencies changed are rayon, utils, num_cpus. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14059) <!-- Reviewable:end -->
|
|
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.
nmvk commentedNov 4, 2016
•
edited by nox
This pull request contains commit implementing initial steps for Improving Network Security project. As part of initial steps referer policy enums for strict-origin and strict-origin-when-cross-origin have been added to hyper. Unit tests and additional logic has been added to handle these policies. Since enum changes are available on hyper version 0.9.11. We had to update hyper version to 0.9.11.
Hyper 0.9.11 depends on num_cpus 1.1.0. To avoid different version of num_cpus. We have updated rayon version from 0.4.0 to 0.4.3. Cargo.toml of util, style, geckolib, stylo component has been updated to use num_cpus version 1.1.0 instead of 0.2.2.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errors…igin
Referer policy strict-origin and strict-origin-when-cross-origin changes have been implemented. Relevant unit test cases have been added. Enum for RefererPolicy has been added to hyper codebase and v 0.9.11 of hyper contains these changes.
This commit also contains changes related to upgrade of hyper from v0.9.10 to v0.9.11. Other dependencies changed are rayon, utils, num_cpus.
This change is