350790 - calling cancel() in on-modify-request should prevent the request to be sent to the server

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

The request is sent currently. It shouldn't be.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: unit test

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

oops, forgot to update that first comment in this testcase...

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

includes the correct unit test

Comment 4

[Darin Fisher]

Comment on attachment 236163 [details] [diff] [review]
patch

Since we compile with logging enabled, why not compact that to only one LOG statement.  Just pass mCanceled as a parameter to the LOG output.

r=darin with that change

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v2

now with a single LOG statement

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in netwerk/protocol/http/src/nsHttpChannel.cpp;
/cvsroot/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp,v  <--  nsHttpChannel.cpp
new revision: 1.294; previous revision: 1.293
done
Checking in netwerk/test/unit/head_http_server.js;
/cvsroot/mozilla/netwerk/test/unit/head_http_server.js,v  <--  head_http_server.js
new revision: 1.6; previous revision: 1.5
done
RCS file: /cvsroot/mozilla/netwerk/test/unit/test_httpcancel.js,v
done
Checking in netwerk/test/unit/test_httpcancel.js;
/cvsroot/mozilla/netwerk/test/unit/test_httpcancel.js,v  <--  test_httpcancel.jsinitial revision: 1.1
done

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 236250 [details] [diff] [review]
patch v2

(this patch has r=darin on the previous attachment)

This is a low-risk patch to allow extensions to prevent requests to be sent to the server.

Comment 8

[Mike Schroepfer]

Comment on attachment 236250 [details] [diff] [review]
patch v2

We are just coming up on final release - we've lived with this behavior since 1.0 - and there are workarounds so 181 drivers would rather play it extra safe and not touch core code.  Great for the trunk - and thanks for tracking this down...

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

OK, but I want to note that I don't think there's a workaround in the case that the extension wants to read the POST data before cancelling.

Comment 10

[Nikitas Liogkas]

It seems that, this only works for cancelling the request for the main page, but not for parts of the page, e.g. images. Is there a workaround for this?

Comment 11

[Christian :Biesinger (don't email me, ping me on IRC)]

This should work for all requests. Can you file a new bug with a testcase? (note: images may be part of the imagelib cache, in which case it is possible that no HTTP request is made for them, so there's nothing to cancel here)

Comment 12

[Nikitas Liogkas]

I did some more digging and the problem is not limited to images. I found out something really surprising. I'm not sure this belongs in a new bug report, so I'll report it here for now. Let me know if you want me to open a new report nevertheless. 

Here is the testcase: I have a simple page with some text and an image, which is being served by my own webserver (Apache 2.0.54 on Fedora). My user agent is Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0. I clear the cache before every experiment. I monitor the HTTP requests and responses from my webserver.

I register an observer for 'http-on-modify-request', and in the observe() method I do:
var httpChannel = subject.QueryInterface(Components.interfaces.nsIHttpChannel);
httpChannel.cancel(Components.results.NS_BINDING_ABORTED);

The surprising thing is the following: I can see an HTTP request for the main page (but not for the image) coming to my webserver, and it sending a 200 OK response back. However, the browser never displays the page; it's as if it ignores the response.

Could it be because the browser sends the request before I have a chance to cancel it (for performance reasons), but then realizes it should have never been sent and thus does not render the received page? 

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

Firefox 2 does not have this bugfix. You'll have to wait for Firefox 3.

Comment 14

[Nikitas Liogkas]

I see. Thanks for the info, I didn't know that.