Opened 4 years ago
Last modified 4 years ago
#51368 new defect (bug)
Remove windows path check from validate_file
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | Filesystem API | Keywords: | |
Focuses: | Cc: |
Description (last modified by )
Let's talk about validate_file
function, and specifically about this fragment here:
It checks whether or not the path is a Windows drive path. Why is this logic needed? It doesn't seem to play any role or even make sense - why allow arbitrary unix paths, but not windows paths? It was introduced 16 years ago when the function was first created, and even then there was no clear explanation why is it even being checked: [2019].
Attachments (2)
Change History (7)
#2
in reply to:
↑ description
@
4 years ago
- Description modified (diff)
Replying to zieladam:
It was introduced 16 years ago when the function was first created, and even then there was no clear explanation why is it even being checked: [2019].
Just noting the ':' === substr( $file, 1, 1 )
check itself is even older and comes from b2/cafelog:
https://core.trac.wordpress.org/browser/trunk/b2template.php?rev=3&marks=79-80#L68
That is the line that ended up in validate_file()
after being moved around quite a few times.
Looking at the "Sorry, can't call files with their real path" message, it was probably added as some sort of a security precaution, though perhaps no longer relevant.
#4
@
4 years ago
I think that rather than removing the windows check, we can just be a little more explicit in the checking of the file paths. I could go either way here.
#5
@
4 years ago
@whyisjake
The hard part here is that not only are all of these are valid paths on windows:
C:Projects\apilibrary\apilibrary.sln D:\FY2018 \\system07\C$\ \Server2\Share\Test\Foo.txt c:\temp\test-file.txt \\127.0.0.1\c$\temp\test-file.txt \\LOCALHOST\c$\temp\test-file.txt \\.\c:\temp\test-file.txt \\?\c:\temp\test-file.txt \\.\UNC\LOCALHOST\c$\temp\test-file.txt \\127.0.0.1\c$\temp\test-file.txt \?\Volume{b75e2c83-0000-0000-0000-602f00000000}\Test\Foo.txt
But also that:
On Windows, both slash (/) and backslash (\) are used as directory separator character. In other environments, it is the forward slash (/).
– PHP docs https://www.php.net/manual/en/function.basename.php
In this context, these are valid paths on both windows and linux:
//system07/C$/ //./c:/temp/test-file.txt //LOCALHOST/c$/temp/test-file.txt
This one is either absolute or relative depending on the context: C:Projects/apilibrary/apilibrary.sln
.
cc @dd32