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

On December 14, 2002 09:53 am, Andi Gutmans wrote: > At 09:41 AM 12/12/2002 -0500, Ilia A. wrote: > >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(). > > Are there places which need to check for c:\ and not UNC which you know of? As far as I can tell on both checks should be done together always. Ilia