-
-
Notifications
You must be signed in to change notification settings - Fork 620
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support server paths #1136
Support server paths #1136
Conversation
@@ -67,6 +67,14 @@ int path_getrelative(lua_State* L) | |||
return 1; | |||
} | |||
|
|||
/* Relative paths within a server can't climb outside the server root. | |||
* If the paths don't share server name, return the absolute path. */ | |||
if (src[0] == '/' && src[1] == '/' && last == 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel that this might overrun the buffer if one were to attempt to get a relative path for ""
. To avoid that, this could probably be:
if (src[0] != '\0' && src[0] == '/' && src[1] == '/' && last == 1) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But isn't src[0] != '\0' && src[0] == '/'
redundant? if src[0] == '/'
then it's guaranteed to not be 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, empty strings are covered in the previous check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, not really sure what I was thinking when I made that comment. Sorry about that!
/* add to the result, filtering out duplicate slashes */ | ||
if (ch != '/' || last != '/') { | ||
/* add to the result, filtering out duplicate slashes, except when they are leading slashes */ | ||
if (str == &source[1] || (ch != '/' || last != '/')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is the line I was meant to comment on. Is source
guaranteed to not be an empty string here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without testing, I believe so. if the string is empty, then str
will be equal to endPtr
and the loop condition while (str != endPtr)
will fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some testing I found out that it never attempt to normalize an empty string, thanks to
premake-core/src/host/path_normalize.c
Line 108 in a713ecc
while (*endPtr) { |
For context, see #1129
TLDR: Server paths (i.e. "//server/files") were treated like any non-normalized path with duplicate slashes.