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

RESOLVED FIXED in mozilla10

Status

()

Core
Networking
P2
normal
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: ssetty, Assigned: bz)

Tracking

({testcase})

5 Branch
mozilla10
testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 6 obsolete attachments)

(Reporter)

Description

6 years ago
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

6 years ago
Can you provide a page where this happens?

Comment 2

6 years ago
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.
(Reporter)

Comment 3

6 years ago
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

6 years ago
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?
(Reporter)

Comment 5

6 years ago
Created attachment 558518 [details]
file upload form

this file has upload dorm
(Reporter)

Comment 6

6 years ago
Comment on attachment 558518 [details]
file upload form

this file is file upload form
(Reporter)

Comment 7

6 years ago
Created attachment 558519 [details]
it just says file uploaded

it just says file uploaded
(Reporter)

Comment 8

6 years ago
please find attachment fileupload.html and result.html.

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

i am using tomcat server.

Comment 9

6 years ago
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.
Status: UNCONFIRMED → NEW
Component: General → HTML: Form Submission
Ever confirmed: true
Keywords: regression, regressionwindow-wanted
OS: Windows 7 → All
Product: Firefox → Core
QA Contact: general → form-submission
Hardware: x86_64 → All
ssety and aceman, one of you could host those file somewhere and make a test case?
Keywords: testcase-wanted
(Assignee)

Comment 11

6 years ago
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

6 years ago
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.)
Attachment #558518 - Attachment is obsolete: true
Attachment #558519 - Attachment is obsolete: true

Comment 13

6 years ago
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

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

Comment 15

6 years ago
ssetty, can you post your list of extensions? You can find it in about:support. We can compare them to mine.
I'm able to reproduce this with yesterday trunk in debug on GNU/Linux.
Keywords: testcase-wanted → testcase
Version: 5 Branch → Trunk
(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.
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.
Keywords: testcase → testcase-wanted
Version: Trunk → 5 Branch

Comment 19

6 years ago
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.
Keywords: regression, regressionwindow-wanted, testcase-wanted → testcase
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).
(Reporter)

Comment 21

6 years ago
Created attachment 559058 [details]
about:support
(Reporter)

Comment 22

6 years ago
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

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

Comment 24

6 years ago
i am unable to reproduce problem in Firefox safe mode.
(Assignee)

Comment 25

6 years ago
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.
(Reporter)

Comment 26

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

Comment 27

6 years ago
Httpwatch seems disabled in the list.
ssetty: yes try to disable them.
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

6 years ago
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}
(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

6 years ago
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

6 years ago
Created attachment 559424 [details]
config exported from noscript

Comment 33

6 years ago
Created attachment 559425 [details]
noscript prefs cut cut out of prefs.js
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).
Attachment #559424 - Attachment is obsolete: true
Attachment #559425 - Attachment is obsolete: true

Updated

6 years ago
Summary: unable to upload zero kb file → Unable to upload 0KB file if an observer reads the upload stream and rewinds it
(Reporter)

Comment 35

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

Updated

6 years ago
Component: HTML: Form Submission → Networking: HTTP
QA Contact: form-submission → networking.http
(Assignee)

Comment 36

6 years ago
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.
(Assignee)

Updated

6 years ago
Assignee: nobody → bzbarsky
Component: Networking: HTTP → Networking
Priority: -- → P2
QA Contact: networking.http → networking
Whiteboard: [need review]
(Assignee)

Comment 37

6 years ago
Created attachment 567002 [details] [diff] [review]
Make sure that zero-size files wrapped in buffered streams are correctly reopened by a seek after EOF.
Attachment #567002 - Flags: review?(honzab.moz)
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/
Attachment #567002 - Flags: review?(honzab.moz) → review+
(Assignee)

Comment 39

6 years ago
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.
Attachment #567144 - Flags: review?(honzab.moz)
(Assignee)

Updated

6 years ago
Attachment #567002 - Attachment is obsolete: true
(Assignee)

Comment 40

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

Benajmin, could you just sr the one idl change?
Attachment #567144 - Flags: superreview?(benjamin)
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?
Attachment #567144 - Flags: superreview?(benjamin) → superreview+
(Assignee)

Comment 42

6 years ago
I think it does at least for xpconnect, but I added it explicitly just to be sure.
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?
(Assignee)

Comment 44

6 years ago
Created attachment 567464 [details] [diff] [review]
With those changes
Attachment #567464 - Flags: review?(honzab.moz)
(Assignee)

Updated

6 years ago
Attachment #567144 - Attachment is obsolete: true
Attachment #567144 - Flags: review?(honzab.moz)
Comment on attachment 567464 [details] [diff] [review]
With those changes

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

r=honzab

Thanks.
Attachment #567464 - Flags: review?(honzab.moz) → review+
(Assignee)

Comment 46

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fc2fc28c0ec
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla10
https://hg.mozilla.org/mozilla-central/rev/7fc2fc28c0ec
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.