Closed Bug 413974 Opened 16 years ago Closed 16 years ago

[FIX]Firefox appends charset to Content-Type: multipart/form-data in XmlHttpRequest, PHP can't parse it

Categories

(Core :: DOM: Core & HTML, defect, P2)

x86
Other
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: michal-mozbug, Assigned: bzbarsky)

References

Details

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; cs; rv:1.9b2) Gecko/2007121016 Firefox/3.0b2
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; cs; rv:1.9b2) Gecko/2007121016 Firefox/3.0b2

Firefox 3.0b2 appends charset to Content-Type header in XmlHttpRequest even if content type is multipart/form-data. Header in form

Content-Type: multipart/form-data; boundary=-----------------------------AaB03x; charset=UTF-8

is not successfully parsed by PHP (at least in version 5.2) which makes some applications broken with Firefox 3.0b2 (upload plugin for TiddlyWiki, for example).

Temporary solution is to force charset field in Content-Type header before boundary - than PHP can parse this header. It may be bug in PHP request parser (but does it make sense to have charset in Content-Type: multipart/form-data?), but there is probably no chance that all PHP installations will be fixed in near future so it's probably easier to workaround it in Firefox?

There is no such problem in Firefox 2.x.

Example page and PHP script:

============ HTML
<html>
    <head>
        <script type="text/javascript">
            function upload(withCharset) {
                var xmlhttp = (window.XMLHttpRequest ? new XMLHttpRequest : (window.ActiveXObject ? new ActiveXObject("Microsoft.XMLHTTP") : false));
                if (!xmlhttp) {
                    return false;
                }
                xmlhttp.open("POST", "http://localhost/test.php");
                xmlhttp.onreadystatechange = function() {
                    if (xmlhttp.readyState == 4) {
                        alert(xmlhttp.responseText);
                    }
                };
                var boundary = "-----------------------------AaB03x";
                if (withCharset) {
                    xmlhttp.setRequestHeader("Content-Type", "multipart/form-data; charset=UTF-8; boundary="
                            + boundary);
                } else {
                    xmlhttp.setRequestHeader("Content-Type", "multipart/form-data; boundary="
                            + boundary);
                }

                var fileContent = "xxxx";
                var content = "--" + boundary + "\r\n";
                content += "Content-disposition: form-data; name=\"userfile\"; filename=\"file.txt\"\r\n";
                content += "Content-Type: text/plain; charset=UTF8\r\n";
                content += "Content-Length: " + fileContent.length + "\r\n\r\n";
                content += fileContent;
                content += "\r\n--" + boundary + "--\r\n";

                xmlhttp.send(content);
                return true;
            }
        </script>
    </head>

    <body>
        <a href="#" onclick="upload(0); return false;">upload file with AJAX</a><br />
        <a href="#" onclick="upload(1); return false;">upload file with AJAX (with charset)</a>
    </body>

</html>

============= PHP script

<? var_dump($_FILES); ?>

Reproducible: Always

Steps to Reproduce:
1. create PHP script with content noted above and place it to website with PHP support
2. create HTML page with content noted above and change URL used in xmlhttp.open to URL of PHP script
3. open HTML page in Firefox 3.0b2
Actual Results:  
click on first link: alert will display "array(0) {}"

click on second link: alert will display something like this:
array(1) {
  ["userfile"]=>
  array(5) {
    ["name"]=>
    string(8) "file.txt"
    ["type"]=>
    string(10) "text/plain"
    ["tmp_name"]=>
    string(14) "/tmp/phpACkzbd"
    ["error"]=>
    int(0)
    ["size"]=>
    int(4)
  }
}


Expected Results:  
click on both links should display something like this:
array(1) {
  ["userfile"]=>
  array(5) {
    ["name"]=>
    string(8) "file.txt"
    ["type"]=>
    string(10) "text/plain"
    ["tmp_name"]=>
    string(14) "/tmp/phpACkzbd"
    ["error"]=>
    int(0)
    ["size"]=>
    int(4)
  }
}
Component: General → DOM: Mozilla Extensions
Product: Firefox → Core
QA Contact: general → general
Version: unspecified → Trunk
So the problem is that if the charset comes before the boundary param PHP is OK, and if it comes after the boundary param PHP shows its "MIME spec, what MIME spec?" colors once more?

I guess we can try to move the charset param around... blizzard, shaver sez you can follow this up with the PHP folks?
Status: UNCONFIRMED → NEW
Ever confirmed: true
So one option here is to have extractCharsetFromContentType set the charset position to right after the winning type if that winning type has no charset param.  That would guarantee that the charset= comes before the boundary=.  Christian, what do you think of that approach?
Flags: blocking1.9?
Moving to blocking p1 because of web compat
Flags: blocking1.9? → blocking1.9+
Priority: -- → P1
Assigning to Jonas.
Assignee: nobody → jonas
The solution in comment 2 sounds like a good one FWIW.

Why can't people just parse their MIME correctly :(
It seems that PHP parses headers according to obsoleted RFC 1867 where comma is mentioned as separator (section 6, Examples). Parsing of header is not too smart - it looks for substring "boundary" and then try to trim everything from next comma to end of line. This is the reason, why charset before boundary doesn't bother.
Uh... the examples section there is non-normative, and doesn't actually follow the MIME RFCs relevant at the time...  I'd certainly hope no one wrote a parser based on the examples section.  :(  Clearly I hope in vain....
Another, possibly simpler and maybe more correct, solution is to just not append a charset at all for multipart/form-data, since the type isn't actually defined to have such a parameter.  It's the responsibility of the data creator to set the charset on each part individually...

Ideally there would be a simple way to tell from the type whether it wants a charset param, but there isn't.  So we'd have to just hardcode the "multipart/form-data".
I managed to solve it by simply adding the charset myself before the boundary... it seems that php was taking the charset as part of the boundary string

the below should fix the trick

http_request.setRequestHeader("Content-Type", "multipart/form-data; charset=utf-8; boundary=---------------------------" + border);
Please not that the duplicate (and closed) Bug 416178 is more general: it affects normal XHR requests of form posts that have the standard application/x-www-form-urlencoded encoding. The appended charset= breaks many webservers and is thus considered a bad-thing-on-the-web(tm), although the HTTP spec allows it.

There is *no* workaround for it and this bug will break some Ajax apps out there.
Jonas, any progress on this one?
Smaug: You have the cycles for this one?
Assignee: jonas → Olli.Pettay
Attached patch Fix (obsolete) — Splinter Review
Assignee: Olli.Pettay → bzbarsky
Status: NEW → ASSIGNED
Attachment #304679 - Flags: superreview?(jonas)
Attachment #304679 - Flags: review?(cbiesinger)
Summary: Firefox appends charset to Content-Type: multipart/form-data in XmlHttpRequest, PHP can't parse it → [FIX]Firefox appends charset to Content-Type: multipart/form-data in XmlHttpRequest, PHP can't parse it
Attachment #304679 - Attachment is obsolete: true
Attachment #304680 - Flags: superreview?(jonas)
Attachment #304680 - Flags: review?(cbiesinger)
Attachment #304679 - Flags: superreview?(jonas)
Attachment #304679 - Flags: review?(cbiesinger)
Comment on attachment 304680 [details] [diff] [review]
Same without the extra test in the makefile

Looks good. Feel free to land, but it wouldn't hurt to get biesis input too at some point.
Attachment #304680 - Flags: superreview?(jonas)
Attachment #304680 - Flags: superreview+
Attachment #304680 - Flags: review+
The network stuff really needs review from biesi before I'd be comfortable landing it.
+                charsetStart =  flatStr.Length();

should have just one space
Attachment #304680 - Flags: review?(cbiesinger) → review+
Comment on attachment 304680 [details] [diff] [review]
Same without the extra test in the makefile

Requesting approval.  I think we want to fix this for the beta...  Risk should be pretty low, win is reasonably big.
Attachment #304680 - Flags: approval1.9b4?
Comment on attachment 304680 [details] [diff] [review]
Same without the extra test in the makefile

a1.9b4=beltzner
Attachment #304680 - Flags: approval1.9b4? → approval1.9b4+
Flags: tracking1.9+ → blocking1.9+
Keywords: checkin-needed
Priority: P1 → P2
Keywords: checkin-needed
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Does not fix bug 416178.
Component: DOM: Mozilla Extensions → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: