336501 - crash if update post containing file to upload [@ nsBufferedInputStream::Read]

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Paul Yanchenko]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3

Firefox crashes if POST-form with file is submitted and then refreshed.

Reproducible: Always

Steps to Reproduce:
1. create or find the html form with file input
2. submit the form
3. press f5




It's appeared after updating to 1.5.0.3 from 1.5.0.2 with partial update.

Comment 1

[Ryan Flint [:rflint] (ping via IRC for reviews)]

WFM Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3

Could possibly be caused by an extension. Are you able to reproduce the crash in safe mode (http://kb.mozillazine.org/Safe_mode) and if so, could you paste the Talkback ID (http://kb.mozillazine.org/Talkback) here?

Comment 2

[Paul Yanchenko]

I can't reproduce this bug in Safe Mode. You are right, this is caused by extension. I suppose, by Web Developer Extension (1.0.2). Because it causes other bugs.

I using following extensions:

 * DOM Inspector 1.8.0.3
 * Web Developer 1.0.2
 * Paste and Go 0.4.3
 * Tab Sidebar 1.0.3
 * Live HTTP Headers 0.11

No any updates exists for these extensions.

What else can I do for help you to reproduce this bug on your side?

Comment 3

[Paul Yanchenko]

By enumerative technique (by uninstalling one-by-one) I found which extension crashes firefox, this is "Live HTTP Headers" version 0.11 (latest, as I know).

Comment 4

[Adam Guthrie]

I'd still like a talkback ID if you feel like getting one for the crash. http://kb.mozillazine.org/Talkback

Comment 5

[Boris Zbarsky [:bzbarsky]]

Is there a talkback ID for this crash?

Comment 6

[Paul Yanchenko]

I'm sorry, it's seems I did not install Talkback when installed Firefox. I need to download full version and reinstall firefox. Wait 10 min.

Comment 7

[Paul Yanchenko]

Hmm. I never used Talkback before and maybe I'm doing something wrong, but I've got the Talkback window when firefox was crashed again, there was a form with my email and details. I filled it out, placed link to this bug in details and pressed Send. After data is sent - the window has been disappered without any confirmation dialog and I did not got any IDs. "Talkback ID" - it must be a number, yes?

Comment 8

[Ryan Flint [:rflint] (ping via IRC for reviews)]

TB18348159:

Stack Signature	 msvcrt.dll + 0x36fa3 (0x77c36fa3) e98e3c05
Product ID	Firefox15
Build ID	2006042618
Trigger Time	2006-05-05 14:47:11.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	msvcrt.dll + (00036fa3)
URL visited	
User Comments	This is a talkback id generated specially for bug #336501 https://bugzilla.mozilla.org/show_bug.cgi?id=336501
Since Last Crash	174 sec
Total Uptime	174 sec
Trigger Reason	Access violation
Source File, Line No.	N/A
Stack Trace 	
msvcrt.dll + 0x36fa3 (0x77c36fa3)
nsBufferedInputStream::Read  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/netwerk/base/src/nsBufferedStreams.cpp, line 316]
nsMultiplexInputStream::Read  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/xpcom/io/nsMultiplexInputStream.cpp, line 200]
nsMultiplexInputStream::Read  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/xpcom/io/nsMultiplexInputStream.cpp, line 200]
nsMIMEInputStream::Read  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/netwerk/base/src/nsMIMEInputStream.cpp, line 262]
nsBufferedInputStream::Fill  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/netwerk/base/src/nsBufferedStreams.cpp, line 388]
nsBufferedInputStream::ReadSegments  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/netwerk/base/src/nsBufferedStreams.cpp, line 351]
nsHttpTransaction::ReadSegments  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/netwerk/protocol/http/src/nsHttpTransaction.cpp, line 392]
nsHttpConnection::OnSocketWritable  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/netwerk/protocol/http/src/nsHttpConnection.cpp, line 559]
nsHttpConnection::OnOutputStreamReady  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/netwerk/protocol/http/src/nsHttpConnection.cpp, line 771]
nsSocketTransport::OnSocketReady  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/netwerk/base/src/nsSocketTransport2.cpp, line 1469]
nsThread::Main  [c:/builds/tinderbox/Fx-Mozilla1.8.0/WINNT_5.2_Depend/mozilla/xpcom/threads/nsThread.cpp, line 134]
kernel32.dll + 0xb50b (0x7c80b50b)

Comment 9

[Boris Zbarsky [:bzbarsky]]

Darin, biesi, any ideas?

Comment 10

[Adam Guthrie]

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

Comment 11

[Adam Guthrie]

See also bug 290003.

Comment 12

[Nickolay_Ponomarev]

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

Comment 13

[Nickolay_Ponomarev]

Note: reporter of bug 336382 (marked duplicate of this one) says he can reproduce the crash in safe mode by uploading an image to http://brotherli.ch/temp/base64.php and reloading using F5.

Comment 14

[Alfonso Martinez]

I have sent TB19713030Q after realoading a POST with a file. 
Now I've uninstalled Live HTTP Headers and Firefox no longer crash

(winXP Sp2, Firefox 1.5.0.4)

Comment 15

[Boris Zbarsky [:bzbarsky]]

What's the URI to the "Live HTTP Headers" extension?

Also, I suspect there may be two separate issues here -- one with Live HTTP Headers, and a separate issue for bug 336382.  I'll reopen that bug.

Comment 16

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

http://livehttpheaders.mozdev.org/ seems to be it

Comment 17

[Boris Zbarsky [:bzbarsky]]

Attachment: nsHeaderInfo.js from Live HTTP Headers

So would it be relevant that Live HTTP Headers closes the POST data stream?  The relevant code is in the visitPostHeaders() function in this file.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Ah, yeah.  That's it.  No wonder I couldn't reproduce this on trunk!

On branch, if someone calls nsBufferedInputStream::Close that will call nsBufferedStream::Close which deletes mBuffer and sets it to null and sets mCursor to 0.  It does NOT change mFillPoint, however.

Then nsBufferedInputStream::Read is called.  If mFillPoint > mCursor, it memcopies from mBuffer + mCursor.  This pointer is null if Close() has been called, so we crash.

We should probably at least set mFillPoint to 0 in nsBufferedStream::Close on branch...  Once that's done, we might have to adjust Read and ReadSegments a tad to deal with that.

Alternately, we could null-check mStream up front in Read and ReadSegments and bail out if it's null.

Comment 19

[timeless]

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

Comment 20

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

it seems more important to fix livehttpheaders not to close the stream, because as long as it does so that will probably trigger incorrect behaviour in mozilla in various places.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Well.  We should be doing both.  http://mozdev.org/bugs/show_bug.cgi?id=12050 covers the LiveHTTPHeaders end of things; I just commented there.

Comment 22

[Daniel Veditz [:dveditz]]

Not blocking 1.8.0.x but would consider approving a safe patch.

Comment 23

[Darin Fisher]

I'll add some null checks internally to harden us against extensions like this.

Comment 24

[Darin Fisher]

Attachment: v1 patch

Comment 25

[Boris Zbarsky [:bzbarsky]]

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

Looks good.

Comment 26

[Darin Fisher]

Regression test added to the trunk.

Comment 27

[Darin Fisher]

fixed1.8.1

Comment 28

[timeless]

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

Comment 29

[timeless]

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

Comment 30

[Mats Palmgren (inactive)]

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

Comment 31

[Daniel Veditz [:dveditz]]

Nominating for the next 1.5 release since there's a patch now.

Comment 32

[Kai Engert [:KaiE:]]

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

Comment 33

[Mats Palmgren (inactive)]

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

Comment 34

[Daniel Veditz [:dveditz]]

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

approved for 1.8.0 branch, a=dveditz for drivers

Comment 35

[Darin Fisher]

fixed1.8.0.7

Comment 36

[alice nodelman [:alice] [:anode]]

This is a one line fix to set mFillPoint = 0.

Verified by inspection for 1.8.0.7 and 1.8.1.

Comment 37

[Adam Guthrie]

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

Comment 38

[Daniel Veditz [:dveditz]]

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