393968 - [FIX]XMLHttpRequest error status 500 on POST multipart form-data

Status: RESOLVED FIXED

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

Description

[Laurent Fournier]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007082705 Minefield/3.0a8pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007082705 Minefield/3.0a8pre

AJAX request to upload files does not work, and use to work with FF2
The pb occurs only with multipart type request, normal POST request works. 

See the Python CGI bellow to produce the error 

Reproducible: Always

Steps to Reproduce:
run the python CGI

Actual Results:  
http error 500

Expected Results:  
status 200

#!/usr/bin/python

import cgi

class ajax:
    def __init__(self):
        self.fo = cgi.FieldStorage()
        print 'Content-type: application/vnd.mozilla.xul+xml\n'
        print '<?xml version="1.0" encoding="ISO-8859-1"?>'
        if self.fo.has_key('a'):
            print '<a>OK</a>'
        else:
            print '<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" onload="run()">'
            self.js()
            print '<description>Test</description></window>'

    def js(self):
        print """<script language="javascript">
<![CDATA[

function run() {
  var msg = '\\n--AaB03x\\ncontent-disposition: form-data; name="a"; filename="z"\\n'
  + 'Content-Type: application/octet-stream\\n'
  + 'Content-Transfer-Encoding: binary\\n\\n'
  + 'AAAAA_content_AAAAA'
  + '\\n--AaB03x\\n';

  var a1 = new ajax(msg,function(res) { alert((new XMLSerializer()).serializeToString(res));});
  a1.doGet();
}

function ajax(params, cb) {
  var req = new XMLHttpRequest();
  req.onreadystatechange = processRequest;
  function processRequest () {
    if (req.readyState == 4) {
      if (req.status == 200) {
	if (cb) cb(req.responseXML);
      } else {
	alert('Error status:'+ req.status);
      }
    }
  }
  this.doGet = function() {
    req.open('POST', 'tst_ajax.py', true);
    req.setRequestHeader('Content-Type', 'multipart/form-data; boundary="AaB03x"');
    req.setRequestHeader('Content-length', params.length);
    req.setRequestHeader('Connection', 'close');
    req.send(params);
  }  
}
]]>
</script>
"""

if __name__ == '__main__':
    ajax()

Comment 1

[Laurent Fournier]

Bug still occurs on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007090205 Minefield/3.0a8pre

Comment 2

[Laurent Fournier]

The regression is still there with the Minefield release
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007090504 Minefield/3.0a8pre

All Web sites using Ajax Uploading with FF2 may not work with FF3. I do not know if it concerns only XUL app or all HTML apps.

Please look at the bug.

Comment 3

[Laurent Fournier]

I found a rational

ON FF3M8 the "boundary" field is not sent ! Why, I don't know

POST /tramwaytool/tstajax.py HTTP/1.1
Host: faugeres.france.rockwellcollins.com:8008
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007090604 Minefield/3.0a8pre
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-us,en;q=0.5
Accept-Encoding: gzip,deflate
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive: 300
Proxy-Connection: keep-alive
Content-Type: multipart/form-data;charset=UTF-8
Referer: http://faugeres.france.rockwellcollins.com:8008/tramwaytool/tstajax.py
Content-Length: 152
Cookie: MODULE=SAAB2000_AEWC; REPOSITORY=%2Fcvs%2FDT313%2FRTU; SERVER=cvsdt
Authorization: Basic bGZvdXJuaWU6YXF0aHJvdCwy
Pragma: no-cache
Cache-Control: no-cache

ON FF2 IT WORKS well

POST /tramwaytool/tstajax.py HTTP/1.1
Host: faugeres.france.rockwellcollins.com:8008
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6
Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5
Accept-Language: fr,fr-fr;q=0.8,en-us;q=0.5,en;q=0.3
Accept-Encoding: gzip,deflate
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive: 300
Proxy-Connection: keep-alive
Content-Type: multipart/form-data; boundary="xx"
Connection: close
Referer: http://faugeres.france.rockwellcollins.com:8008/tramwaytool/tstajax.py
Content-Length: 152
Cookie: REPOSITORY=%2Fcvs%2FDT313%2FRTU; SERVER=cvsdt; MODULE=SAAB2000_AEWC
Authorization: Basic bGZvdXJuaWU6YXF0aHJvdCwy
Pragma: no-cache
Cache-Control: no-cache

Comment 4

[Laurent Fournier]

Please take 5' to look at this bug. Application using binary file Upload may not work with FF3, because the boundary field is not sent.

Comment 5

[Ellis Lau]

identified the same problem with an independent project.

After a call to setRequestHeader("Content-Type","multipart/form-data; boundary=abcdefg"), in FF 2 the header shows up correctly as 
"multipart/form-data; boundary=abcdefg"

but in FF 3.0a8 the header shows up as
"multipart/form-data;charset=UTF-8"

Comment 6

[Gordon Tran]

This appears to be a regression caused by the patch from bug 382947. The Content-Type is being overwritten without considering any existing boundary field.

Comment 7

[Sebastian Winkler]

The same behaviour can be observed on the Mac in 3a builds too.
In FF2 on the Mac the request doesn't even get dispatched and returns status 400 'Bad Request'

Comment 8

[Sebastian Winkler]

Forget that about multipart/form-data not working at all on the Mac in FF2.
During testing with FF3a I must have corrected an error that caused said behavior. (I think I had the wrong EOL-delimiter: \n instead of \r\n.)

Comment 9

[marko dudic]

Is there a plan to fix this bug, because with version 1.9a9pre upload of files don't work any more.

Comment 10

[Boris Zbarsky [:bzbarsky]]

I have no idea why this got assigned to me (I didn't cause the bug, and I don't recall Brian being my manager), or for that matter why it's stuck in the wrong product, without the dependencies it should have...

Comment 11

[Boris Zbarsky [:bzbarsky]]

We really do need to fix this, though.

biesi, is there a necko helper function we could use here?  We basically want to replace the value of a single parameter in a header with a new value...

Comment 12

[Brian King [:kinger]]

(In reply to comment #10)
> I have no idea why this got assigned to me (I didn't cause the bug, and I don't
> recall Brian being my manager), or for that matter why it's stuck in the wrong
> product, without the dependencies it should have...

Yes, apologies.. completely my fault. I should have triaged it correctly. It seemed like a serious regression and you came to mind first.

Comment 13

[Boris Zbarsky [:bzbarsky]]

One thing that comes to mind is to have nsINetUtil.parseContentType output not only the charset and whether there was a charset, but also where in the header string the charset appeared.  Then in this code we could just do a simple replace if there is already a charset, and an append otherwise.  We'd need to change net_ParseContentType too, but that shouldn't be too bad.

biesi, would you be OK with that sort of change?

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

Like additional parameters "out unsigned long charsetStartIndex, out unsigned long charsetEndIndex"? Fine with me, though perhaps that should be a new additional function so that callers needn't care about those parameters when they don't need them.

Comment 15

[Boris Zbarsky [:bzbarsky]]

I was going to only add out unsigned long charsetStartIndex, since charsetEndIndex is charsetStartIndex + aCharset.Length().

And would you just want a new function on nsINetUtil, or additional signatures for NS_ParseContentType and net_ParseContentType?

Comment 16

[Boris Zbarsky [:bzbarsky]]

My gut instinct, to be honest, is to change the signature in nsINetUtil and add new signatures for NS_ParseContentType and net_ParseContentType.

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #15)
> I was going to only add out unsigned long charsetStartIndex, since
> charsetEndIndex is charsetStartIndex + aCharset.Length().

I hadn't realized that you want this to be just the value of the parameter. Why do you want that? If you use start/end and make it refer to the entire ";charset=foo" string, you can also just replace that with ";charset=bar", and also allow callers to just remove the charset parameter if they want that.

(In reply to comment #16)
> My gut instinct, to be honest, is to change the signature in nsINetUtil and add
> new signatures for NS_ParseContentType and net_ParseContentType.

fine with me

Comment 18

[Boris Zbarsky [:bzbarsky]]

Attachment: Possible patch

Just checkpointing.  Still need to write tests and such.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

I discovered that I wanted different enough behavior in terms of the hadCharset boolean that a separate function makes more sense.  Also prevents me breaking existing code...

That said, maybe we want to return the start/end indices at end of string (and hadCharset == true) in the new function in the circumstance where we now set hadCharset to false because net_ParseContentType returns an index of -1?

Comment 20

[Boris Zbarsky [:bzbarsky]]

That includes the fix for bug 397234, by the way.

Comment 22

[Boris Zbarsky [:bzbarsky]]

Fix and tests checked in.  Laurent, thanks for hunting down exactly what the problem was!

Comment 23

[Laurent Fournier]

Boris and others,

Yes, it works!
Thank you very much. We really needed this bug to be fixed because we are developing an OpenSource XUL application using tree cell editing and file upload...only with FF3.

Thanks again,

Laurent