Skip to content

Fixed invalid json response format on error. #634

Merged
merged 2 commits into from Jan 25, 2013

3 participants

@unluckypixie

If there is a problem with the file handling the resulting error messages are output corrupting the JSON response sent to the browser I have added "@" to suppress these messages so the response from the PHP is valid JSON.

I have also added a Json mime-type header to the output as it's good practice.

Dave Fennell added some commits Oct 18, 2012
Dave Fennell Suppressed error messages from fopen and fclose so that return format…
… is valid json and added json mime type to headers.
64ee24f
Dave Fennell Added correct HTTP response codes from upload.php so that error handl…
…ing in javascript is triggered.
df33f6b
@MaxKorz

it won't work with ie9 using ajax - there will be double request

Perhaps I have missed something but all I have done is corrected the code to behave how it was obviously originally intended to work. If this causes an issue with IE9 then I would suggest that issue is in the Javascript not in this PHP or the PHP has other faults which I have highlighted with this change, either way this change should go in and the other fault should be addressed wherever it is.

I'm sorry. My bad. It's not about ajax, but there's still a bug in IE. Check it in Google Chrome Developer Console or in Firebug, using custom.html and original js files, try to upload a file (in html4 runtime, html5 won't work in ie9). It will cause two requests: http://s47.radikal.ru/i116/1210/3b/fb837cd20a02.png
IE9 (perhaps 6-9) will try to download second request as file: http://s019.radikal.ru/i640/1210/2a/1371cd055931.png if Content-Type is application/json
If we won't set Content-Type header, it will be "text/html" by default. Any (I think) javascript framework can process that request as json (jQuery can). All browsers, except IE, work fine with not correct Content-Type header, but IE breaks with the correct one

It is Internet Explorer bug

@jayarjo
Moxiecode member
jayarjo commented Jan 25, 2013

I'm going to merge this in. However header("Content-Type: application/json"); will have to be removed, as MaxKorz said it's problematic in old IEs.

One could probably play with Accept header, to get predictive behavior. However one important point here - upload.php is really just an example, you are free to alter it however you like or completely replace it with your own. Doesn't have to be so complex in most cases anyway.

@jayarjo jayarjo merged commit f543a24 into moxiecode:master Jan 25, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.