Bug Report #4613

Request class converts . to _ in url query string

Added by Pavel Kulbakin about 2 years ago. Updated 9 months ago.

Status:ClosedStart date:10/14/2012
Priority:NormalDue date:
Assignee:Lorenzo Pisani% Done:

0%

Category:Core
Target version:v3.2.3
Resolution:wontfix Points:1

Description

Request class internally uses parse_str for query string which converts . (dots) into _ (underscore)
It makes the class not possible to use for external requests which have their URLs containing dots in query parameter names

History

#1 Updated by Lorenzo Pisani about 2 years ago

Could you tell me which version of Kohana you're using and show me a quick example to reproduce this? It will just save me some time when I try to solve this later :)

Thanks!

#2 Updated by Pavel Kulbakin about 2 years ago

kohana 3.2.2

i believe the problem in system/classes/kohana/request.php line no 787

        // Initial request has global $_GET already applied
        if (Request::$initial !== NULL)
        {
            if ($split_uri)
            {
                parse_str($split_uri[0], $this->_get);
            }
        }

#3 Updated by Pavel Kulbakin about 2 years ago

as for example

$request = Request::factory('http://some.domain/path?param.x=25&pram.y=45');
$response = $request->execute();
var_dump($response->effective_url()); // this will give you http://some.domain/path?param_x=25&pram_y=45

i.e. as i mentioned in original report it will replace dots to underscores which will make different request not giving what was originally requested

#4 Updated by Lorenzo Pisani about 2 years ago

  • Assignee set to Lorenzo Pisani
  • Target version set to v3.2.3

Interesting :) I'll take a look soon but I want to focus on 3.3 for this week

#5 Updated by Pavel Kulbakin about 2 years ago

checked 3.3 and it must have the same problem
https://github.com/kohana/core/blob/3.3/develop/classes/Kohana/Request.php line no 673

        // Initial request has global $_GET already applied
        if (Request::$initial !== NULL)
        {
            if ($split_uri)
            {
                parse_str($split_uri[0], $this->_get);
            }
        }

exactly the same snippet as in 3.2.2

#6 Updated by Lorenzo Pisani almost 2 years ago

  • Status changed from New to Closed
  • Resolution set to wontfix

So I looked into this a bit and it seems like PHP simply does this for legacy reasons and there are no flags we can set to make it stop (that I could find). You can take a look at http://stackoverflow.com/questions/68651/can-i-get-php-to-stop-replacing-characters-in-get-or-post-arrays for some explanations.

I've also tried using parse_url() but it simply replaced it with a "-" instead of a "_" so that won't work.

What DOES work is if you simply set the query string using the Request::query() method. Since this method does not run parse_str(), it won't give PHP a chance to mess things up :)

Request::factory('http://some.domain/path')->query(array('param.x' => 25, 'pram.y' => 45,));

#7 Updated by Pavel Kulbakin almost 2 years ago

What you tell is rather a workaround, but it doesn't address the original problem which is unexpected transformation of query string.
What i did for my particular task i hacked into core request class and replaced parse_str call with my own implementation of the function which does similar task without this legacy replacement.

If it helps, here is my version

    /**
     * Replacement for parse_str()
     * This version doesn't replace . to _
     * 
     * @return array
     */
    public static function parse_str($str)
    {
        $op = array();
        $pairs = explode("&", $str);
        foreach ($pairs as $pair) {
            if (FALSE === strpos($pair, '=')) {
                $op[$pair] = "";
            } else {
                list($k, $v) = array_map("urldecode", explode("=", $pair));
                if (preg_match('%^(.+?)((\[[^\[\]]*\])+)$%', $k, $m)) {
                    $n = $m[1];
                    isset($op[$n]) or $op[$n] = array();
                    $a = &$op[$n];
                    if (preg_match_all('%\[([^\[\]]*)\]%', $m[2], $mtch)) while($mtch[1]) {
                        $ak = array_shift($mtch[1]);
                        if ('' == $ak) {
                            $a[] = array();
                            $a = &$a[max(array_keys($a))];
                        } else {
                            isset($a[$ak]) or $a[$ak] = array();
                            $a = &$a[$ak];
                        }
                    }
                    $a = $v;
                } else {
                    $op[$k] = $v;
                }
            }
        }
        return $op;
    }

I cannot vouch for it to be exact replacement of parse_str since i couldn't find specification for the latter, but i did some comparison testing and it seems the replacement works similar (for me it did the trick)

#8 Updated by Pavel Kulbakin almost 2 years ago

ups, here is a better version

    /**
     * Replacement for parse_str()
     * This version doesn't replace . to _
     * 
     * @return array
     */
    public static function parse_str($str)
    {
        $op = array();
        $pairs = explode("&", $str);
        foreach ($pairs as $pair) {
            if (FALSE === strpos($pair, '=')) {
                $op[urldecode($pair)] = "";
            } else {
                list($k, $v) = array_map("urldecode", explode("=", $pair));
                if (preg_match('%^(.+?)((\[[^\[\]]*\])+)$%', $k, $m)) {
                    $n = $m[1];
                    isset($op[$n]) or $op[$n] = array();
                    $a = &$op[$n];
                    if (preg_match_all('%\[([^\[\]]*)\]%', $m[2], $mtch)) while($mtch[1]) {
                        $ak = array_shift($mtch[1]);
                        if ('' == $ak) {
                            $a[] = array();
                            $a = &$a[max(array_keys($a))];
                        } else {
                            isset($a[$ak]) or $a[$ak] = array();
                            $a = &$a[$ak];
                        }
                    }
                    $a = $v;
                } else {
                    $op[$k] = $v;
                }
            }
        }
        return $op;
    }

#9 Updated by Lorenzo Pisani almost 2 years ago

  • Status changed from Closed to Feedback

Yeah I'm not sure I want to reimplement a PHP function but I'll leave this as feedback for now. We could add a URL::parse_str() in 3.4.0 if needed =/ silly PHP

#10 Updated by Scott Jungwirth almost 2 years ago

I ran into this same problem while working with Solr, some of which's query params contain dots.

#11 Updated by Scott Jungwirth almost 2 years ago

Another issue when interfacing with Solr, is that it is sometimes needed to pass multiple parameters with the same name, almost like sending an array in the query string but it doesn't accept the arr[]=value1&arr[]=value2 syntax, instead it requires arr=value1&arr=value2. I think I am going to try to add a _raw_query_string property to Request and use that if it exists, otherwise use the _query array.

#12 Updated by Lorenzo Pisani 9 months ago

  • Status changed from Feedback to Closed

Closing this but we can look into how to fix PHPs functionality in a future major version.

Also available in: Atom PDF