HTML: forbid data: and javascript: URLs in the <base> element #41731
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
wpt-pr-bot
requested review from
domenic,
foolip,
GPHemsley,
jdm,
jgraham,
mikewest,
TimothyGu and
zqzhang
|
This ends up excluding a single (sub)test from Interop 2023 when merged that passed in Gecko and failed in Chromium and WebKit. Given the agreement in the issue referenced above that seems like the right thing to do. |
domenic
reviewed
Aug 31, 2023
html/semantics/document-metadata/the-base-element/base-data.html
Outdated
Show resolved
Hide resolved
annevk
added a commit
to whatwg/html
that referenced
this pull request
Aug 31, 2023
Also correct a minor mistake in the invocation of "Is base allowed for Document?". Tests: web-platform-tests/wpt#41731. Fixes #2249.
4 tasks
zcorpan
approved these changes
Aug 31, 2023
annevk
added a commit
to whatwg/html
that referenced
this pull request
Aug 31, 2023
Also correct a minor mistake in the invocation of "Is base allowed for Document?". Tests: web-platform-tests/wpt#41731. Fixes #2249.
webkit-commit-queue
pushed a commit
to annevk/WebKit
that referenced
this pull request
Aug 31, 2023
https://bugs.webkit.org/show_bug.cgi?id=260959 rdar://114756660 Reviewed by Chris Dumez. Document::processBaseElement did not call Document::updateBaseURL when there was something wrong with the new base URL. However, that meant that a newly inserted "blocked" <base> would not impact the document base URL and instead whatever was the prior document base URL would continue to be used. This goes against the HTML standard which requires the first <base> element to be used at all times and if that is "blocked" the fallback base URL would have to be used (typically the document's URL). Tests are upstreamed via web-platform-tests/wpt#41731. Chromium matches our failure, but given that it's an edge case I don't foresee any issues. While we're in the general area, stop invoking Document::processBaseElement for <base> elements not connected to a document as that results in a no-op tree traversal. * LayoutTests/fetch/fetch-url-serialization-expected.txt: * LayoutTests/fetch/fetch-urls.json: * LayoutTests/imported/w3c/web-platform-tests/html/semantics/document-metadata/the-base-element/base-data-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/document-metadata/the-base-element/base-data.html: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/document-metadata/the-base-element/base-javascript-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/document-metadata/the-base-element/base-javascript.html: Added. * LayoutTests/imported/w3c/web-platform-tests/html/semantics/document-metadata/the-base-element/w3c-import.log: * LayoutTests/imported/w3c/web-platform-tests/url/a-element-origin-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/url/a-element-origin-xhtml-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/url/a-element-xhtml_exclude=(file_javascript_mailto)-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/url/a-element_exclude=(file_javascript_mailto)-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/url/resources/a-element-origin.js: (runURLTests): * LayoutTests/imported/w3c/web-platform-tests/url/resources/a-element.js: * Source/WebCore/dom/Document.cpp: (WebCore::Document::processBaseElement): * Source/WebCore/html/HTMLBaseElement.cpp: (WebCore::HTMLBaseElement::attributeChanged): Canonical link: https://commits.webkit.org/267498@main
josepharhar
reviewed
Aug 31, 2023
| @@ -0,0 +1,32 @@ | |||
| <!-- Please update base-javascript.html together with this --> | |||
There was a problem hiding this comment.
Does having a comment instead of doctype on the first line put this into quirks mode?
There was a problem hiding this comment.
No it doesn't. In IE6 it did.
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.
I created these for whatwg/html#2249.
The third test fails in Chromium and WebKit as the insertion of a new
<base>element with adata:orjavascript:URL does not result in an update to the document base URL. That seems like a bug to me as it makes the dynamic case different from the static case.