Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to fix escaping in CSP tests #25275

Merged
merged 1 commit into from Sep 16, 2020
Merged

Attempt to fix escaping in CSP tests #25275

merged 1 commit into from Sep 16, 2020

Conversation

@Hexcles
Copy link
Member

@Hexcles Hexcles commented Aug 28, 2020

{{GET}} is automatically HTML-escaped (not URL-encoded) when used in
.sub.html. There is some insufficient workaround in this test suite
for things like < >. This PR attempts to properly decode HTML entities.

Note: this is most likely not complete (only covers the case in #25141 ).
There are so many uses of {{GET}} in .sub.html in this suite that might
need similar treatment, so we might want to do this in e.g.
originFrameShouldBe instead.

{{GET}} is automatically HTML-escaped (*not* URL-encoded) when used in
`.sub.html`. There is some insufficient workaround in this test suite
for things like < >. This PR attempts to properly decode HTML entities.
@Hexcles
Copy link
Member Author

@Hexcles Hexcles commented Aug 28, 2020

Reviewers, could you please suggest a better place (e.g. a single entry point) to make this change as you're more familiar with the tests? I made this PoC without fully understanding the structure of this test suite.

@Hexcles
Copy link
Member Author

@Hexcles Hexcles commented Aug 28, 2020

cc @ziransun too

@community-tc-integration

This comment has been minimized.

Copy link

@community-tc-integration community-tc-integration bot commented on ebbe4b9 Aug 28, 2020

Uh oh! Looks like an error! Details

Failed to get your artifact.

This comment has been minimized.

Copy link

@community-tc-integration community-tc-integration bot replied Aug 28, 2020

Uh oh! Looks like an error! Details

Failed to get your artifact.

This comment has been minimized.

Copy link

@community-tc-integration community-tc-integration bot replied Aug 28, 2020

Uh oh! Looks like an error! Details

Failed to get your artifact.

This comment has been minimized.

Copy link

@community-tc-integration community-tc-integration bot replied Aug 28, 2020

Uh oh! Looks like an error! Details

Failed to get your artifact.

@Hexcles
Copy link
Member Author

@Hexcles Hexcles commented Sep 14, 2020

Copy link
Contributor

@hillbrad hillbrad left a comment

LGTM.

@Hexcles
Copy link
Member Author

@Hexcles Hexcles commented Sep 16, 2020

I did a cursory scan of frame-ancestors/support/frame-ancestors-test.sub.js and uses of its API in this suite without finding other obvious issues, so I'm merging this PR as is.

@Hexcles Hexcles merged commit 70f1fc9 into master Sep 16, 2020
20 checks passed
20 checks passed
detect-deployment
Details
Azure Pipelines Build #20200828.35 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
download-firefox-nightly Community-TC (pull_request)
Details
lint Community-TC (pull_request)
Details
sink-task Community-TC (pull_request)
Details
wpt-chrome-dev-results Community-TC (pull_request)
Details
wpt-chrome-dev-results-without-changes Community-TC (pull_request)
Details
wpt-chrome-dev-stability Community-TC (pull_request)
Details
wpt-decision-task Community-TC (pull_request)
Details
wpt-firefox-nightly-results Community-TC (pull_request)
Details
wpt-firefox-nightly-results-without-changes Community-TC (pull_request)
Details
wpt-firefox-nightly-stability Community-TC (pull_request)
Details
wpt.fyi - chrome[experimental] Chrome results
Details
wpt.fyi - firefox[experimental] Firefox results
Details
wpt.fyi - safari[experimental] Safari results
Details
@Hexcles Hexcles deleted the fix-csp branch Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.