315767 - Submission of large files using multipart-post doesn't work

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[jhp (no longer active)]

The testcase (https://bugzilla.mozilla.org/attachment.cgi?id=197763) fails when trying to submit a large file.  For me, it works fine when attaching a file with size 28K.  But when trying to submit a file of size 100K, I see the appropriate network messages on the status bar (that it is communicating with the destination server), but the echo.sh page never loads.

Comment 1

[Allan Beaufour]

(In reply to comment #0)
> The testcase (https://bugzilla.mozilla.org/attachment.cgi?id=197763) fails when
> trying to submit a large file.  For me, it works fine when attaching a file
> with size 28K.  But when trying to submit a file of size 100K, I see the
> appropriate network messages on the status bar (that it is communicating with
> the destination server), but the echo.sh page never loads.
> 

Stupid question: It's not just a limit on echo.sh? (I haven't tested myself)

Comment 2

[jhp (no longer active)]

Doesn't seem to be a problem with the server.  We created an HTML testcase, using HTML file submission, and that one worked fine.  Debugging a little more, it looks like the file input stream is returning NS_BASE_STREAM_WOULD_BLOCK at some point.  No idea why, yet.

CCing Darin, since he wrote some of the submission code and might be able to help.

Comment 3

[jhp (no longer active)]

So we get status == NS_BASE_STREAM_WOULD_BLOCK in nsXFormsSubmissionElement::OnStopRequest(), which is called because we set the notification callbacks to this class object at the bottom of nsXFormsSubmissionElement::SendData().  The listener is set as the output end of an nsIPipe, whose docs explain that a non-blocking stream will return NS_BASE_STREAM_WOULD_BLOCK if more input can't be read without blocking the thread.

Creating the pipe with the output set to blocking won't work, since this blocks the thread indefinitely.  Reading the nsIPipe docs, I'm guessing that we need to use an nsIAsyncOutputStream, and use AsyncWait to get the remainder of the data.

Comment 4

[jhp (no longer active)]

Attachment: http log - failure

Log of HTTP submit and response, using large file (~150K).  As you can see, response is cut off.

Comment 5

[jhp (no longer active)]

Attachment: NSPR log of HTTP - failure

NSPR log of Http:5 of same failure case.

Comment 6

[Darin Fisher]

Attachment: v1 patch

This should work.  The problem is that we were passing 0 for the segmentSize of the pipe, which causes the pipe to use its default configuration, ignoring the value of maxSize passed to it.  The real fix to this bug is to change NS_NewPipe to honor maxSize even when segmentSize is 0.

Comment 7

[jhp (no longer active)]

Comment on attachment 214573 [details] [diff] [review]
v1 patch

Sweet, works for me.

However, I did notice that the throbber was not doing its thing when awaiting the response.  Since I am testing with a large file, it took a while to submit and get the response.  But the only indication that any waiting was taking place came from looking at the status bar.  It would be nice if the throbber also showed progress.

Comment 8

[Darin Fisher]

Hmm.. not sure why the throbber wouldn't spin.  If the progress bar is moving, then the throbber should as well.  That's really odd.

Comment 9

[jhp (no longer active)]

Sorry, should have been more specific.  There is no progress bar, but I do see the "Transferring from..." text in the status bar.

Comment 10

[Darin Fisher]

Ah, ok.  That still seems wierd.  Afterall, we hook up the document's loadgroup to the channel used for submission, and that should be sufficient to drive the proper progress notifications.

Comment 11

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

I believe we only do the throbber/progress bar stuff if we have a channel with LOAD_DOCUMENT_URI In the loadgroup, or something like that...

Comment 12

[jhp (no longer active)]

Attachment: patch v2

Sweet!  Adding the LOAD_DOCUMENT_URI flag made the throbber work.  Here's that patch, with Darin's fix.

Comment 13

[jhp (no longer active)]

Checked in to trunk. -> FIXED

Darin, did you already open a bug for the NS_NewPipe issue?

Comment 14

[Darin Fisher]

No, I did not.  If you can do that, I'd appreciate it.