[FIX] LibraryFormatter: Fix handling of paths containing special characters #622
Conversation
| const basePathPrefixRegExp = new RegExp(`^${posixBasePath}/?`); | ||
|
|
||
| if (!basePathPrefixRegExp.test(posixFsPath)) { | ||
| if (!posixFsPath.startsWith(posixBasePath)) { | ||
| if (posixBasePath === posixFsPath + "/") { |
There was a problem hiding this comment.
this condition suggests to me that posixBasePath always ends with a slash, but the test mocks the method with a return value without a trailing slash?
There was a problem hiding this comment.
As discussed we should ensure that posixBasePath ends with a slash to make this comparison stable
| if (namespacePath.startsWith("/")) { | ||
| // Remove any leading slash | ||
| namespacePath = namespacePath.substr(1); | ||
| } |
There was a problem hiding this comment.
What about the else case, isn't it an error? Depends on the question whether posixBasePath ends with a slash (then it's not an error) or not.
There was a problem hiding this comment.
Since we don't enforce the presence (or absence) of a trailing slash in the project path mapping, we need to deal with both cases. I updated my initial commit to deal with optional trailing slashes everywhere and make the comparison somewhat easier to understand
…acters FS paths containing characters which can be interpreted as regular expression metacharacters may lead to issues during the namespace detection of libraries. Alternatively we could switch to using https://www.npmjs.com/package/escape-string-regexp which we already use elsewhere. But I think this case here can be kept simple. Fixes SAP/ui5-tooling#526
Since we don't validate namespaces yet, they may contain regular expression metacharacters which can lead to unexpected behaviors when using them in a dynamically created RegExp instance.
RandomByte
force-pushed
the
fix-library-namespace-regex-paths
branch
from
044f088 to
a1a14dd
Compare
FS paths containing characters which can be interpreted as regular
expression metacharacters may lead to issues during the namespaces
detection of libraries.
Alternatively we could switch to using
https://www.npmjs.com/package/escape-string-regexp which we already use
elsewhere. But I think this case here can be kept simple.
Fixes SAP/ui5-tooling#526