Closed Bug 1435781 Opened 4 years ago Closed 4 years ago

fix fetch-event.https.html to not timeout by using a content-type for reflected post response

Categories

(Core :: DOM: Service Workers, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: bkelly, Assigned: bkelly)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

We started timing out fetch-event.https.html in bug 1435337.  I want to fix this so we don't lose the test coverage.

The issue is that the test now reflects a POST request body back to the browser, but without any content-type.  This means the browser has to try to sniff the type of response and firefox ends up opening a download box.

This test is not really about sniffing, but whether the body was sent correctly to the server or not.  Therefore I'm going to fix this by just setting a text/plain content-type on the response.
I cleaned up the WPT .ini expectations quite a bit.  Lots of old stuff in there.  So lets run a try build to make sure I didn't break it on some platform:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f1c78e5d7b212a214b530cc830e3a163780df601
Priority: -- → P2
Comment on attachment 8948451 [details] [diff] [review]
Make fetch-event.https.html POST tests set a content-type when reflecting the upload body back in the response. r=bz

Boris, this WPT test is trying to verify that the POST upload body is sent to the server correctly if the service worker gets a FetchEvent, but does not call FetchEvent.respondWith().  Its doing this by reflecting the body back from the server .py as a response body.  The body ends up as the content of an iframe load.

We were timing out the test because the server .py does not set a content-type on the response.  This forces us to try to sniff the content type.  For whatever reason, this sniffing resulted in FF opening a download dialog instead of loading the content in the iframe.

The simplest fix here is to just set a content-type of text/plain.

While fixing the .ini expectations I also removed an old pref we no longer need.  The test used to contain a check for a case that is not permitted by default browser configurations.  That test case has been removed, so we don't need the pref any more.
Attachment #8948451 - Flags: review?(bzbarsky)
Comment on attachment 8948451 [details] [diff] [review]
Make fetch-event.https.html POST tests set a content-type when reflecting the upload body back in the response. r=bz

r=me

Fwiw, I expect the issue was that we sniffed the URL filename and decided to open it as a Python file.  That comes before the text-or-binary sniff.
Attachment #8948451 - Flags: review?(bzbarsky) → review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/370e267e1605
Make fetch-event.https.html POST tests set a content-type when reflecting the upload body back in the response. r=bz
https://hg.mozilla.org/mozilla-central/rev/370e267e1605
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Created web-platform-tests PR https://github.com/w3c/web-platform-tests/pull/9608 for changes under testing/web-platform/tests
Comment on attachment 8980591 [details]
Bug 1435781 - Part 1: Prevent content sniffing of the response in original echo-content.py,

https://reviewboard.mozilla.org/r/246750/#review252892

Wrong bug number?  This bug was closed a while ago...
Attachment #8980591 - Flags: review-
Attachment #8980591 - Attachment is obsolete: true
Attachment #8980591 - Flags: review?(nika)
You need to log in before you can comment on or make changes to this bug.