Peer reviewed, looks good to me, just need to format the if-else statement correctly:
https://github.com/mouneyrac/moodle/compare/master...MDL-29279#L0R94

Thanks Dongsheng. I fixed the indentation.

where is the code?

Code is back

Why are you removing only

unset($_REQUEST['restformat']);

? Why not also _GET and _POST?
Are you sure no custom external lib function ever used 'restformat' parameter? Would it be possible to use something more unique?

Hi Jerome,

I've been having a look at this now.
In regards to Petr's comments I certainly agree that restformat should be more unique, what about `wsrestformat` at a glance that looks like it would be more inline with the other vars used by the rest server. In fact would it be a better idea if that was collected by parse_request like the other params?
As for the unsetting of $_REQUEST I agree with Petr in that it would be best to unset it from post and get as well, however I see looking at the webservice code that it routinely uses and then deletes from $_REQUEST so I would be happy for that to go in as it is presently providing a new bug is created to clean that up throughout the webservice code (which may be a better idea changing that everywhere at once).

Let me know what you think - I'll leave this in the current sprint for the time being and if you clean up the naming of that string it'd get my +1.

Cheers
Sam

Hi Jerome - just reopening this issue so that when you're done with the changes you can put it back up and we will know to look at it again

I did the change Sam, you can integrate. Thanks both of you for reviewing

Integrated (master only).

I must recognize that:

• I accepted it in "blind" mode. Not sure how that works.
• I don't like having to unset those RPGC variables that way. Cannot we have some central place to ban all them following some rule before arriving to lower levels (Zend?). Or doesn't Zend provide some facility to keep out some RPGC ?

Thanks Jerome this has been tested now.
Please note that you'll need to update the test client as some point now as param restformat has of course being changed.

Cheers
Sam

Eloy:
a) server.php level wants to know about 'moodlewsrestformat'
b) authentication level wants to know about 'wsfunction' / 'wstoken'
c) Zend server read all the params
d) valid_param() checks all params are valid against the function description (so we don't want 'moodlewsrestformat' / 'wsfunction' / 'wstoken')

What are the benefits to introduce a banning rule system? Wouldn't it make the web service structure more complex? I'm not against it, I just don't know.

Sam:
it's done.

Thank you all

And this code has been spread to all Moodle git and cvs repositories. Many thanks! Closing.

Ciao