1624318 - retransmit a form via 'reload page' fails

Status: VERIFIED FIXED

(Core :: Networking, defect, P2)

Description

[Thomas Hänig]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:74.0) Gecko/20100101 Firefox/74.0

Steps to reproduce:

0 - my form's url is like this: http://my_site/index.php?target=upload (with a 'get'-parameter)
1 - submit a filled out form (and get the respective response from server)
2 - press <F5> / <Ctrl><R> or button 'reload current page'
3 - answer the popup "To display this page, Firefox must send information that will repeat any action (such as a search or order confirmation) that was performed earlier." with 'Resend'

Actual results:

the form gets not retransmitted, the $_POST array (php) is empty), $_GET contains paramters from the URL bar

what puzzles me a bit is, the servers log shows a 'POST'

Expected results:

the form shoul be retransmitted and the respective server variables filled (as it has been up to FF vers. 74)

Comment 1

[Thomas Hänig]

This seems to happen only if there is a <input type='file'> within the form with a file selected.
If there is no file selected or no <input type='file'> present, the form retransmits fine.

Comment 2

[Andrei Purice]

Hey Thomas,

Can we have a test case or a page where we can test this ?

Can you also test this on the latest Nightly? Download the build from : https://www.mozilla.org/en-US/firefox/nightly/all/ .

Comment 3

[Thomas Hänig]

Attachment: simple php script with a form which on submit shows $_GET, $_POST, $_FILES

Comment 4

[Thomas Hänig]

with nightly the form works the same as with FF74, meaning refresh does not retransmit the file

Comment 5

[Andrei Purice]

Adding a component for this issue in order to get the dev team involved.
If you feel like it's not the correct one please feel free to asign it to a proper one.

Comment 6

[Dragana Damjanovic [:dragana]]

This does not sound like a necko problem, let's try with DOM

Comment 7

[Hsin-Yi Tsai (she/her) | away [:hsinyi]]

Maybe John can take a look here, thanks.

Comment 8

[John Dai[:jdai]]

I'll take a look. Keep NI for tracking.

Comment 9

[John Dai[:jdai]]

After I click resend, I saw some warning message.

[Parent 13309, Socket Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80470002: file /mnt/mydrive/bug/1624318/netwerk/base/nsFileStreams.cpp, line 82
[Parent 13309, Main Thread] WARNING: This file has not been opened (or could not be opened). Sending an invalid file descriptor to the other process!: file /mnt/mydrive/bug/1624318/netwerk/base/nsFileStreams.cpp, line 583
[Child 13643, Main Thread] WARNING: Received a bad file descriptor index!: file /mnt/mydrive/bug/1624318/netwerk/base/nsFileStreams.cpp, line 622
[Child 13643, Main Thread] WARNING: This file has not been opened (or could not be opened). Sending an invalid file descriptor to the other process!: file /mnt/mydrive/bug/1624318/netwerk/base/nsFileStreams.cpp, line 583
[Child 13643, Main Thread] WARNING: NS_ENSURE_TRUE(request) failed: file /mnt/mydrive/bug/1624318/netwerk/base/nsLoadGroup.cpp, line 591
[Child 13643, Main Thread] WARNING: This file has not been opened (or could not be opened). Sending an invalid file descriptor to the other process!: file /mnt/mydrive/bug/1624318/netwerk/base/nsFileStreams.cpp, line 583
[Parent 13309, Main Thread] WARNING: Received a bad file descriptor index!: file /mnt/mydrive/bug/1624318/netwerk/base/nsFileStreams.cpp, line 622
[Parent 13309, StreamTrans #88] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file /mnt/mydrive/bug/1624318/netwerk/base/nsFileStreams.cpp, line 179
[Parent 13309, StreamTrans #88] WARNING: 'NS_FAILED(rv)', file /mnt/mydrive/bug/1624318/xpcom/io/nsMultiplexInputStream.cpp, line 361
[Parent 13309, StreamTrans #88] WARNING: 'NS_FAILED(mStream->Available(&size))', file /mnt/mydrive/bug/1624318/xpcom/io/InputStreamLengthHelper.cpp, line 35
[Parent 13309, Main Thread] WARNING: Received a bad file descriptor index!: file /mnt/mydrive/bug/1624318/netwerk/base/nsFileStreams.cpp, line 622
[Parent 13309, StreamTrans #91] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file /mnt/mydrive/bug/1624318/netwerk/base/nsFileStreams.cpp, line 179
[Parent 13309, StreamTrans #91] WARNING: 'NS_FAILED(rv)', file /mnt/mydrive/bug/1624318/xpcom/io/nsMultiplexInputStream.cpp, line 361
[Parent 13309, StreamTrans #91] WARNING: 'NS_FAILED(mStream->Available(&size))', file /mnt/mydrive/bug/1624318/xpcom/io/InputStreamLengthHelper.cpp, line 35
[Parent 13309, Main Thread] WARNING: 'NS_FAILED(rv)', file /mnt/mydrive/bug/1624318/netwerk/protocol/http/HttpBaseChannel.cpp, line 1094
[Parent 13309, Main Thread] WARNING: This file has not been opened (or could not be opened). Sending an invalid file descriptor to the other process!: file /mnt/mydrive/bug/1624318/netwerk/base/nsFileStreams.cpp, line 583
[Child 13643, Main Thread] WARNING: Received a bad file descriptor index!: file /mnt/mydrive/bug/1624318/netwerk/base/nsFileStreams.cpp, line 622

Comment 10

[John Dai[:jdai]]

Here is the result from my mozregression, it seems from Bug 1583700.

Last good revision: f748a3d2cdf108e9443fd15332efe477c7c398a9
First bad revision: 25b533bff4051e6a8177bbc83eed49ac460e109e
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f748a3d2cdf108e9443fd15332efe477c7c398a9&tochange=25b533bff4051e6a8177bbc83eed49ac460e109e

Comment 11

[Matt Woodrow (:mattwoodrow)]

baku, do you know how form submission of files works (in e10s)?

It looks like we submit it correctly initially, but the copy of the post data we send back with RedirectToRealChannelArgs is broken and doesn't have a reference to the file.

That's the data that we put into session history (currently, parent process session history will be nice), so when we reload we try to submit the post data that doesn't have a valid file handle.

Comment 12

[Andrea Marchesini [:baku]]

Files and Blobs can be read via their nsIInputStream. There are different kinds of inputStreams based on the type of File/Blob and where they have been created. When submitting an OS file, the DOM File object is created on the parent process and the content process receives a IPCBlob. See more here: https://searchfox.org/mozilla-central/rev/a4d62e09a4c46aef918667fa759bf9ae898dc258/dom/file/ipc/IPCBlobUtils.h#14-212
That long long comment explains the whole magic.

When submitting a file, the entrypoint is here:
https://searchfox.org/mozilla-central/rev/a4d62e09a4c46aef918667fa759bf9ae898dc258/dom/html/HTMLFormSubmission.cpp#413

In FSMultipartFormData, we extract the nsIInputStream from the File object, wrapped in a bufferedInputStream. Note that, because of the IPCBlobInputStream the content of the file is not sent to the content process. We just have a ref of it (see the IPCBlobUtils comment).

During the 'deserialization' of the nsIInputStream on the parent process, we are able to replace the IPCBlobInputStream with the original nsIFileInputStream (pointing to the OS file). The final deserialized stream is sent to necko, which completes the network operation.

It seems that RedirectToRealChannelArgs() introduces some form of regression here.

Comment 13

[Andrea Marchesini [:baku]]

Ok here is what is happening:

1. The form inputStream is created on the content process. The result is this stream:

nsMIMEInputStream
  - nsMultiplexInputStream
    - nsStringInputStream
    - nsBufferedInputStream
      - IPCBlobInputStream
    - nsStringInputStream

As you can see, the real nsIFileInputStream is unknown by the content-process. IPCBlobInputStream has a ref to it. The real nsIFileInputStream is on the parent process only.

2. Then the stream is serialized and sent to the parent process. The parent process deserializes the stream replacing IPCBlobInputStream with nsFileInputStream. Necko receives the stream and it completes the first POST loading.

3. mozilla::net::DocumentChannelParent::RedirectToRealChannel is called. And the stream is fully serialized. the FileInputStream is sent to the content process.

4. the content process sends the same FileInputStream back to the parent process... this kind of work.

5. the content process sends the same FileInputStream back to the parent process. This doesn't work and we have the bug.

Here is what I suggest:
Let's use IPCBlobInputStream here, removing the 'blob' part of it and generalize it to handle any kind of stream.
Matt and I spoke on IRC about it. We are going to experiment with this plan.

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

This was regressed by a networking bug, moving back there.

Comment 15

[Honza Bambas (:mayhemer)]

Matt. are you willing to work on this? Treating as a P2 since this is out for some time and is a corner case and doesn't affect e.g. internet banking or eshop order submitting when re-POSTing.

Comment 16

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1624318 - Serialize replacement config input stream as a blob. r?baku

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/965dd8d808bc
Serialize replacement config input stream as a blob. r=baku,necko-reviewers,valentin

Comment 18

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/965dd8d808bc

Comment 19

[Cristian Fogel [:cfogel]]

Verified with 77.0.1 on Windows 10, macOS 10.15.5, Ubuntu 20.