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.

Sign up for GitHub

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

XMLHttpRequest: response header value containing 0x00 #10424

Merged
merged 2 commits into from
Aug 28, 2018
Merged

XMLHttpRequest: response header value containing 0x00 #10424

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
74c1042
XMLHttpRequest: response header value containing 0x00
annevk Apr 11, 2018
e5088a4
address nit
annevk Aug 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions xhr/headers-normalize-response.htm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
<title>Whitespace and null in header values</title>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script src=support.js?pipe=sub></script>

<h1>Whitespace and null in response header values</h1>

<div id=log></div>

<script>
function error(val) {
test(() => {
const client = new XMLHttpRequest();
client.open("GET", "resources/parse-headers.py?my-custom-header="+encodeURIComponent(val), false);
assert_throws("NetworkError", () => client.send());
}, "Header value: " + val.replace("\0", "\\0"));
}

function matchHeaderValue(val) {
test(function () {
Expand All @@ -20,12 +22,12 @@ <h1>Whitespace and null in response header values</h1>
var r = client.getResponseHeader("My-Custom-Header");

assert_equals(r, trimmed);
}, "Header value: " + val.replace(/\t/g, "[tab]").replace(/ /g, "_").replace("\0", "\\0"));
}, "Header value: " + val.replace(/\t/g, "[tab]").replace(/ /g, "_"));
}

matchHeaderValue("hello world\0");
matchHeaderValue("\0hello world");
matchHeaderValue("hello\0world");
error("hello world\0");
error("\0hello world");
error("hello\0world");
matchHeaderValue(" hello world");
matchHeaderValue("hello world ");
matchHeaderValue(" hello world ");
Expand All @@ -34,9 +36,12 @@ <h1>Whitespace and null in response header values</h1>
matchHeaderValue("\thello world\t");
matchHeaderValue("hello world");
matchHeaderValue("hello\tworld");
matchHeaderValue("\0");
error("\0");
matchHeaderValue(" ");
matchHeaderValue("\t");
matchHeaderValue("");

promise_test(t => {
return promise_rejects(t, new TypeError(), fetch("resources/parse-headers.py?my-custom-header="+encodeURIComponent("x\0x")));
}, "Ensure fetch() rejects too")
Copy link
Member

Choose a reason for hiding this comment

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

This seems pretty out of place here; another separate file would be much better, in case people are testing fetch() and XHR in isolation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why would they do that?

Copy link
Member

Choose a reason for hiding this comment

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

Because they're working on their fetch() or XHR implementation?

Copy link
Member Author

Choose a reason for hiding this comment

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

You need to support both and they both need to fail in the same way due to both operating on the same low-level primitive.

Copy link
Member Author

Choose a reason for hiding this comment

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

(And it's not like both are tested in the same test so it'd be easy to ignore some results.)

Copy link
Member Author

Choose a reason for hiding this comment

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

(I actually think we might want to merge these directories at some point due to the worry of missing test coverage in either API.)

Copy link
Member

Choose a reason for hiding this comment

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

I'm still not comfortable testing fetch in the XHR directory, sorry. It makes maintainers lives harder when they don't know where a given API's tests are.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's already the case though for many cross-purpose tests. I guess we could move this into the http/ directory...

</script>