Last Comment Bug 684806 - Unable to upload 0KB file if an observer reads the upload stream and rewinds it
: Unable to upload 0KB file if an observer reads the upload stream and rewinds it
Status: RESOLVED FIXED
: testcase
Product: Core
Classification: Components
Component: Networking (show other bugs)
: 5 Branch
: All All
: P2 normal (vote)
: mozilla10
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-06 02:06 PDT by ssetty
Modified: 2011-10-19 03:21 PDT (History)
7 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
file upload form (498 bytes, text/plain)
2011-09-06 10:34 PDT, ssetty
no flags Details
it just says file uploaded (60 bytes, text/plain)
2011-09-06 10:36 PDT, ssetty
no flags Details
testcase using landfill.mozilla.org echo script (619 bytes, text/html)
2011-09-07 00:44 PDT, :aceman
no flags Details
screenshot of webconsole with failed upload (55.39 KB, image/png)
2011-09-07 00:52 PDT, :aceman
no flags Details
about:support (22.11 KB, text/plain)
2011-09-07 23:58 PDT, ssetty
no flags Details
config exported from noscript (14.11 KB, text/plain)
2011-09-09 04:58 PDT, :aceman
no flags Details
noscript prefs cut cut out of prefs.js (690 bytes, text/plain)
2011-09-09 04:59 PDT, :aceman
no flags Details
A minimal request observer which reads the upload stream and rewinds it (997 bytes, text/x-c)
2011-09-09 07:45 PDT, Giorgio Maone [:mao]
no flags Details
Make sure that zero-size files wrapped in buffered streams are correctly reopened by a seek after EOF. (6.99 KB, patch)
2011-10-13 20:11 PDT, Boris Zbarsky [:bz]
honzab.moz: review+
Details | Diff | Splinter Review
Updated to comments (8.97 KB, patch)
2011-10-14 11:28 PDT, Boris Zbarsky [:bz]
benjamin: superreview+
Details | Diff | Splinter Review
With those changes (11.11 KB, patch)
2011-10-17 08:49 PDT, Boris Zbarsky [:bz]
honzab.moz: review+
Details | Diff | Splinter Review

Description ssetty 2011-09-06 02:06:00 PDT
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 2011-09-06 06:27:32 PDT
Can you provide a page where this happens?
Comment 2 :aceman 2011-09-06 06:34:36 PDT
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 2011-09-06 10:08:13 PDT
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 2011-09-06 10:13:21 PDT
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 2011-09-06 10:34:31 PDT
Created attachment 558518 [details]
file upload form

this file has upload dorm
Comment 6 ssetty 2011-09-06 10:35:14 PDT
Comment on attachment 558518 [details]
file upload form

this file is file upload form
Comment 7 ssetty 2011-09-06 10:36:13 PDT
Created attachment 558519 [details]
it just says file uploaded

it just says file uploaded
Comment 8 ssetty 2011-09-06 10:38:54 PDT
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 2011-09-06 12:38:29 PDT
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) 2011-09-06 13:33:36 PDT
ssety and aceman, one of you could host those file somewhere and make a test case?
Comment 11 Boris Zbarsky [:bz] 2011-09-06 19:49:57 PDT
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 2011-09-07 00:44:52 PDT
Created attachment 558743 [details]
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 2011-09-07 00:52:57 PDT
Created attachment 558745 [details]
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 2011-09-07 01:19:36 PDT
The testcase works fine in Safe mode. Must be some extension. Sorry about that :(
Comment 15 :aceman 2011-09-07 01:20:53 PDT
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) 2011-09-07 02:01:57 PDT
I'm able to reproduce this with yesterday trunk in debug on GNU/Linux.
Comment 17 Mounir Lamouri (:mounir) 2011-09-07 02:03:57 PDT
(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) 2011-09-07 02:16:39 PDT
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 2011-09-07 02:34:48 PDT
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 [:mao] 2011-09-07 03:35:40 PDT
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 2011-09-07 23:58:00 PDT
Created attachment 559058 [details]
about:support
Comment 22 ssetty 2011-09-08 00:07:30 PDT
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 fiddlerhook@fiddler2.com 
Firebug            1.8.1    true firebug@software.joehewitt.com 
Flash Video        2.0.29   true artur.dubovoy@gmail.com 
Downloader Youtube Downloader Facebook
Flashbug           1.8.0    true flashbug@coursevector.com 
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 yslow@yahoo-inc.com 
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 refractor@developer.mozilla.org 
RapidShare         1.0      false rsDownloadHelper@yevgenyandrov.net 
DownloadHelper 
Veoh Video Compass 1.5.2    false searchrecs@veoh.com
Comment 23 :aceman 2011-09-08 00:58:03 PDT
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 2011-09-08 01:09:13 PDT
i am unable to reproduce problem in Firefox safe mode.
Comment 25 Boris Zbarsky [:bz] 2011-09-08 07:11:24 PDT
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 2011-09-08 07:15:08 PDT
okay,i need to disable extension 1 by 1. or should uninstall extensions.
Comment 27 :aceman 2011-09-08 07:25:58 PDT
Httpwatch seems disabled in the list.
ssetty: yes try to disable them.
Comment 28 Giorgio Maone [:mao] 2011-09-08 15:24:54 PDT
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 2011-09-09 00:00:55 PDT
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  cards@clav.mozdev.org
DOM Inspector               2.0.10        true  inspector@mozilla.org
IE Tab +                    2.04.20110724 true  coralietab@mozdev.org
MinimizeToTray revived (MinTrayR) 1.0     true  mintrayr@tn123.ath.cx
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  service@touchpdf.com
PrefBar                     6.0.1         true  {8A6C82A1-F6C9-481a-AAE7-C96444C9A754}
printpdf                    0.76          true  printpdf@pavlov.net
Slovníky slovenského pravopisu 2.03.2     true  sk@dictionaries.addons.mozilla.org
Web Developer                  1.1.9      true  {c45c406e-ab73-11d8-be73-000a95be3b12}
Comment 30 Giorgio Maone [:mao] 2011-09-09 02:28:59 PDT
(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 2011-09-09 04:48:39 PDT
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 2011-09-09 04:58:34 PDT
Created attachment 559424 [details]
config exported from noscript
Comment 33 :aceman 2011-09-09 04:59:18 PDT
Created attachment 559425 [details]
noscript prefs cut cut out of prefs.js
Comment 34 Giorgio Maone [:mao] 2011-09-09 07:45:27 PDT
Created attachment 559458 [details]
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 2011-09-12 04:11:37 PDT
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 [:bz] 2011-10-13 15:05:13 PDT
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 [:bz] 2011-10-13 20:11:31 PDT
Created attachment 567002 [details] [diff] [review]
Make sure that zero-size files wrapped in buffered streams are correctly reopened by a seek after EOF.
Comment 38 Honza Bambas (:mayhemer) 2011-10-14 07:15:50 PDT
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 [:bz] 2011-10-14 11:28:11 PDT
Created attachment 567144 [details] [diff] [review]
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 [:bz] 2011-10-14 11:31:15 PDT
Comment on attachment 567144 [details] [diff] [review]
Updated to comments

Benajmin, could you just sr the one idl change?
Comment 41 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-10-14 11:39:56 PDT
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 [:bz] 2011-10-14 11:45:03 PDT
I think it does at least for xpconnect, but I added it explicitly just to be sure.
Comment 43 Honza Bambas (:mayhemer) 2011-10-17 07:29:16 PDT
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 [:bz] 2011-10-17 08:49:39 PDT
Created attachment 567464 [details] [diff] [review]
With those changes
Comment 45 Honza Bambas (:mayhemer) 2011-10-17 14:19:15 PDT
Comment on attachment 567464 [details] [diff] [review]
With those changes

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

r=honzab

Thanks.
Comment 47 Marco Bonardo [::mak] (Away 6-20 Aug) 2011-10-19 03:21:00 PDT
https://hg.mozilla.org/mozilla-central/rev/7fc2fc28c0ec

Note You need to log in before you can comment on or make changes to this bug.