684806 - Unable to upload 0KB file if an observer reads the upload stream and rewinds it

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[ssetty]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0.1) Gecko/20100101 Firefox/6.0.1
Build ID: 20110830092941

Steps to reproduce:

i am trying to upload a zero kb file using basic form.


Actual results:

i am unable to upload zero kb file in Firefox 5 but same file upload works on Firefox 3.6


Expected results:

it should submit the form irrespective of the size of the file.

Comment 1

[:aceman]

Can you provide a page where this happens?

Comment 2

[:aceman]

I just checked and I can upload a 0 bytes file to my webmail into a message and it accepts it and sends the email. Using FF6 on Win XP, 32bit. This bugzilla system does not accept empty files, but that is analyzed on the server after the upload succeeds.

Comment 3

[ssetty]

hi aceman,
     i have just created simple file upload form using html and tried to upload a 0kb file. when i submit it. it just doesn't do any thing and even i have tried with file more than 0 kb file then it is successfully submitting it. webemail may be using flash object for upload.

Comment 4

[:aceman]

My webmail does not use flash. Can you post an URL to your form? Or can you attach the source html so we can try it?

Comment 5

[ssetty]

Attachment: file upload form

this file has upload dorm

Comment 6

[ssetty]

Comment on attachment 558518 [details]
file upload form

this file is file upload form

Comment 7

[ssetty]

Attachment: it just says file uploaded

it just says file uploaded

Comment 8

[ssetty]

please find attachment fileupload.html and result.html.

fileupload.html form will submit to result.html file.

i am using tomcat server.

Comment 9

[:aceman]

Ok, on the attached testcase I can reproduce the problem. I do not see any obvious bug in your html. I tested on apache server with FF 6 and 8. Both have the problem. The Web console shows a HTTP POST request for the result.html file, however the server is never contacted (according to its log) and no reply is received by FF.

Comment 10

[Mounir Lamouri (:mounir)]

ssety and aceman, one of you could host those file somewhere and make a test case?

Comment 11

[Boris Zbarsky [:bzbarsky]]

The attached testcase worksforme on Mac, posting to http://landfill.mozilla.org/ryl/echo.cgi

aceman, if you use that url as the action, do things work for you?  If not, is this Windows-only?

Comment 12

[:aceman]

Attachment: testcase using landfill.mozilla.org echo script

Updated the testcase to use that echo script and also fixed the HTML (strict mode, comments with 2 dashes (w3c validator complained), etc.)

Comment 13

[:aceman]

Attachment: screenshot of webconsole with failed upload

I used the attached form using your echo script. The screenshot shows there is no reply from landfill.mozilla.org (I assume it was never contacted). It is from Win XP.
Comment 9 is made on Linux, that is why I changed the platform fields.

Comment 14

[:aceman]

The testcase works fine in Safe mode. Must be some extension. Sorry about that :(

Comment 15

[:aceman]

ssetty, can you post your list of extensions? You can find it in about:support. We can compare them to mine.

Comment 16

[Mounir Lamouri (:mounir)]

I'm able to reproduce this with yesterday trunk in debug on GNU/Linux.

Comment 17

[Mounir Lamouri (:mounir)]

(In reply to Mounir Lamouri (:volkmar) from comment #16)
> I'm able to reproduce this with yesterday trunk in debug on GNU/Linux.

Goes without saying: no extensions and no plugins.

Comment 18

[Mounir Lamouri (:mounir)]

Oh, ignore comment 16 and comment 17. I was using an invalid file :(

aceman, do you use NoScript? I believe the issue you see can come from NoScript.
Could you do this and try againg:
1. Go to NoScript preferences;
2. 'Advanced' tab;
3. 'XSS' tab;
4. Uncheck "Turn cross-site POST requests [...]";
5. Click 'OK';

On a Firefox 5 clean profile with only NoScript, I had the same issue. Doing that fixed it.

Comment 19

[:aceman]

Yes, I also found out it is due to noscript (by disabling it).
Unchecking the feature you say fixes the problem too.
However, shy would it only stop empty files and it doesn't seem to convert the requests to GET (webconsole still shows POST).
And also, when I tested the original testcase (in obsoleted attachments) on my local server, the request was not cross-site. The feature seems to be broken and interfere wrongly. I'll contact noscript author.

Comment 20

[Giorgio Maone [:ma1]]

I'm traveling and commenting here from my smartphone, but the data gathered so far seems to hint at an edge case mishandling in the injection checks on cross-site POST requests. Will check as soon as I'm back (tomorrow).

Comment 21

[ssetty]

Attachment: about:support

Comment 22

[ssetty]

below is the list of my extension in firefox
Name               Version  Enabled  ID  
DownThemAll!       2.0.7    true {DDC359D1-844A-42a7-9AA1-88A850A938A8} 
FiddlerHook        2.3.4.4  true [e-mail] 
Firebug            1.8.1    true [e-mail] 
Flash Video        2.0.29   true [e-mail] 
Downloader Youtube Downloader Facebook
Flashbug           1.8.0    true [e-mail] 
Live HTTP headers  0.17     true {8f8fe09b-0bd3-4470-bc1b-8cad42b8203a} 
Page Speed         1.12     true {e3f6c2cc-d8db-498c-af6c-499fb211db97} 
WOT                20110704 true {a0d7ccb3-214d-498b-b4aa-0e8fda9a7bf7} 
YSlow              3.0.4    true [e-mail] 
Click to call      5.6.0.8153 false {82AF8DCA-6DE9-405D-BD5E-43525BDAD38A} 
with Skype
HttpWatch          7.0.23   false {1E2593B2-E106-4697-BCE7-A9D30DE05D73}
Professional Edition 
Java Console       6.0.24   false {CAFEEFAC-0016-0000-0024-ABCDEFFEDCBA} 
Java Console       6.0.26   false {CAFEEFAC-0016-0000-0026-ABCDEFFEDCBA} 
Leak Monitor       0.4.6    false {1ed6b678-1f93-4660-a9c5-01af87b323d3} 
Prism for Firefox  1.0b3    false [e-mail] 
RapidShare         1.0      false [e-mail] 
DownloadHelper 
Veoh Video Compass 1.5.2    false [e-mail]

Comment 23

[:aceman]

No Noscript??? Can you confirm the problem doesn't happen in Firefox safe mode? https://support.mozilla.com/en-US/kb/Safe+Mode

Comment 24

[ssetty]

i am unable to reproduce problem in Firefox safe mode.

Comment 25

[Boris Zbarsky [:bzbarsky]]

OK, can you hunt down which extension is causing the problem for you?

HttpWatch would be my first guess to try, followed by Firebug and Live HTTP headers.

Comment 26

[ssetty]

okay,i need to disable extension 1 by 1. or should uninstall extensions.

Comment 27

[:aceman]

Httpwatch seems disabled in the list.
ssetty: yes try to disable them.

Comment 28

[Giorgio Maone [:ma1]]

I cannot reproduce on a clean profile with just NoScript installed.

@ssetty: another suspect is Live HTTP Headers, but you seems to have tons of HTTP monitoring/fiddling extensions, and I wouldn't be surprised by multiple conflicts :/

Comment 29

[:aceman]

Giorgio, have you enabled the option from comment 18?

These are the extensions I have enabled on one of my machines where I see the problem. Just disabling noscript out of them fixes the problem:
Card Games                  0.98          true  [e-mail]
DOM Inspector               2.0.10        true  [e-mail]
IE Tab +                    2.04.20110724 true  [e-mail]
MinimizeToTray revived (MinTrayR) 1.0     true  [e-mail]
Mozilla Archive Format            2.0.1   true  {7f57cf46-4467-4c2d-adfa-0cba7c507e54}
NoScript                    2.1.2.6       true  {73a6fe31-595d-460b-a920-fcc0f8843232}
Nuke Anything Enhanced      1.0.2         true  {1ced4832-f06e-413f-aa14-9eb63ad40ace}
pdfit                       1.15          true  [e-mail]
PrefBar                     6.0.1         true  {8A6C82A1-F6C9-481a-AAE7-C96444C9A754}
printpdf                    0.76          true  [e-mail]
Slovníky slovenského pravopisu 2.03.2     true  [e-mail]
Web Developer                  1.1.9      true  {c45c406e-ab73-11d8-be73-000a95be3b12}

Comment 30

[Giorgio Maone [:ma1]]

(In reply to aceman from comment #29)
> Giorgio, have you enabled the option from comment 18?

It is enabled by default.

As I said, looks like a multiple extensions conflict.
Could you try disabling all your extensions except NoScript, then readding them one by one?

Comment 31

[:aceman]

Having only noscript enabled with that option enabled is enough to reproduce the problem (WinXP). Turning off that option fixes the problem. Should I attach my noscript configuration? Scripts are currently globally enabled.

Comment 32

[:aceman]

Attachment: config exported from noscript

Comment 33

[:aceman]

Attachment: noscript prefs cut cut out of prefs.js

Comment 34

[Giorgio Maone [:ma1]]

Attachment: A minimal request observer which reads the upload stream and rewinds it

Any http-on-modify-request observer which needs to read the upload stream (like NoScript and any HTTP monitoring extension) triggers this bug, which probably consists in the upload stream being left in an inconsistent state when it's read and rewound, but only if a 0-length file is present.

All you need to do to reproduce on a clean profile is running this script from a chrome privileged context (e.g. the Error Console) and using the testcase in attachment 558743 [details] immediately after (the observer gets removed on each upload).

Comment 35

[ssetty]

i have tested by disabling 1 by 1 extension.it is working fine.
DownThemAll!       2.0.7  is extension causing issue for me.

Comment 36

[Boris Zbarsky [:bzbarsky]]

Giorgio, thanks.  

Looks like the issue is that Seek() on a buffered stream doesn't pass the seek on to the underlying stream in cases when the seek is to a position that's already in the buffer.  Which means the "reopen on rewind" behavior doesn't kick in, the file stream remains closed, and things fail.

If the file stream is nonzero in size then mBufferStartOffset in the buffered stream ends up nonzero, 0-mBufferStartOffset ends up as a really big number, and hence > mFillPoint and we end up actually doing the seek.

Comment 37

[Boris Zbarsky [:bzbarsky]]

Attachment: Make sure that zero-size files wrapped in buffered streams are correctly reopened by a seek after EOF.

Comment 38

[Honza Bambas (:mayhemer)]

Comment on attachment 567002 [details] [diff] [review]
Make sure that zero-size files wrapped in buffered streams are correctly reopened by a seek after EOF.

Review of attachment 567002 [details] [diff] [review]:
-----------------------------------------------------------------

You should set mEOF = true also in nsBufferedInputStream::Read when mBufferDisabled == true.  I think for zero-size stream the condition offsetInBuffer <= mFillPoint will pass for disabled buffering.

r=honzab with the comment above

::: netwerk/test/unit/test_filestreams.js
@@ +251,5 @@
> +
> +  // OK, now seek back to start
> +  buffered.seek(Ci.nsISeekableStream.NS_SEEK_SET, 0);
> +
> +  // Now check that available() does not throw throw

s/throw throw/throw/

Comment 39

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated to comments

Good catch on mBufferDisabled.  Added a test for that, which did fail with the original patch, but that needs nsIStreamBufferAccess to be scriptable, which I think is fine anyway.  Going to get sr from bsmedberg on that part.

Comment 40

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 567144 [details] [diff] [review]
Updated to comments

Benajmin, could you just sr the one idl change?

Comment 41

[Benjamin Smedberg]

Comment on attachment 567144 [details] [diff] [review]
Updated to comments

Does [notxpcom] in nsIStreamBufferAccess *imply* [noscript], or do you have to add it explicitly to those methods?

Comment 42

[Boris Zbarsky [:bzbarsky]]

I think it does at least for xpconnect, but I added it explicitly just to be sure.

Comment 43

[Honza Bambas (:mayhemer)]

Comment on attachment 567144 [details] [diff] [review]
Updated to comments

Review of attachment 567144 [details] [diff] [review]:
-----------------------------------------------------------------

You may want to change all places to use bool/true/false consistently, PRBool -> bool is going to happen in few minutes from now.

And I forgot about one more place: not sure what the behavior should be when someone calls SetEOF on the stream.  It looks like some input streams implement that method too.  Maybe if call to the unbuffered stream's method succeeded set mEOF=true too?

Comment 44

[Boris Zbarsky [:bzbarsky]]

Attachment: With those changes

Comment 45

[Honza Bambas (:mayhemer)]

Comment on attachment 567464 [details] [diff] [review]
With those changes

Review of attachment 567464 [details] [diff] [review]:
-----------------------------------------------------------------

r=honzab

Thanks.

Comment 46

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7fc2fc28c0ec

Comment 47

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/7fc2fc28c0ec