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

Value of $_SERVER['QUERY_STRING] different when deployed #756

Closed
DaMitchell opened this issue Sep 21, 2020 · 7 comments · Fixed by #1437
Closed

Value of $_SERVER['QUERY_STRING] different when deployed #756

DaMitchell opened this issue Sep 21, 2020 · 7 comments · Fixed by #1437
Labels

Comments

@DaMitchell
Copy link

DaMitchell commented Sep 21, 2020

Description
When making a request to /orders?billingAddress.surname=test and accessing the value of $_SERVER['QUERY_STRING'] the expected value should be:

billingAddress.surname=test

Instead the value is:

billingAddress_surname=test

When testing the issue locally using the bref/fpm-dev-gateway and bref/php-74-fpm-dev images and dumping out the values from $_SERVER the expected value can be seen.

Cause
When an instance of HttpRequestEvent is created the method HttpRequestEvent::rebuildQueryString is called which passed the value through parse_str causing the conversion. This value is set on the request passed the the FPM process by:

$request->setCustomVar('QUERY_STRING', $event->getQueryString());

Which is here https://github.com/brefphp/bref/blob/master/src/Event/Http/FpmHandler.php#L182. I can also see it being used in the Psr7Bridge here https://github.com/brefphp/bref/blob/master/src/Event/Http/Psr7Bridge.php#L33

@mnapoli mnapoli added the bug label Sep 22, 2020
@DaMitchell DaMitchell changed the title Value of $_SERVER['QUERY_STINRG] different when deployed Value of $_SERVER['QUERY_STRING] different when deployed Sep 28, 2020
@DaMitchell
Copy link
Author

@mnapoli Any ideas on when this can be addressed? I am happy to put a PR in myself just a little unsure about what you would expect in it. Other than updating the HttpRequestEvent::getQueryString() is there anything else you would expect to be updated?

@mnapoli
Copy link
Member

mnapoli commented Sep 29, 2020

A first step that could be very useful would be to add a failing test case. You can start by looking at these tests: https://github.com/brefphp/bref/blob/master/tests/Event/Http/CommonHttpTest.php#L76-L126 and maybe try to write a new one?

If that doesn't reproduce, then maybe you'll want to add it here: https://github.com/brefphp/bref/blob/master/tests/Handler/FpmHandlerTest.php

If you see a solution feel free to implement it too.

@vasoft
Copy link

vasoft commented Feb 12, 2023

I am migrating a large Api-platform 2.7(SF 6.2, PHP 8.1) to Bref & Serverless, and I am blocked on this problem with (.) becoming (_). Our project has a lot of nested filters in the request query string, and they are not working anymore.
I suppose anyone migrating Api-platform would face this issue.

@mnapoli
Copy link
Member

mnapoli commented Feb 12, 2023

FYI this PR might be very close to be merged: #1383

@sladg
Copy link
Contributor

sladg commented Mar 2, 2023

Heya! is there any movement on this issue? Experiencing exactly same issue (param names including dot have underscore in them once Bref stack is deployed).

ApiPlatform makes use of dots pretty standart and having them converted makes it rather difficult to deal with.

@mnapoli any work left / blocker on the PR mentioned? :)

@mnapoli
Copy link
Member

mnapoli commented Mar 3, 2023

@sladg yes, the blocker is this: #1383 (comment)

Feel free to open a new PR with these changes!

@vasoft
Copy link

vasoft commented Mar 3, 2023

@sladg, to move things forward with the project, we did an unorthodox thing until this PR gets merged. we searched for all the nested filters in the project,(order query string is not an issue), and replaced them back with (.) in the Kernel handle.

`
class Kernel extends BrefKernel
{
use MicroKernelTrait;

public function handle($request, $type = HttpKernelInterface::MAIN_REQUEST, $catch = true): Response
{
    // to be removed once this issue is merged https://github.com/brefphp/bref/issues/756
    $find = [
        'town_name',
        'product_price',
        //...
    ];
    $replace = [
        'town.name',
        'product.price',
        //...
    ];

    $rawRequestServerQueryString  = $request->server->get('QUERY_STRING');
    // fix in the query string
    $newRequestServerQueryString  = str_replace($find, $replace, $request->server->get('QUERY_STRING'));
    // replace the query string only for not to affect the route path
    $newRequestServerRequestUri  = str_replace(
        $rawRequestServerQueryString,
        $newRequestServerQueryString,
        $request->server->get('REQUEST_URI')
    );

    $request->server->set('QUERY_STRING',   $newRequestServerQueryString);
    $request->server->set('REQUEST_URI',    $newRequestServerRequestUri);

    return parent::handle($request, $type, $catch);
}

}
`

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