klein / klein.php

A fast & flexible router
MIT License
2.66k stars 290 forks source link

GET parameters not encoded anymore #277

Closed korpa closed 9 years ago

korpa commented 9 years ago

Hi Chriso,

When updating from 2.0.2 to 2.1.0 parameter handling changed. In 2.0.2 I got parameter encoded, while in 2.1.0 parameters are already decoded.

<?php

require '../vendor/autoload.php';

$klein = new \Klein\Klein();

$klein->respond('/[:name]', function ($request) {
    return 'Hello ' . $request->param['name'];
});    

$klein->dispatch();

When running /xyz%3Fxyz in 2.0.2 it shows xyz%3Fxyz, while in 2.1.0 it shows xyz?xyz. This breaks a few things in my code. Is there a way to get the parameter encoded?

Rican7 commented 9 years ago

Hi @korpa,

Yea, named parameters were improperly left encoded in v2.0.x. This was reported as a bug (#117) and after research was done and a plan to fix was made, the fix landed in #119. You'll notice that this was actually listed as a bug fix in the v2.1.0 release notes too.

I'm sorry if this effected your application in a negative way during the upgrade process. I'd suggest, if you have the resources/time, to make your application work with the new behavior as the old behavior was not correct.

Otherwise, you may be able to reverse the decoding by simply re-encoding the parameters in a catch-all block with a rawurlencode() call to the underlying parameters. Something like this maybe:

$klein = new \Klein\Klein();

/**
 * Revert decoding introduced in bugfix
 *
 * @link https://github.com/chriso/klein.php/pull/119
 */
$klein->respond(function ($request) {
    $named_params = $request->paramsNamed();

    $reencoded_params = array_map(
        'rawurlencode',
        $named_params->all()
    );

    $named_params->replace($named_params);
});

NOTE: I strongly suggest you not use the above. It would be much wiser to work with the newer, more correct behavior.

I hope this helped! Sorry if the upgrade caused any headaches. I tried to make it as easy as possible. :/

korpa commented 9 years ago

Hi @Rican7,

thanks for your quick answer. I think I have missed this part in the release notes :(

Normally this new behavior would work fine for me. But I have one case where I have to test, if the parameter is encoded or not. Therefore re-encoding makes no sense in this case because the test always would be true. What about a new method param_raw in $request? This will work like param, but will not change the parameters in anyway.

Rican7 commented 9 years ago

Hmm, yea, that's a bit of a pickle.

Unfortunately, a new method won't solve that problem, as the parameters are decoded before they're stored in the request container instance. Sorry. :/