slimphp / Slim-Http

A set of PSR-7 object decorators providing useful convenience methods
MIT License
151 stars 47 forks source link

ServerRequest getQueryParams doesn't allow to unset all QueryParams using withQueryParams #202

Open HLeithner opened 2 years ago

HLeithner commented 2 years ago

Hi,

I have a middleware which parses the lang query parameter and set's the language in the container. Afterwards it removes the lang parameter from the query array and gives this new request to the next handler. This works aslong as you have at least another query parameter.

if you don't have a query parameter this psr-7 implementation automatically parses Uri directly.

    public function __invoke(Request $request, RequestHandler $handler): Response
    {
        $query = $request->getQueryParams();

        // do somthing with the $query['lang] entry

        // cleanup Parameters
        unset($query['lang']);
        $request = $request->withQueryParams($query);

        return $handler->handle($request);
    }

When I later validate the request for "unwanted" parameters with something like:

public function __invoke(Request $request, Response $response)
{
  $params = $request->getQueryParams();
  if (isset($params['lang'])) {
    // throw exception or what ever.
  }
}

The $params has the lang parameter set again because of this line https://github.com/slimphp/Slim-Http/blob/master/src/ServerRequest.php#L220

Not sure why slim/http does it this way but I think it's wrong, because then Request::withQueryParams([]) is pretty useless and doesn't work.

Not sure if there is a workaround...

thanks

l0gicgate commented 1 year ago

I think the issue stems from this condition: https://github.com/slimphp/Slim-Http/blob/master/src/ServerRequest.php#L215

It checks to see if the underlying array is empty. It is debatable whether we should do this or not to trigger the reparsing of the query params. @akrabat do you believe an empty array should retrigger parsing of the query params? I have no strong position on this.

A workaround would be to ensure the array is not empty when you modify it with your middleware:

public function __invoke(Request $request, RequestHandler $handler): Response
{
    $query = $request->getQueryParams();

    // do something with the $query['lang] entry

    // cleanup Parameters
    unset($query['lang']);

    // setup workaround flag to ensure array is not empty
    $query['QUERY_PARAMS_PARSED'] = true;

    $request = $request->withQueryParams($query);

    return $handler->handle($request);
}
HLeithner commented 1 year ago

A workaround would be to ensure the array is not empty when you modify it with your middleware:

public function __invoke(Request $request, RequestHandler $handler): Response
{
    $query = $request->getQueryParams();

    // do something with the $query['lang] entry

    // cleanup Parameters
    unset($query['lang']);

    // setup workaround flag to ensure array is not empty
    $query['QUERY_PARAMS_PARSED'] = true;

    $request = $request->withQueryParams($query);

    return $handler->handle($request);
}

of course I can add a dummy parameter, but later in the code I validate all parameters and if we get an invalid parameter we drop the request. In this case the API endpoint as no GET parameters which are not globally handled (language in this case) so I wouldn't expect a GET parameter reaching the API endpoint controller. I would expect that's a middle ware is used for, preparing the request for the controller.