Skip to content

Update dangling markup mitigations. #6046

Closed
chromium-wpt-export-bot wants to merge 2 commits intomasterfrom
chromium-export-9e5ae90166
Closed

Update dangling markup mitigations. #6046
chromium-wpt-export-bot wants to merge 2 commits intomasterfrom
chromium-export-9e5ae90166

Conversation

@chromium-wpt-export-bot
Copy link
Copy Markdown
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented May 24, 2017 •
edited by wpt-pr-bot
Loading

Still behind a flag, just updating the checks to look for both \n and
< rather than just the former. This is in line with the patches up at
whatwg/url#284 and
whatwg/fetch#519.

Intent to Remove: https://groups.google.com/a/chromium.org/d/msg/blink-dev/KaA_YNOlTPk/VmmoV88xBgAJ.

Bug: 680970
Change-Id: Ifda61a0afe1f0e97620acef7dc54b005c6f74840
Reviewed-on: https://chromium-review.googlesource.com/514024
Commit-Queue: Mike West mkwst@chromium.org
Reviewed-by: Jochen Eisinger jochen@chromium.org
Cr-Commit-Position: refs/heads/master@{#474341}

@mikewest
Update dangling markup mitigations.
2d09e36 
Still behind a flag, just updating the checks to look for both `\n` and
`<` rather than just the former. This is in line with the patches up at
whatwg/url#284 and
whatwg/fetch#519.

Intent to Remove: https://groups.google.com/a/chromium.org/d/msg/blink-dev/KaA_YNOlTPk/VmmoV88xBgAJ.

Bug: 680970
Change-Id: Ifda61a0afe1f0e97620acef7dc54b005c6f74840
Reviewed-on: https://chromium-review.googlesource.com/514024
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#474341}
wpt-pr-bot
wpt-pr-bot approved these changes May 24, 2017
View reviewed changes
Copy link
Copy Markdown
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already reviewed downstream.

@ghost
Copy link
Copy Markdown

ghost commented May 24, 2017 •
edited by ghost
Loading

View the complete job log.

Firefox (nightly)

Testing web-platform-tests at revision abff125
Using browser at version BuildID 20170530100155; SourceStamp 286f71223256cbb3a769432fd860f563c4886e81
Starting 10 test iterations
All results were stable

All results

1 test ran
/fetch/dangling-markup-mitigation.tentative.html
Subtest Results Messages
OK
Fetch: /images/green-1x1.png PASS
Fetch: /images/gre\nen-1x1.png PASS
Fetch: /images/gre\ten-1x1.png PASS
Fetch: /images/gre\ren-1x1.png PASS
Fetch: /images/green-1x1.png?img=< PASS
Fetch: /images/green-1x1.png?img=&lt; PASS
Fetch: /images/green-1x1.png?img=%3C PASS
Fetch: /images/gr\neen-1x1.png?img=%3C PASS
Fetch: /images/gr\reen-1x1.png?img=%3C PASS
Fetch: /images/gr\teen-1x1.png?img=%3C PASS
Fetch: /images/green-1x1.png?img=&#10; PASS
Fetch: /images/gr\neen-1x1.png?img=&#10; PASS
Fetch: /images/gr\reen-1x1.png?img=&#10; PASS
Fetch: /images/gr\teen-1x1.png?img=&#10; PASS
Fetch: /images/gre\nen-1x1.png?img=< FAIL assert_unreached: Fetch should fail. Reached unreachable code
Fetch: /images/gre\ren-1x1.png?img=< FAIL assert_unreached: Fetch should fail. Reached unreachable code
Fetch: /images/gre\ten-1x1.png?img=< FAIL assert_unreached: Fetch should fail. Reached unreachable code
Fetch: /images/green-1x1.png?<\n=block FAIL assert_unreached: Fetch should fail. Reached unreachable code
Fetch: /images/green-1x1.png?<\r=block FAIL assert_unreached: Fetch should fail. Reached unreachable code
Fetch: /images/green-1x1.png?<\t=block FAIL assert_unreached: Fetch should fail. Reached unreachable code
<img id="dangling" src="/images/green-1x1.png?img=&lt;b"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&#10;b"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&amp;#10;b"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&amp;lt;b"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&amp;#10;b&amp;lt;c"> PASS
\n <img id="dangling" src="\n /images/green-1x1.png?img=\n ">\n PASS
\n <img id="dangling" src="\n /images/green-1x1.png?img=&amp;lt;\n ">\n PASS
\n <img id="dangling" src="\n /images/green-1x1.png?img=&amp;#10;\n ">\n PASS
<img id="dangling" src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="> PASS
<img id="dangling" src="data:image/png;base64,&#10;iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="> PASS
<img id="dangling" src="data:image/png;base64,i&#10;VBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="> PASS
<img id="dangling" src="data:image/svg+xml;utf8,\n <svg width='1' height='1' xmlns='http://www.w3.org/2000/svg'>\n <rect width='100%' height='100%' fill='rebeccapurple'/>\n <rect x='10%' y='10%' width='80%' height='80%' fill='lightgreen'/>\n </svg>"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&#10;&lt;b"> FAIL assert_equals: Height expected 0 but got 1
<img id="dangling" src="/images/green-1x1.png?img=&lt;&#10;b"> FAIL assert_equals: Height expected 0 but got 1
\n <img id="dangling" src="/images/green-1x1.png?img=\n &lt;\n &#10;b\n ">\n FAIL assert_equals: Height expected 0 but got 1

@ghost
Copy link
Copy Markdown

ghost commented May 24, 2017 •
edited by ghost
Loading

View the complete job log.

Sauce (safari)

Testing web-platform-tests at revision abff125
Using browser at version 10.0
Starting 10 test iterations

Unstable results

Test Subtest Results Messages
/fetch/dangling-markup-mitigation.tentative.html <img id="dangling" src="/images/green-1x1.png?img=&#10;&lt;b"> FAIL: 1/10, PASS: 9/10 assert_equals: Height expected 0 but got 1

All results

1 test ran
/fetch/dangling-markup-mitigation.tentative.html
Subtest Results Messages
OK
Fetch: /images/green-1x1.png FAIL Can't find variable: fetch
Fetch: /images/gre\nen-1x1.png FAIL Can't find variable: fetch
Fetch: /images/gre\ten-1x1.png FAIL Can't find variable: fetch
Fetch: /images/gre\ren-1x1.png FAIL Can't find variable: fetch
Fetch: /images/green-1x1.png?img=< FAIL Can't find variable: fetch
Fetch: /images/green-1x1.png?img=&lt; FAIL Can't find variable: fetch
Fetch: /images/green-1x1.png?img=%3C FAIL Can't find variable: fetch
Fetch: /images/gr\neen-1x1.png?img=%3C FAIL Can't find variable: fetch
Fetch: /images/gr\reen-1x1.png?img=%3C FAIL Can't find variable: fetch
Fetch: /images/gr\teen-1x1.png?img=%3C FAIL Can't find variable: fetch
Fetch: /images/green-1x1.png?img=&#10; FAIL Can't find variable: fetch
Fetch: /images/gr\neen-1x1.png?img=&#10; FAIL Can't find variable: fetch
Fetch: /images/gr\reen-1x1.png?img=&#10; FAIL Can't find variable: fetch
Fetch: /images/gr\teen-1x1.png?img=&#10; FAIL Can't find variable: fetch
Fetch: /images/gre\nen-1x1.png?img=< FAIL Can't find variable: fetch
Fetch: /images/gre\ren-1x1.png?img=< FAIL Can't find variable: fetch
Fetch: /images/gre\ten-1x1.png?img=< FAIL Can't find variable: fetch
Fetch: /images/green-1x1.png?<\n=block FAIL Can't find variable: fetch
Fetch: /images/green-1x1.png?<\r=block FAIL Can't find variable: fetch
Fetch: /images/green-1x1.png?<\t=block FAIL Can't find variable: fetch
<img id="dangling" src="/images/green-1x1.png?img=&lt;b"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&#10;b"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&amp;#10;b"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&amp;lt;b"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&amp;#10;b&amp;lt;c"> FAIL assert_equals: Height expected 1 but got 0
\n <img id="dangling" src="\n /images/green-1x1.png?img=\n ">\n PASS
\n <img id="dangling" src="\n /images/green-1x1.png?img=&amp;lt;\n ">\n PASS
\n <img id="dangling" src="\n /images/green-1x1.png?img=&amp;#10;\n ">\n FAIL assert_equals: Height expected 1 but got 0
<img id="dangling" src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="> PASS
<img id="dangling" src="data:image/png;base64,&#10;iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="> FAIL assert_equals: Height expected 1 but got 0
<img id="dangling" src="data:image/png;base64,i&#10;VBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="> FAIL assert_equals: Height expected 1 but got 0
<img id="dangling" src="data:image/svg+xml;utf8,\n <svg width='1' height='1' xmlns='http://www.w3.org/2000/svg'>\n <rect width='100%' height='100%' fill='rebeccapurple'/>\n <rect x='10%' y='10%' width='80%' height='80%' fill='lightgreen'/>\n </svg>"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&#10;&lt;b"> FAIL: 1/10, PASS: 9/10 assert_equals: Height expected 0 but got 1
<img id="dangling" src="/images/green-1x1.png?img=&lt;&#10;b"> PASS
\n <img id="dangling" src="/images/green-1x1.png?img=\n &lt;\n &#10;b\n ">\n FAIL assert_equals: Height expected 0 but got 1

@ghost
Copy link
Copy Markdown

ghost commented May 24, 2017 •
edited by ghost
Loading

View the complete job log.

Chrome (unstable)

Testing web-platform-tests at revision abff125
Using browser at version 60.0.3107.4 dev
Starting 10 test iterations
All results were stable

All results

1 test ran
/fetch/dangling-markup-mitigation.tentative.html
Subtest Results Messages
OK
Fetch: /images/green-1x1.png PASS
Fetch: /images/gre\nen-1x1.png PASS
Fetch: /images/gre\ten-1x1.png PASS
Fetch: /images/gre\ren-1x1.png PASS
Fetch: /images/green-1x1.png?img=< PASS
Fetch: /images/green-1x1.png?img=&lt; PASS
Fetch: /images/green-1x1.png?img=%3C PASS
Fetch: /images/gr\neen-1x1.png?img=%3C PASS
Fetch: /images/gr\reen-1x1.png?img=%3C PASS
Fetch: /images/gr\teen-1x1.png?img=%3C PASS
Fetch: /images/green-1x1.png?img=&#10; PASS
Fetch: /images/gr\neen-1x1.png?img=&#10; PASS
Fetch: /images/gr\reen-1x1.png?img=&#10; PASS
Fetch: /images/gr\teen-1x1.png?img=&#10; PASS
Fetch: /images/gre\nen-1x1.png?img=< FAIL assert_unreached: Fetch should fail. Reached unreachable code
Fetch: /images/gre\ren-1x1.png?img=< FAIL assert_unreached: Fetch should fail. Reached unreachable code
Fetch: /images/gre\ten-1x1.png?img=< FAIL assert_unreached: Fetch should fail. Reached unreachable code
Fetch: /images/green-1x1.png?<\n=block FAIL assert_unreached: Fetch should fail. Reached unreachable code
Fetch: /images/green-1x1.png?<\r=block FAIL assert_unreached: Fetch should fail. Reached unreachable code
Fetch: /images/green-1x1.png?<\t=block FAIL assert_unreached: Fetch should fail. Reached unreachable code
<img id="dangling" src="/images/green-1x1.png?img=&lt;b"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&#10;b"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&amp;#10;b"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&amp;lt;b"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&amp;#10;b&amp;lt;c"> PASS
\n <img id="dangling" src="\n /images/green-1x1.png?img=\n ">\n PASS
\n <img id="dangling" src="\n /images/green-1x1.png?img=&amp;lt;\n ">\n PASS
\n <img id="dangling" src="\n /images/green-1x1.png?img=&amp;#10;\n ">\n PASS
<img id="dangling" src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="> PASS
<img id="dangling" src="data:image/png;base64,&#10;iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="> PASS
<img id="dangling" src="data:image/png;base64,i&#10;VBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="> PASS
<img id="dangling" src="data:image/svg+xml;utf8,\n <svg width='1' height='1' xmlns='http://www.w3.org/2000/svg'>\n <rect width='100%' height='100%' fill='rebeccapurple'/>\n <rect x='10%' y='10%' width='80%' height='80%' fill='lightgreen'/>\n </svg>"> PASS
<img id="dangling" src="/images/green-1x1.png?img=&#10;&lt;b"> FAIL assert_equals: Height expected 0 but got 1
<img id="dangling" src="/images/green-1x1.png?img=&lt;&#10;b"> FAIL assert_equals: Height expected 0 but got 1
\n <img id="dangling" src="/images/green-1x1.png?img=\n &lt;\n &#10;b\n ">\n �[32;1mnothing changed, not updating cache�[0m

@ghost
Copy link
Copy Markdown

ghost commented May 24, 2017 •
edited by ghost
Loading

View the complete job log.

Sauce (MicrosoftEdge)

Testing web-platform-tests at revision abff125

foolip
foolip reviewed May 24, 2017
View reviewed changes
Comment thread fetch/dangling-markup-mitigation.tentative.html Outdated
// Leading and trailing whitespace is stripped:
`
<img id="dangling" src="
/images/green-1x1.png?img=
Copy link
Copy Markdown
Member

@foolip foolip May 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikewest, the lint is unhappy about trailing whitespace here and in one other place, but I'm not certain if it's intentional.

Copy link
Copy Markdown
Member

@mikewest mikewest May 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as it's not unhappy about trailing \n or \r, removing it should be fine.

@mikewest
Copy link
Copy Markdown
Member

mikewest commented May 26, 2017

@foolip: Hopefully fixed in 67ed572

@jeffcarp
Copy link
Copy Markdown
Contributor

jeffcarp commented May 26, 2017

Looks like now the results are unstable on Safari?
https://travis-ci.org/w3c/web-platform-tests/jobs/236363153

Will close and reopen to verify if it's consistent.

@jeffcarp jeffcarp closed this May 26, 2017
@jeffcarp jeffcarp reopened this May 26, 2017
@jeffcarp jeffcarp force-pushed the chromium-export-9e5ae90166 branch from 67ed572 to 9b4a61a Compare May 26, 2017 19:44
@mikewest mikewest closed this May 30, 2017
@mikewest mikewest reopened this May 30, 2017
@mikewest
Copy link
Copy Markdown
Member

mikewest commented May 30, 2017

Actually, I'm just going to close this and try again, since we apparently synced down a deletion to Chromium's repository (which seems like surprising behavior :( ).

@mikewest mikewest closed this May 30, 2017
@mikewest mikewest deleted the chromium-export-9e5ae90166 branch May 30, 2017 12:49
@jeffcarp
Copy link
Copy Markdown
Contributor

jeffcarp commented May 30, 2017

@mikewest what was the deletion you're referring to? I can look into it

@mikewest
Copy link
Copy Markdown
Member

mikewest commented May 30, 2017

@jeffcarp: fetch/dangling-markup-mitigation.tentative.html was added in https://chromium-review.googlesource.com/514024, which generated this PR. The next day, the autoroller removed the test from the Chromium repository in https://chromium-review.googlesource.com/c/515422/ (I guess because this PR hasn't landed because of flake under Safari). +@foolip, who I talked to briefly about this earlier.

I'm trying to re-land it along with other changes in https://chromium-review.googlesource.com/c/518156/ (#6085). Let's see how that goes.

@foolip
Copy link
Copy Markdown
Member

foolip commented May 30, 2017

It looks like https://chromium-review.googlesource.com/c/515422/ happened before this CL was ever closed, so possible it has to do with the logic the importer has to avoid doing an importer if there are exportable commits not yet exported. I filed https://crbug.com/727923 and assigned to @qyearsley based on my guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mikewest mikewest mikewest left review comments

@foolip foolip foolip left review comments

@wpt-pr-bot wpt-pr-bot wpt-pr-bot approved these changes

Assignees

No one assigned

Labels

chromium-export fetch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@chromium-wpt-export-bot @mikewest @jeffcarp @foolip @wpt-pr-bot