Exclude referrer patch code when url is chrome-extension #421
Conversation
This file contains 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
bridiver
reviewed
Sep 7, 2018
| + common_params_.url = last_committed_entry->GetURL(); | ||
| + // With chrome-extension by using Pocket on a page with a youtube iframe, | ||
| + // it can lead to insecure referrer mismatch error. | ||
| + if (!common_params_.url.SchemeIs("chrome-extension")) { |
There was a problem hiding this comment.
maybe we should be checking secure/insecure schemes for all redirects? Might be other similar issues somewhere? Isn't there a referrer sanitizing method that checks that?
There was a problem hiding this comment.
I wasn't sure if common_params_.url is always set or not. Otherwise I considered using GURL::SchemeIsCryptographic.
There was a problem hiding this comment.
I don't think there's a clear way directly in this patch to do it though, because in this case the url we'd overwrite to is secure, but for some reason it still fails with the extension isn't insecure.
There was a problem hiding this comment.
So I think this way is the least risky.
bbondy
force-pushed
the
pocket
branch
2 times, most recently
from
cedd4f2 to
06901e4
Compare
This is moved to the common network redirect so the test was meant to be removed
|
++'ed in slack by bridiver |
bbondy
added a commit
that referenced
this pull request
Sep 8, 2018
Exclude referrer patch code when url is chrome-extension
|
0.55.x ccda442 |
fmarier
pushed a commit
that referenced
this pull request
Oct 29, 2019
Touch overridden objective-c++ file(.mm) also
petemill
pushed a commit
that referenced
this pull request
Jul 27, 2020
Touch overridden objective-c++ file(.mm) also
petemill
pushed a commit
that referenced
this pull request
Jul 28, 2020
Touch overridden objective-c++ file(.mm) also
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.
Fix brave/brave-browser#991
This just only runs code we patched in in one less case than it used to before. Risk should be small.
We do have referrer tests from before and they still pass.
Submitter Checklist:
git rebase -ito squash commits (if needed).Test Plan:
Reviewer Checklist: