Re: [ZEND-ENGINE-CVS] cvs: TSRM / tsrm_virtual_cwd.h

On December 12, 2002 12:40 am, Andi Gutmans wrote: > At 02:06 AM 12/11/2002 -0500, Ilia A. wrote: > >On December 11, 2002 12:16 am, Andi Gutmans wrote: > > > This is not the correct patch but adds other bugs. You should check for > > > UNC separetely in the code like in virtual_realpath(). > > > IS_ABSOLUTE_PATH() is only meant to detect the drive letter and is > > > always followed by use of COPY_WHEN_ABSOLUTE() (which happens to also > > > be 2 but it's still not right. Stuff might break). > > > >The patch only affects the behavior of IS_ABSOLUTE_PATH on Win32 where > >UNC path is absolute. PHP's behavior is the same when either > > IS_ABSOLUTE_PATH or IS_UNC_PATH occur. Separation of the two, only adds > > another len >= 2 check, beyond that nothing else changes. I do not see > > how this can break any existing code on Win32 platform, perhaps you could > > offer me some hints about situations where this may cause problems? > >I've looked at the usage of IS_ABSOLUTE_PATH macro and it is only used 4 > >times > >outside of TSRM/tsrm_virtual_cwd.c, where it only used in 2 functions. > > From what I've been able to see the change does not break any existing > > code. It can however prevent problems for paths on Win32 that begin with > > \\ or //, since unless the UNC check is explicitly done the code will try > > to 'resolve' those paths breaking them in the process. > > I didn't mean it'll break things right away. I'm just saying the fact that > by chance sizeof("c:") == sizeof("\\") should not be taken advantage of by > the code (IMO). It means the code doesn't stay very clean and if we ever > make a change there things might break. I think there is another possible danger here, at the moment very few pieces of PHP code rely on the IS_ABSOLUTE_PATH macro and that had already lead to one problem, which this bug had fixed because UNC paths were accounted for. If this macro becomes more commonly used this may become even a larger problem. Perhaps, an alternative to changing how IS_ABSOLUTE_PATH() works, we could introduce IS_ABSOLUTE_OR_UNC_PATH macro (IS_ABSOLUTE_PATH() || IS_UNC_PATH), so where UNC paths may be of concern this would be used instead of IS_ABSOLUTE_PATH(), on non-win32 systems this macro would work in the same manner as IS_ABSOLUTE_PATH(). Ilia