Firefox 45 does not send content-type in empty input[type=file] anymore

RESOLVED FIXED

Status

()

RESOLVED FIXED
3 years ago
14 days ago

People

(Reporter: gokhun, Assigned: baku)

Tracking

(4 keywords)

45 Branch
dev-doc-complete, regression, site-compat, testcase
Points:
---

Firefox Tracking Flags

(firefox45blocking fixed, firefox46 unaffected, firefox47 unaffected, firefox48 unaffected, firefox-esr4545+ fixed, relnote-firefox 45+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160304114926

Steps to reproduce:

I sent post request via jquery.ajax with FormData as data attribute. There was an empty <input type="file">. The issue occurs only when the file input is empty.


Actual results:

<input type="file"> is serialized without content type and filename in Firefox 45.

Content-Disposition: form-data; name="multipartFileList"

In the server side this is not recognized as a multipartFile but as an empty string.


Expected results:

<input type="file"> was serialzed with content type and empty filename in Firefox 44. (Also tested with other browsers, opera, chrome, ie11).

Content-Disposition: form-data; name="multipartFileList"; filename=""
Content-Type: application/octet-stream

Comment 1

3 years ago
Could you attach a testcase or provide a live demo, please.
Component: Untriaged → Networking
Flags: needinfo?(gokhun)
Keywords: testcase-wanted
Product: Firefox → Core
This is likely not a networking issue, since the code that sets headers for form submissions lives in places like dom/html/nsFormSubmission.cpp.
Component: Networking → DOM
This looks very related to bug 1250148 but that was only merged into FF 46, not 45.
status-firefox46: --- → ?
status-firefox47: --- → ?
status-firefox48: --- → ?
When there's a testcase, we should check if this affects all versions.
status-firefox45: --- → affected
tracking-firefox46: --- → ?
bug 1250148 was supposed to fix issues which were only in FF46 and FF47.
Or at least I wasn't aware of issues in FF45.

This sounds like something we may need to fix even in 45.x release.
baku, could you take a look?
Status: UNCONFIRMED → NEW
tracking-firefox45: --- → ?
Ever confirmed: true
Flags: needinfo?(amarchesini)
(Reporter)

Comment 6

3 years ago
Here is a live demo http://jsfiddle.net/pbp3se90/

Just press submit button after page is loaded. Then look at the network in the developer tools:

I get these:
1. Firefox 44:
1.1 without file selected 
Source
-----------------------------46964740017057619171145980668
Content-Disposition: form-data; name="myfileinput"; filename=""
Content-Type: application/octet-stream


-----------------------------46964740017057619171145980668--


1.2 with file selected:

Source
-----------------------------15226697656855963061031573977
Content-Disposition: form-data; name="myfileinput"; filename="44.html"
Content-Type: text/html

<html>
<head> .... contents of the selected file ...

2. Firefox 45
2.1 without file selected 
Source
-----------------------------18223781982535307361949975872
Content-Disposition: form-data; name="myfileinput"


-----------------------------18223781982535307361949975872--

2.2 with file selected
Source
-----------------------------1624144146134441497915738203
Content-Disposition: form-data; name="myfileinput"; filename="44.html"
Content-Type: text/html

<html>
<head> .... contents of the selected file ...
Flags: needinfo?(gokhun)
(Assignee)

Updated

3 years ago
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)

Comment 7

3 years ago
45b1 had the bug.
(Assignee)

Comment 8

3 years ago
Posted patch a.patch (obsolete) — Splinter Review
Attachment #8730129 - Flags: review?(bugs)

Updated

3 years ago
Keywords: testcase-wanted → testcase
Comment on attachment 8730129 [details] [diff] [review]
a.patch

Please add some test too, or copy a test from current beta where we hopefully have a test for this case.
If we don't have a test for this case in beta/aurora/nightly, we certainly need to add one.

This doesn't affect to FormData usage because append/set take non-null Blob, right?
Attachment #8730129 - Flags: review?(bugs) → review+
(Assignee)

Comment 10

3 years ago
Posted patch a.patchSplinter Review
Patch with test.
Attachment #8730129 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
Isn't this patch only for FF45? so approval would be needed. But if we don't get more bug report, perhaps we could wait for FF46, which doesn't have this bug, right?
But test could land on nightly too.
Which branch is this supposed to be landing on? Doesn't apply to inbound.
Keywords: checkin-needed
(Assignee)

Comment 14

3 years ago
It's for FF45.
Keywords: checkin-needed
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1245043
We don't just land patches on release branches without approval. This needs to go through the regular approval process if you want this to land on mozilla-release or esr45.
Keywords: checkin-needed
Sylvestre, you may want to have a look at this

What about 45esr? We likely need this for 45.1.0, even if we don't do a 45.0.2 release to fix this. 
We also may need to wait until next Tuesday to consider a dot release, since many people have time off for Easter on Friday and Monday.
Flags: needinfo?(sledru)
Also I'm happy to take a patch in aurora and beta as soon as possible if they are affected. Does this only affect 45?  Or does it also affect other branches?
Flags: needinfo?(amarchesini)
(Assignee)

Comment 19

3 years ago
> What about 45esr? We likely need this for 45.1.0, even if we don't do a
> 45.0.2 release to fix this. 

This patch is actually for 45esr. Aurora and beta are ok.
Flags: needinfo?(amarchesini)

Updated

3 years ago
status-firefox46: ? → unaffected
status-firefox47: ? → unaffected
status-firefox48: ? → unaffected
tracking-firefox46: ? → ---
tracking-firefox47: --- → ?
tracking-firefox48: --- → ?

Updated

3 years ago
status-firefox-esr45: --- → affected
tracking-firefox47: ? → ---
tracking-firefox48: ? → ---
tracking-firefox-esr45: --- → ?
Baku, we are probably going to do a 45.0.2, could you fill the uplift request to 45 & 45esr? Thanks
tracking-firefox45: ? → blocking
Flags: needinfo?(sledru) → needinfo?(amarchesini)
(Assignee)

Comment 21

3 years ago
Comment on attachment 8732270 [details] [diff] [review]
a.patch

Approval Request Comment
[Feature/regressing bug #]: bug 1250148
[User impact if declined]: A wrong form submission data is sent when the user doesn't select a File for an input type=file.
[Describe test coverage new/current, TreeHerder]: test included.
[Risks and why]: This patch is very similar to what we have in m-c. It seems tested enough, so I would say: no risks.
[String/UUID change made/needed]: none

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: web compatibility broken
User impact if declined:  A wrong form submission data is sent when the user doesn't select a File for an input type=file.
Fix Landed on Version: 46
Risk to taking this patch (and alternatives if risky): Read above.
String or UUID changes made by this patch: none
Flags: needinfo?(amarchesini)
Attachment #8732270 - Flags: approval-mozilla-esr45?
Attachment #8732270 - Flags: approval-mozilla-aurora?
Comment on attachment 8732270 [details] [diff] [review]
a.patch

[Triage Comment]
If you meant release.
Attachment #8732270 - Flags: approval-mozilla-release+
Attachment #8732270 - Flags: approval-mozilla-esr45?
Attachment #8732270 - Flags: approval-mozilla-esr45+
Attachment #8732270 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/releases/mozilla-esr45/rev/261d78308372e2f555b77aa79c59ed57606d70ae
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox-esr45: affected → fixed
Resolution: --- → FIXED

Comment 25

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-esr45/rev/cc9d03146a39
Landed to GECKO4501esr_2016031618_RELBRANCH
tracking-firefox-esr45: ? → 45+
Added to the release notes with "Fix a regression impacting some specific uploads (1255735)" as wording.
relnote-firefox: --- → 45+
Just found this in the release notes. No "regression" keyword? :( Need a site compat doc here.
Keywords: dev-doc-needed, regression, site-compat
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.