# primary file realpath change

- Thread: https://externals.io/message/22317
- Original archive: https://news-web.php.net/php.internals/22317
- Started by Brian J. France on 2006-03-10T05:13:56+00:00
- Messages: 8
- 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

---

## #22317 — Brian J. France — 2006-03-10T05:13:56+00:00

URL: https://externals.io/message/22317#22317

Does anybody see a problem with this patch (which is currently  
against the 5.1.2 release)?

expand_filepath will eventual do a realpath, but it also uses the  
realpath cache before calling realpath.  VCWD_REALPATH just maps  
directly to realpath and doesn't use the realpath cache so for every  
request you get a realpath call (and all the syscalls that go with  
it) on the primary file.

I think I need to pass TSRMLS_CC, but I am not sure.

Any problems committing it to PHP_5_1 and HEAD?

Brian

--- ../php-5.1.2.orig/main/main.c       Sun Jan  1 04:50:17 2006
+++ main/main.c Thu Mar  9 20:00:47 2006
@@ -1684,7 +1684,7 @@
                         char realfile[MAXPATHLEN];
                         int realfile_len;
                         int dummy = 1;
-                       if (VCWD_REALPATH(primary_file->filename,  
realfile)) {
+                       if (expand_filepath(primary_file->filename,  
realfile TSRMLS_CC)) {
                                 realfile_len =  strlen(realfile);
                                 zend_hash_add(&EG(included_files),  
realfile, realfile_len+1, (void *)&dummy, sizeof(int), NULL);
                                 if (strncmp(realfile, primary_file- 
 >filename, realfile_len)) {

---

## #22318 — Andi Gutmans — 2006-03-10T06:25:33+00:00

URL: https://externals.io/message/22317#22318 · In reply to: #22317

Are you sure VCWD_REALPATH doesn't use the realpath cache? It did 
last time I checked and I think is still the right method to use there...

At 09:13 PM 3/9/2006, Brian J. France wrote:
>Does anybody see a problem with this patch (which is currently
>against the 5.1.2 release)?
>
>expand_filepath will eventual do a realpath, but it also uses the
>realpath cache before calling realpath.  VCWD_REALPATH just maps
>directly to realpath and doesn't use the realpath cache so for every
>request you get a realpath call (and all the syscalls that go with
>it) on the primary file.
>
>I think I need to pass TSRMLS_CC, but I am not sure.
>
>Any problems committing it to PHP_5_1 and HEAD?
>
>Brian
>
>--- ../php-5.1.2.orig/main/main.c       Sun Jan  1 04:50:17 2006
>+++ main/main.c Thu Mar  9 20:00:47 2006
>@@ -1684,7 +1684,7 @@
>                         char realfile[MAXPATHLEN];
>                         int realfile_len;
>                         int dummy = 1;
>-                       if (VCWD_REALPATH(primary_file->filename,
>realfile)) {
>+                       if (expand_filepath(primary_file->filename,
>realfile TSRMLS_CC)) {
>                                 realfile_len =  strlen(realfile);
>                                 zend_hash_add(&EG(included_files),
>realfile, realfile_len+1, (void *)&dummy, sizeof(int), NULL);
>                                 if (strncmp(realfile, 
> primary_file- >filename, realfile_len)) {

---

## #22319 — Rasmus Lerdorf — 2006-03-10T06:31:05+00:00

URL: https://externals.io/message/22317#22319 · In reply to: #22318

Andi Gutmans wrote:
> Are you sure VCWD_REALPATH doesn't use the realpath cache? It did last 
> time I checked and I think is still the right method to use there...

quite

#define VCWD_REALPATH(path, real_path) realpath(path, real_path)

-Rasmus

---

## #22320 — Rasmus Lerdorf — 2006-03-10T06:57:40+00:00

URL: https://externals.io/message/22317#22320 · In reply to: #22319

Rasmus Lerdorf wrote:
> Andi Gutmans wrote:
> > […]
> 
> quite
> 
> #define VCWD_REALPATH(path, real_path) realpath(path, real_path)

By the way, I agree that expand_filepath() is not the right fix here 
since the first thing it does is a getcwd.  We have just done a chdir 
and haven't executed any script yet so we know the cwd.  Feed that 
directly to virtual_file_ex() there perhaps?  I haven't really dug into it.

-Rasmus

---

## #22321 — Andi Gutmans — 2006-03-10T06:59:46+00:00

URL: https://externals.io/message/22317#22321 · In reply to: #22319

Seems that needs fixing then (non-TSRM). We should support the 
realpath cache also in non-TSRM mode.

At 10:31 PM 3/9/2006, Rasmus Lerdorf wrote:
>Andi Gutmans wrote:
> > […]
>
>quite
>
>#define VCWD_REALPATH(path, real_path) realpath(path, real_path)
>
>-Rasmus

---

## #22333 — Brian J. France — 2006-03-10T15:19:14+00:00

URL: https://externals.io/message/22317#22333 · In reply to: #22320

> Rasmus Lerdorf wrote:
> > […]
>
> By the way, I agree that expand_filepath() is not the right fix  
> here since the first thing it does is a getcwd.  We have just done  
> a chdir and haven't executed any script yet so we know the cwd.   
> Feed that directly to virtual_file_ex() there perhaps?  I haven't  
> really dug into it.

Were you seeing two getcwd calls?  I can remove one, but I don't see  
away around the second one.  I changed to virtual_realpath which does  
remove the getcwd in php_execute_script function, but it seems that  
is shows up a little later when it uses the stream open which called  
expand_filepath:

#0  0x203ab410 in __getcwd () from /usr/lib/libc.so.4
#1  0x203b2ab6 in getcwd () from /usr/lib/libc.so.4
#2  0x2060de86 in expand_filepath (filepath=0x2cbc08 "<path>/ 
bench_main.php", real_path=0x0)
     at main/fopen_wrappers.c:521
#3  0x2061c1bd in _php_stream_fopen (filename=0x2cbc08 "<path>/ 
bench_main.php", mode=0x206f1fe3 "rb",
     opened_path=0x9fbff7cc, options=133)
     at /main/streams/plain_wrapper.c:855
#4  0x2061c974 in _php_stream_fopen_with_path (filename=0x2cbc08  
"<path>/bench_main.php",
     mode=0x206f1fe3 "rb", path=0x12b640 ".:/<path>/pear",  
opened_path=0x9fbff7cc, options=133)
     at /main/streams/plain_wrapper.c:1238
#5  0x2061c338 in php_plain_files_stream_opener (wrapper=0x2077c008,  
path=0x2cbc08
     "<path>/bench_main.php", mode=0x206f1fe3 "rb", options=133,  
opened_path=0x9fbff7cc, context=0x0)
     at/main/streams/plain_wrapper.c:931
#6  0x2061962e in _php_stream_open_wrapper_ex (path=0x2cbc08  
"<path>bench_main.php",
     mode=0x206f1fe3 "rb", options=141, opened_path=0x9fbff7cc,  
context=0x0)
     at main/streams/streams.c:1771
#7  0x2060943b in php_stream_open_for_zend (filename=0x2cbc08 "<path>/ 
bench_main.php",
     handle=0x9fbff7c4) at main/main.c:901
#8  0x2064a866 in zend_stream_open (filename=0x2cbc08 "<path>/ 
bench_main.php",
     handle=0x9fbff7c4) at Zend/zend_stream.c:47
#9  0x2064a8dc in zend_stream_fixup (file_handle=0x9fbff7c4) at Zend/ 
zend_stream.c:62
#10 0x20623ee7 in open_file_for_scanning (file_handle=0x9fbff7c4) at  
Zend/zend_language_scanner.c:2982
#11 0x20623fce in compile_file (file_handle=0x9fbff7c4, type=2) at  
Zend/zend_language_scanner.c:3068
#12 0x2063cd1c in zend_execute_scripts (type=8, retval=0x0,  
file_count=3) at Zend/zend.c:1093
#13 0x2060a653 in php_execute_script (primary_file=0x9fbff7c4) at  
main/main.c:1724

Brian

---

## #22334 — Rasmus Lerdorf — 2006-03-10T15:58:48+00:00

URL: https://externals.io/message/22317#22334 · In reply to: #22333

After poking this a bit more, it looks like this whole block of code in 
main.c is redundant.  Here is the story:

We start our journey on a dark and stormy night in June 2002 with this 
bug:  http://bugs.php.net/17720

This led to this fix: 
http://cvs.php.net/viewcvs.cgi/php-src/main/main.c?r1=1.461&r2=1.462
which had this commit message:
Main script should not be parsed when (include|require)_once()'ed, #17720
# Derick, MFH?

Now, at the time this was the correct fix.  However, fast forward to 
March 2005 and we have a patch from Stas to zend_execute_scripts:
http://cvs.php.net/viewcvs.cgi/ZendEngine2/zend.c?view=diff&r1=1.297&r2=1.298
which adds the opened file to the included_files list.  This means that 
we are now trying to add the main script to the included_file list twice 
which in itself is bad.

We'll test this a bit more, but it looks safe to remove that block of 
code.  The test case in bug 17720 works fine without that block of code 
in main.c and I see no other reason to have it there.

-Rasmus

Brian J. France wrote:
> 
> 
> 
> > […]
> 
> Were you seeing two getcwd calls?  I can remove one, but I don't see 
> away around the second one.  I changed to virtual_realpath which does 
> remove the getcwd in php_execute_script function, but it seems that is 
> shows up a little later when it uses the stream open which called 
> expand_filepath:
> 
> #0  0x203ab410 in __getcwd () from /usr/lib/libc.so.4
> #1  0x203b2ab6 in getcwd () from /usr/lib/libc.so.4
> #2  0x2060de86 in expand_filepath (filepath=0x2cbc08 
> "<path>/bench_main.php", real_path=0x0)
>     at main/fopen_wrappers.c:521
> #3  0x2061c1bd in _php_stream_fopen (filename=0x2cbc08 
> "<path>/bench_main.php", mode=0x206f1fe3 "rb",
>     opened_path=0x9fbff7cc, options=133)
>     at /main/streams/plain_wrapper.c:855
> #4  0x2061c974 in _php_stream_fopen_with_path (filename=0x2cbc08 
> "<path>/bench_main.php",
>     mode=0x206f1fe3 "rb", path=0x12b640 ".:/<path>/pear", 
> opened_path=0x9fbff7cc, options=133)
>     at /main/streams/plain_wrapper.c:1238
> #5  0x2061c338 in php_plain_files_stream_opener (wrapper=0x2077c008, 
> path=0x2cbc08
>     "<path>/bench_main.php", mode=0x206f1fe3 "rb", options=133, 
> opened_path=0x9fbff7cc, context=0x0)
>     at/main/streams/plain_wrapper.c:931
> #6  0x2061962e in _php_stream_open_wrapper_ex (path=0x2cbc08 
> "<path>bench_main.php",
>     mode=0x206f1fe3 "rb", options=141, opened_path=0x9fbff7cc, context=0x0)
>     at main/streams/streams.c:1771
> #7  0x2060943b in php_stream_open_for_zend (filename=0x2cbc08 
> "<path>/bench_main.php",
>     handle=0x9fbff7c4) at main/main.c:901
> #8  0x2064a866 in zend_stream_open (filename=0x2cbc08 
> "<path>/bench_main.php",
>     handle=0x9fbff7c4) at Zend/zend_stream.c:47
> #9  0x2064a8dc in zend_stream_fixup (file_handle=0x9fbff7c4) at 
> Zend/zend_stream.c:62
> #10 0x20623ee7 in open_file_for_scanning (file_handle=0x9fbff7c4) at 
> Zend/zend_language_scanner.c:2982
> #11 0x20623fce in compile_file (file_handle=0x9fbff7c4, type=2) at 
> Zend/zend_language_scanner.c:3068
> #12 0x2063cd1c in zend_execute_scripts (type=8, retval=0x0, 
> file_count=3) at Zend/zend.c:1093
> #13 0x2060a653 in php_execute_script (primary_file=0x9fbff7c4) at 
> main/main.c:1724
> 
> Brian

---

## #22335 — Rasmus Lerdorf — 2006-03-10T18:03:44+00:00

URL: https://externals.io/message/22317#22335 · In reply to: #22334

Rasmus Lerdorf wrote:
> After poking this a bit more, it looks like this whole block of code in 
> main.c is redundant.  Here is the story:
> 
> We start our journey on a dark and stormy night in June 2002 with this 
> bug:  http://bugs.php.net/17720
> 
> This led to this fix: 
> http://cvs.php.net/viewcvs.cgi/php-src/main/main.c?r1=1.461&r2=1.462
> which had this commit message:
> Main script should not be parsed when (include|require)_once()'ed, #17720
> # Derick, MFH?
> 
> Now, at the time this was the correct fix.  However, fast forward to 
> March 2005 and we have a patch from Stas to zend_execute_scripts:
> http://cvs.php.net/viewcvs.cgi/ZendEngine2/zend.c?view=diff&r1=1.297&r2=1.298 
> 
> which adds the opened file to the included_files list.  This means that 
> we are now trying to add the main script to the included_file list twice 
> which in itself is bad.
> 
> We'll test this a bit more, but it looks safe to remove that block of 
> code.  The test case in bug 17720 works fine without that block of code 
> in main.c and I see no other reason to have it there.

Just as a final follow-up to this.  Further testing showed that in CLI 
mode where we pass in a ZEND_HANDLE_FP instead of a ZEND_HANDLE_FILENAME 
we still need this block of code.  So the fix is to check and only skip 
this block if we are passed a ZEND_HANDLE_FILENAME.  Commit coming up.

-Rasmus
