Closed Bug 1208124 Opened 9 years ago Closed 9 years ago

In firefox 41 it is impossible to make a custom nsIProtocolHandler/nsIChannel which supports POST data.

Categories

(Core :: Networking, defect)

41 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed
firefox45 --- affected
firefox46 --- affected
firefox47 --- fixed

People

(Reporter: mkm, Assigned: mkm)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch nsDocShell.cpp.patch (obsolete) — Splinter Review
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:41.0) Gecko/20100101 Firefox/41.0
Build ID: 20150917150946

Steps to reproduce:

Im a developer of a custom protocol handler / channel which needs to accept POST data. Before firefox 41 we implemented nsIUploadChannel and nsIHttpChannel, nsIHttpChannel is neccessary due to a bug in nsDocShell.cpp.

In firefox 41 the interface nsIHttpChannel has been tagged builtinclass that means it's no longer possible to implement that interface in a javascript extension. Which means nsDocShell.cpp refuses to call SetUploadStream on our nsIUploadChannel.

The root cause for this problem is in nsDocShell.cpp:10731

  //
  // If this is a HTTP channel, then set up the HTTP specific information
  // (ie. POST data, referrer, ...)
  //
  if (httpChannel) {

It should instead check if the channel support nsIUploadChannel because POST requires nsIUploadStream but not neccessarily nsIHttpChannel.

There are two solutions for this either remove the builtinclass attribute on nsIHttpChannel (revert to old behavior), or use the applied patch to fix the root cause which is a bug in nsDocShell.cpp.
Component: Untriaged → Networking
Product: Firefox → Core
Attachment #8665512 - Flags: review?(mcmanus)
Attachment #8665512 - Flags: review?(mcmanus) → review?(bzbarsky)
Comment on attachment 8665512 [details] [diff] [review]
nsDocShell.cpp.patch

> nsIHttpChannel is neccessary due to a bug in nsDocShell.cpp

Did that bug get reported, or just worked around?

The patch looks reasonable, though AddHeadersToChannel should continue to be conditional on httpChannel being non-null, since it will just fail otherwise anyway.  This patch also needs a commit message and author info...  Please add those and fix the AddHeadersToChannel thing and let me know whether you can push to try yourself or whether I should do it.
Attachment #8665512 - Flags: review?(bzbarsky) → review+
AddHeadersToChannel is only called on http channels now.
Attachment #8665512 - Attachment is obsolete: true
Do you need me to push that to try?
Flags: needinfo?(mkm)
(In reply to Boris Zbarsky [:bz] from comment #3)
> Do you need me to push that to try?

It would be very appreciated if you can push it to the respective repositories.
Flags: needinfo?(mkm)
Assignee: nobody → mkm
Status: UNCONFIRMED → NEW
Ever confirmed: true
Try run is looking good....
Keywords: checkin-needed
Thank you for the patch!
https://hg.mozilla.org/mozilla-central/rev/8bbed05c1661
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Depends on: 1241100
Depends on: 1241377
This was backed out by bug 1241100 in Firefox 44-46. Re-fixing depends on bug 1241377
You need to log in before you can comment on or make changes to this bug.