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

Already on GitHub? Sign in to your account

CHttpRequest::decodePathInfo crash PHP on windows for a long pathInfo #2292

Closed
Maxsh opened this Issue Apr 3, 2013 · 13 comments

Comments

6 participants
Contributor

Maxsh commented Apr 3, 2013

Regular expressions in the decodePathInfo() method crash PHP on a windows platform.
I've tested on 3 win platforms and got an error when I was trying use pathInfo with length around 310 characters and more
This issue is known. Please see for details
http://stackoverflow.com/questions/1282986/utf-8-validation-in-php-without-using-preg-match

Please see log from win machine:
Имя сбойного приложения: httpd.exe, версия: 2.2.22.0, отметка времени: 0x4f242d7a
Имя сбойного модуля: php5ts.dll, версия: 5.3.11.0, отметка времени 0x4f97356f
Код исключения: 0xc00000fd
Смещение ошибки: 0x0018b5d3
Идентификатор сбойного процесса: 0x1598
Время запуска сбойного приложения: 0x01ce3074ef780b8b
Путь сбойного приложения: C:\Program Files (x86)\Apache Software Foundation\Apache2.2\bin\httpd.exe
Путь сбойного модуля: C:\Program Files (x86)\PHP\php5ts.dll
Код отчета: 7e848139-9c68-11e2-bfeb-bcaec5bd8575

Owner

qiangxue commented Apr 3, 2013

Please see the last comment by Rasmus: https://bugs.php.net/bug.php?id=36463

@qiangxue qiangxue closed this Apr 3, 2013

Contributor

Maxsh commented Apr 3, 2013

but I have bigger values

pcre.backtrack_limit 1000000 1000000
pcre.recursion_limit 100000 100000

Owner

qiangxue commented Apr 3, 2013

I think your value is too big and thus cause depletion of memory (and thus the crash). Try with smaller values?

Contributor

Maxsh commented Apr 4, 2013

I've tried run with 10k, 100k, 500k values but anyway script was crashed with the same length of pathInfo (310 characters). It seems the issue doesn't depends on changes.
Anyway, I suppose this method is bottleneck. At least I suggest adding an option to disable this method run and using the old way (urldecode).

Owner

qiangxue commented Apr 4, 2013

I set prcre.recursion_limit to be 100 and it was able to work.

I agree we should look into some alternative solution to this.

@qiangxue qiangxue reopened this Apr 4, 2013

Contributor

Maxsh commented Apr 4, 2013

In my version, I've extended CHttpRequest and added property $decodePathUtf

protected function decodePathInfo($pathInfo)
{
    if (true === $this->decodePathUtf) {
        $pathInfo = parent::decodePathInfo($pathInfo);
    } else {
        $pathInfo = urldecode($pathInfo);
    }
    return $pathInfo;
}
Contributor

Maxsh commented Apr 4, 2013

and yes, with setting prcre.recursion_limit 100 it works fine!
but default value is 100k and I wouldn't want to restrict someone who use our product to change this settings on win platform

Member

klimov-paul commented Apr 4, 2013

"pcre.recursion_limit" could be setup runtime via ini_set:
ini_set('pcre.recursion_limit',100);

Contributor

Maxsh commented Apr 9, 2013

@klimov-paul You're right. It works fine at runtime.

Owner

samdark commented Apr 9, 2013

@Maxsh so can we close the issue or it still looks like it worth fixing?

Contributor

Maxsh commented Apr 9, 2013

I think the issue still worth fixing.
If we don't do any changes then on a win platform we can get the above bug and for this case we should apply the fix from @klimov-paul to Yii framework. But the value 100 is empirical and we have to make sure that this value won't cause any problem further. For example, I've changed pcre.backtrack_limit to 100 and got an issue with regular expression for a long text.
Or, we can change the behavior for determinate utf symbols in $pathInfo (don't ask me how :) )
Or, just to add option to avoid it #2292 (comment)

As a more solid use case, I ran into this issue when using CGridView with 'urlFormat'=>'path' for CUrlManager and having a large amount of searchable columns.

Corfiot commented May 15, 2015

I opted for the workaround in the config file. This will allow configuration of the recursion limit at will, at least until this is permanently solved.

//#2292
ini_set('pcre.recursion_limit',100);

@samdark samdark added the Won't Fix label Mar 13, 2017

@samdark samdark closed this Mar 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment