47936 - Large http posts (> 10K) fail

Status: VERIFIED FIXED

(Core :: Networking, defect, P3)

Description

[Scott MacGregor]

This problem has been around for a long time. Shame on me for not filing it
until now.

If you try to perform an http post for documents that are somewhat large, we fail.

I run into this most often posting a patch to bugzilla as an attachment. I
routinely have attachments (in the form of text patches) that are around 10-14K.
They always fail.

4.7 will post the file to bugzilla but we can't =(.

Comment 1

[Gagan]

eric: You had been looking at something similar a while back (right?) I can't
seem to remember the bug number. But I agree that this needs to be addressed for
nsbeta3. 

Comment 2

[ruslan]

I think this is yours :-)

Comment 3

[Gagan]

no this was deligated! :) u have been volunteered!

On a serious note, I do need to balance the load across the team and for HTTP
there is no better help than you! So could you be so kind to take this bug,
please? Thank you!

Comment 4

[ruslan]

Ok. Presently it doesn't work at all due to the fact that 
nsHTTPEncodeStream::ReadSegments is not implemented. Must be Warren's changes. 
Warren?

Comment 5

[ruslan]

Attachment: Trying to post smth

Comment 6

[ruslan]

Attachment: Now poisting a big attachment

Comment 7

[ruslan]

Hmm. After fixing encode streams it seems like a socket transport buffer is 
busted again .... I fixed this already a long time ago. Must be a regression. 
Weird. Keep on looking.

Comment 8

[Eric Pollmann]

*** Bug 46498 has been marked as a duplicate of this bug. ***

Comment 9

[Eric Pollmann]

A dup of this bug was recently marked dogfood+, transferring status to this bug.

Comment 10

[ruslan]

Attachment: test

Comment 11

[ruslan]

Ok. This is definitely a regression in a webshell or elsewhere cuz the straam 
supplied to AsyncWrite doesn't behave correctly. If our streams were less 
complicated the life would have been much easier. I can always fix it via the 
old way of doing things via transfer buffer in nsSocketStream, unless Gagan 
wants to take another look.

Comment 12

[ruslan]

done

Comment 13

[Tom Everingham]

Attachment: test

Comment 14

[Tom Everingham]

Attachment: test

Comment 15

[Tom Everingham]

Attachment: test - small html

Comment 16

[Tom Everingham]

Ruslan, I've tried to post a 28k text file and an 18k html file and neither will
post. When I click on the submit button, nothing happens. Smaller files post
fine. I'm using the mozilla build from 8/28 on WinNT.

Comment 17

[ruslan]

Hmm. I did test it with 40K+ file ...

Comment 18

[Tom Everingham]

Attachment: test using 4.7 - 18k html file

Comment 19

[Tom Everingham]

I cannot repeat the last test using todays mozilla build.  For some reason I 
cannot post the 18k attachment

Comment 20

[ruslan]

Ok. There's another regression here: here's the guilty code around line 1261:

  if ((NS_SUCCEEDED(rv) && (0 == totalBytesWritten)) ||         // eof, or
      (GetFlag(eSocketWrite_Async) && (0 == mWriteCount))) {    // wrote 
everything
    mSelectFlags &= (~PR_POLL_WRITE);
    rv = NS_OK;
  }

the problem is that when we write from file - mWriteCount is always zero, cuz we 
don't know the length in advance. If PR_Write retruns would_block for whatever 
reason (that's why it's hard to reproduce this bug while debugging the code) - 
it will bail out of loop and hit this check - which is bogus. Simply removing it 
will break people who use AsyncWrite with write count argument (I don't know if 
we have any, but we may). Gagan - can you look into it (looks like old rpotts 
code) and think of what's the right fix is for this so we don't break anybody?

Comment 21

[Gagan]

cc'ing rpotts

Comment 22

[ruslan]

Attachment: Bit attachment

Comment 23

[ruslan]

Fixed now. Tom, verify please

Comment 24

[Tom Everingham]

Attachment: test attach

Comment 25

[Tom Everingham]

verif:
Win NT 2000090708

Comment 26

[Randell Jesup [:jesup] (needinfo me)]

Attachment: re-upload from freebsd 4.1 20000925xx of the last file (17K)

Comment 27

[Randell Jesup [:jesup] (needinfo me)]

Attachment: upload of 40930-byte GIF

Comment 28

[Randell Jesup [:jesup] (needinfo me)]

Note: bug 54081 may be a regression due to the fix for this bug - or something
else entirely.  Data is being corrupted on upload, at an offset just a little
larger than the test file that was being used for verifying the fix to this bug.

Anyone who was involved in this bug (and is on this CC list) may want to look at
bug 54081.