Closed Bug 1176988 Opened 9 years ago Closed 9 years ago

POST XHR does not get intercepted by ServiceWorker.

Categories

(Core :: DOM: Service Workers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox41 --- affected
firefox43 --- fixed

People

(Reporter: nsm, Assigned: nsm)

References

Details

Attachments

(4 files)

index.html:

<html>
  <head>
    <meta charset="UTF-8" />
  </head>
  <body>
    <h2>XHR</h2>
    <button id="get">GET</button><br/>
    <button id="post">POST</button>
    <hr />
    <h2>FORM</h2>
    <form method="GET" action="index.html"><input type="submit" value="GET" /></form>
    <form method="POST" action="index.html"><input type="submit" value="POST" /></form>
    <script>
      navigator.serviceWorker.register('sw.js');
      var $ = document.querySelector.bind(document);
      $('#get').onclick = function () {
        //fetch(new Request('/index.html', { method: 'GET' }));
        var xhr = new XMLHttpRequest();
        xhr.open('GET', '/index.html');
        xhr.send();
      };
      $('#post').onclick = function () {
        //fetch(new Request('/index.html', { method: 'POST' }));
        var xhr = new XMLHttpRequest();
        xhr.open('POST', '/index.html');
        xhr.send();
      };
    </script>
  </body>
</html>


sw.js:

self.addEventListener('fetch', function (evt) {
  console.log(evt.request.method, evt.request.url);
  evt.respondWith(fetch(evt.request));
});


Expected:
POST request is also logged before being passed on to server.

Actual:
It is not logged.

Other POSTs like using fetch() or a form submission do go through the ServiceWorker.
Assignee: nobody → nsm.nikhil
Not only POST is not intercepted but PUT and DELETE are not working as well.
I'm still not sure why POST doesn't work, but I strongly suspect this check in nsHttpChannel::OpenCacheEntry for why other request methods don't get intercepted:

>    else if (!mRequestHead.IsGet() && !mRequestHead.IsHead()) {
>        // don't use the cache for other types of requests
>        return NS_OK;
>    }

We never hit the HTTP cache (and therefore use a cache entry with a synthesized response) in these cases.
Would the solution be to allow hitting the cache when the channel is being intercepted?
Likely yes.
I figured out why POST requests are ignored for interception - in nsXMLHttpRequest::Send, there's a check for POST requests and the LOAD_BYPASS_CACHE and INHIBIT_CACHING flags are added to the channel. Sigh.
Blocks: 1189536
Blocks: 1189582
Josh, I can't mark this to review since you are on PTO (some new bugzilla feature?) but could you take them once you are back?
Comment on attachment 8649449 [details]
MozReview Request: Bug 1176988 - Patch 2 - Remove XHR cache bypass in cast of POST request. r?jdm

https://reviewboard.mozilla.org/r/16409/#review15039

This is a nicer solution than further hacks in the http cache.
Attachment #8649449 - Flags: review+
Comment on attachment 8649448 [details]
MozReview Request: Bug 1176988 - Patch 1 - Always hit cache irrespective of HTTP method if channel is being intercepted. r?jdm

https://reviewboard.mozilla.org/r/16407/#review15037
Attachment #8649448 - Flags: review+
Comment on attachment 8649472 [details]
MozReview Request: Bug 1176988 - Patch 3 - Tests. r?jdm

https://reviewboard.mozilla.org/r/16415/#review15035

::: dom/workers/test/serviceworkers/fetch_event_worker.js:273
(Diff revision 1)
> +    ev.respondWith(new Response('intercepted'));

`'intercepted ' + ev.request.method` for completeness' sake.
Attachment #8649472 - Flags: review+
For some reason, the 'Pragma' test suddenly starts passing after the change in patch 2. https://dxr.mozilla.org/mozilla-central/source/testing/web-platform/tests/XMLHttpRequest/setrequestheader-header-allowed.htm

Is this something to be concerned about or can I update the expectation data and move on?
I think this is due to http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#820 . Previously we were adding LOAD_BYPASS_CACHE and INHIBIT_CACHING to the load flags, so we would append 'once' to any Pragma header value and cause the test's filter not to match. Now that we no longer add those flags,  the test's filter matches so we pass. I think we can safely update the test expectation data.
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/728613ce6c705aac1aa66532b486ff04efca4db6
changeset:  728613ce6c705aac1aa66532b486ff04efca4db6
user:       Nikhil Marathe <nsm.nikhil@gmail.com>
date:       Tue Aug 18 11:30:38 2015 -0700
description:
Bug 1176988 - Patch 1 - Always hit cache irrespective of HTTP method if channel is being intercepted. r=jdm

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/0af43571bfd874f79f5971aa6e3741b810f5fd36
changeset:  0af43571bfd874f79f5971aa6e3741b810f5fd36
user:       Nikhil Marathe <nsm.nikhil@gmail.com>
date:       Tue Aug 18 11:31:27 2015 -0700
description:
Bug 1176988 - Patch 2 - Remove XHR cache bypass in cast of POST request. r=jdm

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/dadd1a75446eb533f5622622658a54f37d6bf71a
changeset:  dadd1a75446eb533f5622622658a54f37d6bf71a
user:       Nikhil Marathe <nsm.nikhil@gmail.com>
date:       Wed Aug 26 08:59:04 2015 -0700
description:
Bug 1176988 - Patch 3 - Tests. r=jdm

Update web-platform-tests expected data
https://hg.mozilla.org/mozilla-central/rev/728613ce6c70
https://hg.mozilla.org/mozilla-central/rev/0af43571bfd8
https://hg.mozilla.org/mozilla-central/rev/dadd1a75446e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Depends on: 1248049
Seems like this is the main cause of bug 1215970.
Depends on: 1215970
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: