FineUploader / fine-uploader

Multiple file upload plugin with image previews, drag and drop, progress bars. S3 and Azure support, image scaling, form support, chunking, resume, pause, and tons of other features.
https://fineuploader.com
MIT License
8.18k stars 1.87k forks source link

Invalid HTTP Header? #1134

Open kevinrjones opened 10 years ago

kevinrjones commented 10 years ago

The uploader sets the X-Mime-Type header from the file's type

xhr.setRequestHeader("X-Mime-Type", file.type);

however type is sometimes empty and this results in an empty header which is not HTTP compliant. Is it reasonable to add this check to the code?

        xhr.setRequestHeader("Content-Type", "application/octet-stream");
        //NOTE: return mime type in xhr works on chrome 16.0.9 firefox 11.0a2
        if (file.type !== "") {
            xhr.setRequestHeader("X-Mime-Type", file.type);
        }

Thanks

rnicholus commented 10 years ago

Yes, that seems reasonable. In fact, the X-Mime-Type should probably be removed entirely, and the Content-Type should contain the MIME type of the file (for non multipart encoded requests). For multipart encoded requests, the MIME type of the file is already provided in the header section of the file's multipart boundary.

rnicholus commented 10 years ago

I would also, personally, change if (file.type !== "") to if (file.type). That new condition will evaluate to false if file.type is null, undefined, or an empty string, plus, it's a little easier on the eyes.

kevinrjones commented 10 years ago

Would you like me to create a pull request for this, or for a quick change like this would it be simpler for you to update master?

rnicholus commented 9 years ago

When completing this task, the code should also be updated to check the custom header array first. If any user-provided headers exist, such as Content-Type, we should refrain from setting that header with some default value anywhere else. Surprisingly, XMLHttpRequest.setRequestHeader will append the passed value if the header has already been set. This is a bit non-intuitive, and I wasn't aware of this behavior until today.

rnicholus commented 9 years ago

This will definitely be part of 6.0