ProgerXP / FileDrop

Self-contained cross-browser pure JavaScript class for Drag & Drop and AJAX (multi) file upload.
filedropjs.org
The Unlicense
264 stars 61 forks source link

File Content-Type #14

Closed rafaelmaiolla closed 10 years ago

rafaelmaiolla commented 10 years ago

Is there a reason to override the Content-type when uploading the file? self.xhr.setRequestHeader('Content-Type', 'application/octet-stream')

And in addition to that, why you prefer to send the raw data? e.target.result

Those two piece of code doesn't seem to work properly when you upload a PNG file (probably other binary files too). Why not just sending the nativeFile and let the XMLHTTPRequest handle the Content-Type? The way it is now is making all file unused as the server doesn't know the mime-type to save it properly...

ProgerXP commented 10 years ago

Is there a reason to override the Content-type when uploading the file?

No specific reason except that it should be better to prevent browsers from messing with the content trying to "convert encoding" or something of that kind.

What sort of problem you're running in when uploading PNGs?

as the server doesn't know the mime-type to save it properly...

You can always use xhrSetup to set/unset headers as you need.

e.target.result is needed for Safari 5.

rafaelmaiolla commented 10 years ago

I'm saving the file content using the following code:

$fp = fopen($attachments_path . urldecode($_SERVER['HTTP_X_FILE_NAME']), 'w');
fwrite($fp, file_get_contents('php://input'));

The file seems to have the proper size, but it cannot be open. It is simply invalid. Something seems to be messing with the file.

ProgerXP commented 10 years ago

Are you sure it's due to Content-Type? This would be strange because binary data is preserved as is no matter what. Have you checked the data being actually transmitted? This is easily done with the Network tab of CHrome/Firefox web dev tools. See if demo works fine and you can upload pictures there (compare MD5 and CRC32, they are provided for this exact reason).

Also, doing fopen() like that is a terrific security whole. What if I upload a .php file? No checks.

And yet another thing is that you should be still looking for $_FILES (see demo's upload.php).

rafaelmaiolla commented 10 years ago

Thanks for your help, but the problem seems to be in my code.

Another library is replacing the Array.prototype.map and that is causing an exception when converting the file to binary. I don't know if I can prevent the other library to replace that, so I will need to add a work-around for that on Chrome.

ProgerXP commented 10 years ago

Thanks for your help, but the problem seems to be in my code.

That's what I supposed to be. Always check FileDrop's demo first to find out if FileDrop is actually the cause of the problem. If map cannot be retained then you can temporary restore it in xhrSend - push a handler in front with preview to restore to browser's default map and push a handler at the end to revert it to whatever the library overrides it (but that's a bad concept anyway).

I am closing this now then.

rafaelmaiolla commented 10 years ago

I didn't see another way to fix that issue, so your "bad concept" worked for me

And in addition to that, now I'm getting the following:

ArrayBuffer is deprecated in XMLHttpRequest.send(). Use ArrayBufferView instead.

Are you planing to update the library to use ArrayBufferView?

ProgerXP commented 10 years ago

I didn't see another way to fix that issue, so your "bad concept" worked for me

That's the power of event-based approach :)

Are you planing to update the library to use ArrayBufferView?

Yep. Somebody else has already asked me about this too. Last update was dedicated to Chrome directory upload (#7). Next update is planned to fix binary reader methods.

rafaelmaiolla commented 10 years ago

Ok, thanks!