Closed Bug 1645781 Opened 10 months ago Closed 10 months ago

Submitting forms with cross origin targets doesn't work.

Categories

(Core :: DOM: Navigation, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla80
Fission Milestone M6a
Tracking Status
firefox80 --- fixed

People

(Reporter: farre, Assigned: farre)

References

Details

Attachments

(3 files)

No description provided.
Severity: -- → S3
Priority: -- → P2
Summary: Submitting forms with cross origin targests doesn't work. → Submitting forms with cross origin targets doesn't work.

When this is fixed, the test for bug 1590762 should start working for fission.

See Also: → 1590762
Assignee: nobody → afarre
Status: NEW → ASSIGNED
Fission Milestone: --- → M6a
Depends on: 1590762
See Also: → 1639410

After much debugging I found out the difference that causes this to happen. If the body being sentin the request is "large enough" we will switch to sending a different nsIInputStream across IPC[1]. Exactly what goes wrong after this I'm still not sure of, but one thing that becomes different as well is the resulting Http request. It looks like it has a limit at a Content-Length of 1024*1024. Below that we get a request like:

 0:06.93 GECKO(42781) [Parent 42781: Main Thread]: E/nsHttp http request [
 0:06.93 GECKO(42781) [Parent 42781: Main Thread]: E/nsHttp   POST http://mochi.test:8888/tests/docshell/test/mochitest/form_submit.sjs HTTP/1.1
 0:06.93 GECKO(42781) [Parent 42781: Main Thread]: E/nsHttp   Host: mochi.test:8888
 0:06.93 GECKO(42781) [Parent 42781: Main Thread]: E/nsHttp   User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:79.0) Gecko/20100101 Firefox/79.0
 0:06.93 GECKO(42781) [Parent 42781: Main Thread]: E/nsHttp   Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
 0:06.93 GECKO(42781) [Parent 42781: Main Thread]: E/nsHttp   Accept-Language: en-US,en;q=0.5
 0:06.93 GECKO(42781) [Parent 42781: Main Thread]: E/nsHttp   Accept-Encoding: gzip, deflate
 0:06.93 GECKO(42781) [Parent 42781: Main Thread]: E/nsHttp   Content-Type: application/x-www-form-urlencoded
 0:06.93 GECKO(42781) [Parent 42781: Main Thread]: E/nsHttp   Content-Length: 1048481
 0:06.93 GECKO(42781) [Parent 42781: Main Thread]: E/nsHttp   Origin: http://mochi.test:8888
 0:06.93 GECKO(42781) [Parent 42781: Main Thread]: E/nsHttp   Connection: keep-alive
 0:06.93 GECKO(42781) [Parent 42781: Main Thread]: E/nsHttp   Referer: http://mochi.test:8888/tests/docshell/test/mochitest/test_bug1645781.html
 0:06.93 GECKO(42781) [Parent 42781: Main Thread]: E/nsHttp   Upgrade-Insecure-Requests: 1
 0:06.93 GECKO(42781) [Parent 42781: Main Thread]: E/nsHttp ]

and above:

 0:06.88 GECKO(43440) [Parent 43440: Main Thread]: E/nsHttp http request [
 0:06.88 GECKO(43440) [Parent 43440: Main Thread]: E/nsHttp   POST http://mochi.test:8888/tests/docshell/test/mochitest/form_submit.sjs HTTP/1.1
 0:06.88 GECKO(43440) [Parent 43440: Main Thread]: E/nsHttp   Host: mochi.test:8888
 0:06.88 GECKO(43440) [Parent 43440: Main Thread]: E/nsHttp   User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:79.0) Gecko/20100101 Firefox/79.0
 0:06.88 GECKO(43440) [Parent 43440: Main Thread]: E/nsHttp   Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
 0:06.88 GECKO(43440) [Parent 43440: Main Thread]: E/nsHttp   Accept-Language: en-US,en;q=0.5
 0:06.88 GECKO(43440) [Parent 43440: Main Thread]: E/nsHttp   Accept-Encoding: gzip, deflate
 0:06.88 GECKO(43440) [Parent 43440: Main Thread]: E/nsHttp   Origin: http://mochi.test:8888
 0:06.88 GECKO(43440) [Parent 43440: Main Thread]: E/nsHttp   Connection: keep-alive
 0:06.89 GECKO(43440) [Parent 43440: Main Thread]: E/nsHttp   Referer: http://mochi.test:8888/tests/docshell/test/mochitest/test_bug1645781.html
 0:06.89 GECKO(43440) [Parent 43440: Main Thread]: E/nsHttp   Upgrade-Insecure-Requests: 1
 0:06.89 GECKO(43440) [Parent 43440: Main Thread]: E/nsHttp ]

Notice the missing Content-Type and Content-Length headers in the request.

[1] https://searchfox.org/mozilla-central/source/ipc/glue/InputStreamUtils.cpp#283-303

Attached file Proposed fix

I debugged this issue a bit. What I see is that:

  1. the content process A submits some data. This is a nsMIMEInputStream + a long long string inputStream. This stream is stored into a nsDocShellLoadState and it's sent to the parent process.
  2. The parent process receives a nsDocShellLoadState containing a nsMIMEInputStream + a pipe stream. The pipe is generated because the string inputStream has more than 1mb of data.
  3. the parent process sends the nsDocShellLoadState to content process B. At that point, nsMIMEInputStream is not serializable and it's sent as a pipe stream. Because of this extra pipe, the MIMEInputStream headers are gone. Also the stream size is gone. Process B, in order to know the amount of data to send, must read the whole stream...

There are different approaches to fix this issue, but I would point out that there are no reasons to send the stream so often around. Why process B needs to receive the whole data? In particular, because process B will send the same data back to the parent process. This means that we end up copying the same data 3 times.

What I suggest is to use IPCBlobInputStream. In this way, when the parent process sends the stream to the process B, it sends just an ID. Process B will receive a IPCBlobInputStream. No data copied.
When process B sends the stream back to the parent process, the parent process retrieves the "original" MIME InputStream from IPCBlobInputStreamStorage using the ID.

I wrote a prototype. It makes the test to pass but there is a bit of cleanup to do.

My patch does the following:

  1. it stores the stream size into a InputStreamLengthWrapper. This means that it's always possible to extract the size from the stream at any time without reading, even if, during the serialization, it's converted into a pipe.
  2. use IPCBlobInputStream in nsDocShellLoadState.
Flags: needinfo?(afarre)
Attachment #9157876 - Flags: feedback?(afarre)
Depends on: 1646933

My patch depends on bug 1646933.

Attachment #9156680 - Attachment description: Bug 1645781 - Test that form submit works with fission. → Bug 1645781 - Part 1: Test that form submit works with fission. r=baku

nsMIMEInputStream was conditionally serializable depending on the
wrapped stream. In general when a stream needs to be sent over IPC, if
it isn't serializable we send it using
InputStreamHelper::SerializeInputStreamAsPipe. There is no reason to
not branch on the wrapped stream when serializing nsMIMEInputStream
and use that, instead of sending the nsMIMEInputStream itself over a
pipe.

Depends on D79672

See Also: → 1648370
See Also: → 1648369
Pushed by afarre@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f7ed7f9dc286
Part 1: Test that form submit works with fission. r=baku
https://hg.mozilla.org/integration/autoland/rev/9df2d1f5a85d
Part 2: Make nsMIMEInputStream always be serializable. r=baku,necko-reviewers
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80
Flags: needinfo?(afarre)
You need to log in before you can comment on or make changes to this bug.