This sounds very similar to MDL-26705.

Could you please have a look at the advice given in that issue? Let me know if the issue is the same for you and if any of the information there helps.

Kassim

Could you please confirm whether:

• this is still an issue that you're facing; and
• whether it is the same issueas discussed in MDL-26705.

We're using reverse proxying here without issue on Moodle 2.3, though we're using nginx and haproxy rather than Apache with mod_proxy, and we aren't facing these issues.

Cheers,

Andrew

Hi Kassim,

I'm going to close this issue as we're unable to replicate it. If you still believe this to be an issue with a supported version of Moodle, could you please re-open the issue.

If you do need to re-open the issue, if you could give us any additional details as to the errors you're seeing and an idea as to your setup, that would be most appreciated.

Thanks for taking the time to report this issue and helping to make Moodle better

Andrew

I would like to request that this be revisited. Our institution uses Juniper's SSL-VPN which implements an extensive content rewriting engine to allow users to access Moodle externally. This causes the dreaded "Invalid JSON String" error when a file is uploaded. I found that changing line 60 of /repository/repository_ajax.php (Moodle 2.2.6+) to "@header('Content-type: application/json;');" will fix this issue. I feel that any JSON web service should be using this header rather than text/html.

If text/html is used, Juniper's appliance transforms the HTTP response from:

to:

<SCRIPT language="javascript" id="dsshim" src="/dana-cached/js/shimdata.cgi"></SCRIPT>
<SCRIPT language="javascript" id="dsfunc" src="/dana-cached/js/oth.js"></SCRIPT>
<SCRIPT language="javascript" id="dstimeout" src="/dana-cached/js/sessiontimeout.js"></SCRIPT>
<SCRIPT language="javascript" id="dsvar" >
//<![CDATA[
var dsnodocwrites = 0 ; var DanaCookie="MoodleSessionsandbox=jioalrm840r02e93s02d4ok811; MoodleSessionsandbox20=huvifijfdv4q6kaiguebgsesk1; MOODLEID1_=%2500%25FAI%2517%255B%25C7"; var DSHost="remote.lsuhsc.edu"; var DSObfuscateHostname=0;var DSTBSettings=16449;var DSTBLU='/dana/home/starter.cgi?startpageonly=1';;danaSetDSHost();
//]]>
</SCRIPT>

</noscript>
<SCRIPT language="javascript" id="dstb-id">
//<![CDATA[
setTimeout("dstb()",0);
//]]>
</SCRIPT>

If "application/json" is used (which is the correct mime type IMHO), no rewriting is performed.

Thanks for that additional information, Chris.

Andrew: this might be worth looking at.

I wonder whether we should perhaps set the content-type for all scripts defined AJAX_SCRIPT to application/json which would solve this across the board.

We'd need to take out the explicit overwriting of this (e.g. in repository/repository_ajax.php which sets it to text/html).

Petr,

I'm adding you as a watcher as I suspect that you're probably he best person to offer insight in this.

I guess we would need to modify lib/setup.php to check whether AJAX_SCRIPT is true and set the header if so. I think that application/json is probably the most appropriate header as all of our AJAX scripts should respond with either some kind of RESTful output or JSON but I may be wrong...

Adding Jerome too as he's most knowledgeable about web services and this /could/ affect him in some way too (either as a similar issue or related).

Actually, it turns out that whenever AJAX_SCRIPT is set, we use the core_renderer_ajax renderer, which sets the content type to application/json, except for file uploads:

From lib/outputrenderers.php::core_renderer_ajax:

    public function header() {

        // unfortunately YUI iframe upload does not support application/json

        if (!empty($_FILES)) {

            @header('Content-type: text/plain; charset=utf-8');

        } else {

            error_log("Setting application/json");

            @header('Content-type: application/json; charset=utf-8');

        }    

So I believe we just need to remove the relevant places where the content-type is forced.

So far I can only see one place where we set the Content-type for AJAX scripts, and that's in repository_ajax.php.

/user/selector/search.php doesn't define itself as an AJAX_SCRIPT though and although it does have the correct content-type set, if it returns an error it will be an HTML error rather than JSON.

I'll provide backport branches for STABLE branches after a successful peer review.

Hi, if I remember it correctly there was a problem with json mimetype in some browsers when uploading files or in file picker in general. Please do not change anything in stable branches because there is a high risk of regression. This needs to be tested in all imaginable browsers, servers and configurations...

Hi Petr,

I'm hoping that this will still be handled by http://git.moodle.org/gw?p=moodle.git;a=blob;f=lib/outputrenderers.php;h=4e0545b9ce2897a8de3f0f5f9fbfd0e1cc804f23;hb=HEAD#l3065 which forces the mimetype to text/plain for file uploads.

Andrew

I notice that in user/selector/search.php there is still this in the other branch of the if
header('Content-type: text/plain; charset=UTF-8');
Does that header call trump the previous $OUTPUT->header();? Aternatively does echo $OUTPUT->header(); need to be at line 42?

To me it looks like you'll need to modify your testing instructions as the user search will only be application/json if the tester is not in developer debug mode (which they almost certainly will be).

[Y] Syntax
[Y] Output
[Y] Whitespace
[NA] Language
[NA] Databases
[?] Testing
[Y] Security
[NA] Documentation
[Y] Git
[Y] Sanity check

Otherwise it seems to make sense.

Hmm, I think it's probably best to remove that debugging option rather than return two content-type headers as the

I've added Tim Hunt here as he originally added that option and may have some additional views.

Note, that this is related in part to MDL-38287.

I think the reason for text/html when debugging was so that when an error occurred, it was easy to see the error message in the browser. (Note that this is when I was developing the user selector in 2008. I'm not sure it is still needed.)

Ah cool. I'll remove it then. Thanks!

I've updated this branch and removed the debugging calls in user/selector/search.php.

I've also added error handling to the JavaScript so I'd appreciate a quick PR to ensure I've not missed anything in my tidy up.

Cheers,
A

I've not seen the use of M.core.ajaxException before. I can only assume that is why we now need moodle-core-notification. (I have read your recent JS docs, and this was still news to me. Perhaps you should cover error handling there.)

So, no problems withe the code, +1 from me.

[Y] Syntax
[Y] Output
[Y] Whitespace
[NA] Language
[NA] Databases
[Y] Testing
[Y] Security
[NA] Documentation
[Y] Git
[Y] Sanity check

Oh! stupid workflow state. Of course, Andrew D may still want to peer review this himself.

Ah - thanks for the suggestion. I'll make a note to cover that.

I was also wondering about creating a moodle YUI module to handle IO which wraps the error handling somehow. Haven't thought about how that would work yet though. Will also be writing API docs for moodle-core-notification soon with the hope that we can generate the docs and publish somewhere.

Oh and yes - M.core.ajaxException and M.core.exception are both provided by moodle-core-notification. I'd prefer to do some lazy-loading there to reduce dependencies, but at present it's included on that page already for something else and I need to check how lazy loading will work in a try/catch scenario.

Rebased on weekly master and submitting for integration.

Crossing fingers... integrated (master only), thanks!

Everything working as described

This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads).

Thanks for your contributions!

I think we've introduced a regression here, apparently not all browsers accept application/json mimetype. IE wants to be different again - MDL-39810 (or maybe not)