# include bug in 5.3 - Thread: https://externals.io/message/39700 - Original archive: https://news-web.php.net/php.internals/39700 - Started by Christian Stocker on 2008-08-06T15:35:16+00:00 - Messages: 41 - Page 1 of 2: messages 1 to 25 (25 per page, add `?page=N` to the URL) - Next page: https://externals.io/message/39700.md?page=2 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #39700 — Christian Stocker — 2008-08-06T15:35:16+00:00 URL: https://externals.io/message/39700#39700 Hi Since quite some time (weeks), I have this very strange and annoying include bug in 5.3-dev and now I think is the time to report it :) The reproducable script is here: http://trash.chregu.tv/include-bug.php.txt In short, when I have foo/alpha (empty) bar/alpha/bar.php and set the include path to foo/:bar/ and then do include("alpha/bar.php"); it throws an not-found error. But if I remove foo/alpha, it works correctly. In the meantime (while I wrote this), I found a similar bug report: http://bugs.php.net/bug.php?id=45044 and added my script. Can please someone look into that, it's not an area where I feel comfortable :) Thanks chregu -- New Office Zurich Opening: http://liip.to/party -- Liip AG // Feldstrasse 133 // CH-8004 Zurich Tel +41 43 500 39 81 // Mobile +41 76 561 88 60 www.liip.ch // blog.liip.ch // GnuPG 0x5CE1DECB --- ## #39721 — Stanislav Malyshev — 2008-08-07T01:03:18+00:00 URL: https://externals.io/message/39700#39721 · In reply to: #39700 Hi! > and set the include path to foo/:bar/ > and then do > include("alpha/bar.php"); > it throws an not-found error. Tried that, worked just fine for me - includes bar/alpha/bar.php. I notice your script on the link does something different, but works for me too with 5.3 CVS. -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #39722 — Christian Stocker — 2008-08-07T01:18:55+00:00 URL: https://externals.io/message/39700#39722 · In reply to: #39721 > Hi! > > > […] > > Tried that, worked just fine for me - includes bar/alpha/bar.php. I > notice your script on the link does something different, but works for > me too with 5.3 CVS. Did you try it on a *BSD Variant? And do you have an empty foo/alpha/ directory? That's what my script does and with which it fails on OS X. It seems to work on Linux. According to http://bugs.php.net/bug.php?id=45044 I'm not the only one with that problem and it really introduces a lot of issues. chregu -- Liip AG // Feldstrasse 133 // CH-8004 Zurich Tel +41 43 500 39 81 // Mobile +41 76 561 88 60 www.liip.ch // blog.liip.ch // GnuPG 0x5CE1DECB --- ## #39729 — Hannes Magnusson — 2008-08-07T11:38:21+00:00 URL: https://externals.io/message/39700#39729 · In reply to: #39722 On Thu, Aug 7, 2008 at 03:18, Christian Stocker wrote: > According to http://bugs.php.net/bug.php?id=45044 I'm not the only one with > that problem and it really introduces a lot of issues. You are not the onlyone. I'm pretty sure I mailed about this to internals@ few weeks ago too, and bugged few people on IRC, when it caused doc.php.net to go down. -Hannes --- ## #39748 — Rasmus Lerdorf — 2008-08-07T18:20:03+00:00 URL: https://externals.io/message/39700#39748 · In reply to: #39700 Christian Stocker wrote: > Hi > > Since quite some time (weeks), I have this very strange and annoying > include bug in 5.3-dev and now I think is the time to report it :) > > The reproducable script is here: > > http://trash.chregu.tv/include-bug.php.txt > > In short, when I have > > foo/alpha (empty) > bar/alpha/bar.php > > and set the include path to foo/:bar/ > and then do > include("alpha/bar.php"); > it throws an not-found error. > > But if I remove foo/alpha, it works correctly. > > In the meantime (while I wrote this), I found a similar bug report: > > http://bugs.php.net/bug.php?id=45044 > > and added my script. > > Can please someone look into that, it's not an area where I feel > comfortable :) I have tried to reproduce this on both OSX and FreeBSD and so far I haven't had any luck. This is what I have: 11:18am trainburn-lm:~/Web> l foo/alpha/ total 0 drwxr-xr-x 2 rasmus users 68 Aug 7 11:10 ./ drwxr-xr-x 3 rasmus users 102 Aug 7 11:10 ../ 11:18am trainburn-lm:~/Web> l bar/alpha/ total 8 drwxr-xr-x 3 rasmus users 102 Aug 7 11:10 ./ drwxr-xr-x 3 rasmus users 102 Aug 7 11:10 ../ -rw-r--r-- 1 rasmus users 12 Aug 7 11:10 bar.php 11:18am trainburn-lm:~/Web> cat foo.php I am getting the contents of bar.php quite nicely both from the command line and through Apache. This should be really easy to debug with gdb, but it requires a reproducable case. -Rasmus --- ## #39749 — Hannes Magnusson — 2008-08-07T18:55:55+00:00 URL: https://externals.io/message/39700#39749 · In reply to: #39748 > Christian Stocker wrote: > > […] > > I have tried to reproduce this on both OSX and FreeBSD and so far I haven't > had any luck. > > This is what I have: > > 11:18am trainburn-lm:~/Web> l foo/alpha/ > total 0 > drwxr-xr-x 2 rasmus users 68 Aug 7 11:10 ./ > drwxr-xr-x 3 rasmus users 102 Aug 7 11:10 ../ > 11:18am trainburn-lm:~/Web> l bar/alpha/ > total 8 > drwxr-xr-x 3 rasmus users 102 Aug 7 11:10 ./ > drwxr-xr-x 3 rasmus users 102 Aug 7 11:10 ../ > -rw-r--r-- 1 rasmus users 12 Aug 7 11:10 bar.php > 11:18am trainburn-lm:~/Web> cat foo.php > ini_set('include_path','foo/:bar/'); > include "alpha/bar.php"; > ?> > > I am getting the contents of bar.php quite nicely both from the command line > and through Apache. This should be really easy to debug with gdb, but it > requires a reproducable case. -bash-2.05b$ php include-bug.php.txt Doesn't work PHP Warning: include(/usr/home/bjori/php5.3-200808071630/one/alpha/bar.php): failed to open stream: No such file or directory in /usr/home/bjori/php5.3-200808071630/include-bug.php.txt on line 40 Warning: include(/usr/home/bjori/php5.3-200808071630/one/alpha/bar.php): failed to open stream: No such file or directory in /usr/home/bjori/php5.3-200808071630/include-bug.php.txt on line 40 PHP Warning: include(): Failed opening 'alpha/bar.php' for inclusion (include_path='./one/:./two/') in /usr/home/bjori/php5.3-200808071630/include-bug.php.txt on line 40 Warning: include(): Failed opening 'alpha/bar.php' for inclusion (include_path='./one/:./two/') in /usr/home/bjori/php5.3-200808071630/include-bug.php.txt on line 40 Now it works: works -bash-2.05b$ find one/ two/ one/ two/ two/alpha two/alpha/bar.php -bash-2.05b$ uname -a FreeBSD pb11.pair.com 4.11-STABLE FreeBSD 4.11-STABLE #1: Tue May 3 13:17:19 EDT 2005 root@pb11.pair.com:/usr/obj/usr/src/sys/NEWPB11 i386 -Hannes --- ## #39751 — Felipe Pena — 2008-08-07T19:49:08+00:00 URL: https://externals.io/message/39700#39751 · In reply to: #39749 Em Qui, 2008-08-07 às 20:55 +0200, Hannes Magnusson escreveu: > > > […] When trying debug it days ago, I noticed add that: +++ TSRM/tsrm_virtual_cwd.c 4 Jul 2008 03:05:16 -0000 @@ -557,9 +557,11 @@ CWD_API int virtual_file_ex(cwd_state *s if (use_realpath != CWD_EXPAND) { #if !defined(TSRM_WIN32) && !defined(NETWARE) char resolved_path[MAXPATHLEN]; + printf(">>> %s\n", path); if (!realpath(path, resolved_path)) { /* Note: Not threadsafe on older *BSD's */ + printf(">> 1\n"); if (use_realpath == CWD_REALPATH) { + printf(">> 2\n"); ---------------------------------------------------------------- LINUX: > > […] foo ---------------------------------- BSD: > > […] Warning: include(/usr/home/felipe/foo.php): failed to open stream: No such file or directory in /usr/home/felipe/test/test.php on line 1 Warning: include(): Failed opening 'foo.php' for inclusion (include_path='.:/usr/local/lib/php') in /usr/home/felipe/test/test.php on line 1 -- Regards, Felipe Pena. --- ## #39752 — Rasmus Lerdorf — 2008-08-07T19:52:37+00:00 URL: https://externals.io/message/39700#39752 · In reply to: #39751 Felipe Pena wrote: > Em Qui, 2008-08-07 às 20:55 +0200, Hannes Magnusson escreveu: > > […] > > When trying debug it days ago, I noticed add that: > +++ TSRM/tsrm_virtual_cwd.c 4 Jul 2008 03:05:16 -0000 > @@ -557,9 +557,11 @@ CWD_API int virtual_file_ex(cwd_state *s > if (use_realpath != CWD_EXPAND) { > #if !defined(TSRM_WIN32)&& !defined(NETWARE) > char resolved_path[MAXPATHLEN]; > + printf(">>> %s\n", path); > if (!realpath(path, resolved_path)) { /* Note: Not > threadsafe on older *BSD's */ > + printf(">> 1\n"); > if (use_realpath == CWD_REALPATH) { > + printf(">> 2\n"); > > ---------------------------------------------------------------- > LINUX: > > […] > foo > > ---------------------------------- > BSD: > > […] > > Warning: include(/usr/home/felipe/foo.php): failed to open stream: No > such file or directory in /usr/home/felipe/test/test.php on line 1 > > Warning: include(): Failed opening 'foo.php' for inclusion > (include_path='.:/usr/local/lib/php') in /usr/home/felipe/test/test.php > on line 1 That's not the same thing though. Your case seems to show that it isn't using the include_path at all. On BSD after checking ./foo.php why doesn't it continue on to /usr/local/lib/php/foo.php? It certain does for me here. -Rasmus --- ## #39753 — Arnaud Le Blanc — 2008-08-07T20:37:01+00:00 URL: https://externals.io/message/39700#39753 · In reply to: #39752 On Thursday 07 August 2008 21:52:37 Rasmus Lerdorf wrote: > Felipe Pena wrote: > > […] > > That's not the same thing though. Your case seems to show that it isn't > using the include_path at all. On BSD after checking ./foo.php why > doesn't it continue on to /usr/local/lib/php/foo.php? It certain does > for me here. > > -Rasmus Hi, I had searched the cause of this bug some days ago and posted what I have found on http://bugs.php.net/bug.php?id=45044 . virtual_file_ex() assumes that `realpath()` returns `NULL` when the file does not exists. But BSD's `realpath()` will not return `NULL` if all components *but the last* exist. So realpath(/foo/bar) will return /foo/bar even if bar does not exists. Regards, Arnaud --- ## #39756 — Hannes Magnusson — 2008-08-08T07:11:30+00:00 URL: https://externals.io/message/39700#39756 · In reply to: #39753 > virtual_file_ex() assumes that `realpath()` returns `NULL` when the file does not > exists. But BSD's `realpath()` will not return `NULL` if all > components *but the last* exist. So realpath(/foo/bar) will return /foo/bar > even if bar does not exists. And that behavior is intended, but is sadly overwritten in ZTS :( -Hannes --- ## #39759 — Dmitry Stogov — 2008-08-08T15:52:26+00:00 URL: https://externals.io/message/39700#39759 · In reply to: #39756 Hi, The attached patch is going to fix the problem. It implements its own `realpath()` function, so we won't depend on system anymore. It also improve realpath cache usage by caching intermediate results. I tested it on Linux and Windows only and it seems to work without problems. It breaks one test related to `clearstatcache()` function, but this break is expected. Could you please test it. Does it really fix the bug on FreeBSD? Thanks. Dmitry. Hannes Magnusson wrote: > > > […] > > And that behavior is intended, but is sadly overwritten in ZTS :( > > -Hannes --- ## #39760 — Pierre Joye — 2008-08-08T17:23:00+00:00 URL: https://externals.io/message/39700#39760 · In reply to: #39759 Hi Dmitry, > Hi, > > The attached patch is going to fix the problem. > It implements its own `realpath()` function, so we won't depend on system > anymore. It also improve realpath cache usage by caching intermediate > results. > > I tested it on Linux and Windows only and it seems to work without > problems. It breaks one test related to `clearstatcache()` function, but > this break is expected. > > Could you please test it. I can test it asap on various windows, probably on Monday. After a quick review, it seems that FindClose is missing in the two buffer overflow check. > - state->cwd_length += ptr_length; > +#ifdef TSRM_WIN32 > + if ((hFind = FindFirstFile(to, &data)) != INVALID_HANDLE_VALUE) { > +fprintf(stderr, "1 %s (%d)\n", to, len); > + j = strlen(data.cFileName); > + > + i = len; > + while (!IS_SLASH(to[i-1])) { > + i--; > + } > + if (i + j >= MAXPATHLEN-1) { > + /*buffer overflow */ FindClose(hFind); > + return NULL; > + } > + if (CWDG(realpath_cache_size_limit)) { > + memcpy(tmp, to, i); > + memcpy(tmp + i, data.cFileName, j + 1); > + realpath_cache_add(to, len, tmp, i + j, t TSRMLS_CC); > + } > + ret -= (j + 1); > + if (ret < buf) { > + /* buffer overflow */ FindClose(hFind); > + return NULL; > + } > + *ret = DEFAULT_SLASH; > + memcpy(ret+1, data.cFileName, j); > + FindClose(hFind); > + len = i - 1; > + to[len] = 0; > +fprintf(stderr, "2 %s - %s\n", to, ret); > + continue; > + } else { > + if (use_realpath == CWD_REALPATH) { > + return NULL; > + } else { > + goto next_dir; > } > - ptr = tsrm_strtok_r(NULL, TOKENIZER_STRING, &tok); > } Thanks for your work! -- Pierre http://blog.thepimp.net | http://www.libgd.org --- ## #39761 — Dmitry Stogov — 2008-08-08T17:28:26+00:00 URL: https://externals.io/message/39700#39761 · In reply to: #39760 Hi Pierre, Thank you for feedback. I already noticed the missing FindClose() and several other issues. I'll send updated patch on weekend, but the sooner I get info if bug is fixed on FreeBSD the better. Thanks. Dmitry. Pierre Joye wrote: > Hi Dmitry, > > > > […] > > I can test it asap on various windows, probably on Monday. > > After a quick review, it seems that FindClose is missing in the two > buffer overflow check. > > > […] > > FindClose(hFind); > > > […] > > FindClose(hFind); > > > […] > > Thanks for your work! --- ## #39762 — Arnaud Le Blanc — 2008-08-08T17:30:18+00:00 URL: https://externals.io/message/39700#39762 · In reply to: #39759 Hi, On Friday 08 August 2008 17:52:26 Dmitry Stogov wrote: > Hi, > > The attached patch is going to fix the problem. > It implements its own `realpath()` function, so we won't depend on system > anymore. It also improve realpath cache usage by caching intermediate > results. > > I tested it on Linux and Windows only and it seems to work without > problems. It breaks one test related to `clearstatcache()` function, but > this break is expected. > > Could you please test it. > Does it really fix the bug on FreeBSD? Yes, it fixes the bug on FreeBSD :) > > Thanks. Dmitry. > > Hannes Magnusson wrote: > > […] Regards, Arnaud --- ## #39763 — Dmitry Stogov — 2008-08-08T18:04:40+00:00 URL: https://externals.io/message/39700#39763 · In reply to: #39762 Great! :) It's mean I'm on the right way. The patch still needs additional work and testing, but I hope it'll be ready before alpha 2. Thanks. Dmitry. Arnaud Le Blanc wrote: > Hi, > > On Friday 08 August 2008 17:52:26 Dmitry Stogov wrote: > > […] > > Yes, it fixes the bug on FreeBSD :) > > > […] > > Regards, > > Arnaud --- ## #39774 — Jani Taskinen — 2008-08-08T21:28:18+00:00 URL: https://externals.io/message/39700#39774 · In reply to: #39759 Just semantics but I'd name the function 'tsrm_realpath[_r]' to be absolutely clear what it does. --Jani Dmitry Stogov kirjoitti: > Hi, > > The attached patch is going to fix the problem. > It implements its own `realpath()` function, so we won't depend on system > anymore. It also improve realpath cache usage by caching intermediate > results. > > I tested it on Linux and Windows only and it seems to work without > problems. It breaks one test related to `clearstatcache()` function, but > this break is expected. > > Could you please test it. > Does it really fix the bug on FreeBSD? > > Thanks. Dmitry. > > > Hannes Magnusson wrote: > > […] --- ## #39775 — Rasmus Lerdorf — 2008-08-08T21:42:48+00:00 URL: https://externals.io/message/39700#39775 · In reply to: #39759 Dmitry Stogov wrote: > Hi, > > The attached patch is going to fix the problem. > It implements its own `realpath()` function, so we won't depend on system > anymore. It also improve realpath cache usage by caching intermediate > results. Very nice. The intermediate caching is going to drastically reduce the amount of memory we need for the cache. I have seen a number of cases where sites had to increase the realpath cache size by quite a bit. -Rasmus --- ## #39776 — Rasmus Lerdorf — 2008-08-08T21:45:37+00:00 URL: https://externals.io/message/39700#39776 · In reply to: #39775 Rasmus Lerdorf wrote: > Dmitry Stogov wrote: > > […] > > Very nice. The intermediate caching is going to drastically reduce the > amount of memory we need for the cache. I have seen a number of cases > where sites had to increase the realpath cache size by quite a bit. Oh, and on top of that we also get a big performance improvement for include_path misses since we no longer have to re-stat every component of the docroot on every miss. -Rasmus --- ## #39790 — Christian Stocker — 2008-08-09T06:43:14+00:00 URL: https://externals.io/message/39700#39790 · In reply to: #39759 Hi Dmitry > Hi, > > The attached patch is going to fix the problem. Works also on my OS X 10.4 box now. Thanks a lot for the effort (and if it brings a performance boost, even better :) ) chregu > It implements its own `realpath()` function, so we won't depend on system > anymore. It also improve realpath cache usage by caching intermediate > results. > > I tested it on Linux and Windows only and it seems to work without > problems. It breaks one test related to `clearstatcache()` function, but > this break is expected. > > Could you please test it. > Does it really fix the bug on FreeBSD? > > Thanks. Dmitry. > > > Hannes Magnusson wrote: > > […] -- New Office Zurich Opening: http://liip.to/party -- Liip AG // Feldstrasse 133 // CH-8004 Zurich Tel +41 43 500 39 81 // Mobile +41 76 561 88 60 www.liip.ch // blog.liip.ch // GnuPG 0x5CE1DECB --- ## #39791 — Hannes Magnusson — 2008-08-09T10:18:45+00:00 URL: https://externals.io/message/39700#39791 · In reply to: #39759 > Hi, > > The attached patch is going to fix the problem. > It implements its own `realpath()` function, so we won't depend on system > anymore. It also improve realpath cache usage by caching intermediate > results. > > I tested it on Linux and Windows only and it seems to work without > problems. It breaks one test related to `clearstatcache()` function, but > this break is expected. > > Could you please test it. > Does it really fix the bug on FreeBSD? Yup. But it introduces a new one. `realpath()` no longer works as documented, now requiring all the components to exist. -Hannes --- ## #39792 — Dmitry Stogov — 2008-08-09T10:31:02+00:00 URL: https://externals.io/message/39700#39792 · In reply to: #39791 Where is it documented? The `realpath()` implementation conforming to 4.4BSD and POSIX.1-2001 requires file existence. With the patch it'll work in the same standard way on all systems. Thanks. Dmitry. Hannes Magnusson wrote: > > > […] > > Yup. But it introduces a new one. > `realpath()` no longer works as documented, now requiring all the > components to exist. > > -Hannes --- ## #39793 — Hannes Magnusson — 2008-08-09T10:41:44+00:00 URL: https://externals.io/message/39700#39793 · In reply to: #39792 > Where is it documented? http://php.net/realpath http://www.ipnom.com/FreeBSD-Man-Pages/realpath.3.html -Hannes --- ## #39794 — Pierre Joye — 2008-08-09T11:16:12+00:00 URL: https://externals.io/message/39700#39794 · In reply to: #39793 hi all! On Sat, Aug 9, 2008 at 12:41 PM, Hannes Magnusson wrote: > > > […] > > http://php.net/realpath > http://www.ipnom.com/FreeBSD-Man-Pages/realpath.3.html "On BSD systems `realpath()` doesn't fail if only the last path component doesn't exist, while other systems will return FALSE." This behavior has genearated enough pain in the past to be dropped. The key problem in php application is portability. I'm in favor of dropping this BSD specific thing in 5.3+. Cheers, -- Pierre http://blog.thepimp.net | http://www.libgd.org --- ## #39795 — Stefan Esser — 2008-08-09T11:35:57+00:00 URL: https://externals.io/message/39700#39795 · In reply to: #39759 -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hello Dmitry, while you are at fixing `realpath()` it might be a good idea to fix the ../ nonsense. What I mean is: fopen("this_is_not_a_dir_but_a_file/../../../../../../../../etc/passwd", "r"); works because of `realpath()` and PHP's wrapper. Same for fopen("this_is_not_existing/../../../../../../../../etc/passwd", "r"); Both is madness... Stefan -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.8 (Darwin) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEUEARECAAYFAkidgR0ACgkQSuF5XhWr2nhovACXZpeATBITDai/M1wsCuavuZ3C OgCgn46uM4XHwENW7si4aJzeNgnuTKg= =QiYy -----END PGP SIGNATURE --- ## #39797 — Dmitry Stogov — 2008-08-09T15:28:07+00:00 URL: https://externals.io/message/39700#39797 · In reply to: #39776 The improved patch fixes all the issues I found during testing. However I wasn't able to test it on NETWARE and on Solaris with relative paths. Please test it as much as possible. I'm going to commit it on Tuesday in case of no objections. Thanks. Dmitry. Rasmus Lerdorf wrote: > Rasmus Lerdorf wrote: > > […] > > Oh, and on top of that we also get a big performance improvement for > include_path misses since we no longer have to re-stat every component > of the docroot on every miss. > > -Rasmus --- Next page: https://externals.io/message/39700.md?page=2