Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request object fails to return the parameters completely when we make a POST method, and spoof it as a GET request using X-HTTP-METHOD-OVERRIDE or _method parameter. #15348

Closed
bs-thomas opened this issue Sep 8, 2016 · 5 comments

Comments

@bs-thomas
Copy link
Contributor

  • Laravel Version: 5.2.31
  • PHP Version: 5.6
  • Database Driver & Version: N/A

Description:

Request object fails to return the parameters completely when we make a POST method, and spoof it as a GET request using X-HTTP-METHOD-OVERRIDE or _method parameter.

It will simply return a blank response no matter what call you use to obtain. The reason is because the parameter was actually gone in the new instance aftering cloning itself inside "createFromBase()". It was overwritten by "$request->request = $request->getInputSource();".

NOTE: Spoofing it as a PUT or DELETE parameter works, because coincidently we happen to use the same call to obtain the parameters ($request->request). Only GET uses $request->query to obtain the parameters.

I noticed that the problem is because getInputSource() uses "$this->getMethod()" to judge if we should obtain the parameter using $this->query or $this->request. This is incorrect because getMethod() could return a "spoofed HTTP method", which is not the real one. What you have to do is to replace that with "strtoupper($this->server->get('REQUEST_METHOD', 'GET')) == 'GET'". I copied this off from Symfony's request library, doing a similar type of checking.

I'll make this change, and do a pull request. Please kindly look into it, and merge if it looks okay, as our project will depend on this change.

Steps To Reproduce:

(1) Create a POST form, with _method = GET, and a parameter test_param = 12345
(2) Make an AJAX request to the server, and try to retrieve the body parameters. You will notice it is not possible.

Thank you!

@GrahamCampbell
Copy link
Member

I don't think there's a bug here.

@bs-thomas
Copy link
Contributor Author

Hello Graham, thanks for the quick response. Actually I did spend some time looking at this library and symfonys.

Symfony actually handles this correctly without problems because it doesn't have the createFromBase() method when initializing. That is where the problem is occurring actually, because it makes a "clone" in that function, which the "request()" part gets overwritten by a call to getInputSource().

You can actually try my case scenario and you will see why. Do a dd() inside createFromBase(), and you will notice how data was mistakenly replaced.

I'm outside now so I can't show the line number. Would it help if I find the line number for you later?

Thanks so much for your time to look into this Graham. Looking forward to your response!

Cheers,
Thomas

@bs-thomas
Copy link
Contributor Author

Hello Graham,

Everytime laravel loads, createFromBase() in Request.php:830 gets called to create an Illuminate instance from a Symfony instance. In this method, it makes use of the following:

$request = (new static)->duplicate() in line 838.

After duplicating, this is still okay, and the parameters are inside the new instance, hidden inside $request->request.

But then, in line 847, a call to getInputSource() overwrites $request->request.

$request->request = $request->getInputSource();

In the method getInputSource(), it makes a check by using getMethod() in Request:668, and if the method is a GET, it will obtain from the query string. However, when you call getMethod(), this is actually the "spoofed request", and not the "real request". So if I spoofed a real POST request into a GET request, getMethod() actually returns GET, which would be most likely irrelevant. But it is supposed to return the Form Body because that's where the entered information is stored.

This is where the problem occurs, and this is a method in Laravel only, and not inside Symfony. That's why Symfony doesn't have this problem.

Please note, the problem is not related to the request override itself and Symfony actually handles this for us without problems. However, in Laravel, it has incorrectly assigned request to the wrong parameterBag.

You may ask why I would want to send a real POST, and override it as a GET request. Reason is actually quite simple, because GET request has a very strict length limitation, and POST is much looser, so for long GET requests in a RESTful API, a common practice is to spoof a POST into a GET. Google's API does this too, and here's a good post about it:
http://stackoverflow.com/questions/19771031/rest-request-cannot-be-encoded-for-get-url-too-long

Our team currently depends on this change, so we would really appreciate if you could help us look into this deeper, or if possible reopen this thread so more people can join the conversation.

Thank you very much again! Looking forward to your update soon.

@bs-thomas
Copy link
Contributor Author

Sorry for the non-stop messages.

Perhaps my issue topic is misleading? I tried to express the "end problem" and not the reason to the problem in the topic. Or if there is a better title for this, please do change that.

Thanks!

@bs-thomas
Copy link
Contributor Author

@GrahamCampbell
Hello, apologies again for the non-stop messages. May I ask if my latest pull request makes more sense? I'm unsure if the issue is still being looked at. If it is, would like to ask when it could be merged over to 5.3. If it's not going to be looked at, then I may have to find alternatives. Thank you very much for your kind attention!

taylorotwell pushed a commit that referenced this issue Sep 16, 2016
…nd spoof it as a GET request using X-HTTP-METHOD-OVERRIDE or _method parameter (#15410)

Fix issue where request object fails to return parameters when we make a POST method, and spoof it as a GET request using X-HTTP-METHOD-OVERRIDE or _method parameter

#15348
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants