fix Incorrect suffix check #61
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
phabricator/webroot/rsrc/externals/javelin/lib/Resource.js
Line 50 in 13264b9
fix the issue, replace the
indexOf
-based suffix check with a more robust approach. The best solution is to useString.prototype.endsWith
, which is specifically designed for this purpose and avoids the pitfalls ofindexOf
. IfendsWith
is not available in the runtime environment, explicitly handle the-1
case by checking the relative lengths of the strings.In this case, we will replace
path.indexOf('.css') == path.length - 4
withpath.endsWith('.css')
. This change ensures that the code correctly identifies whetherpath
ends with.css
without relying on potentially error-prone calculations.The
indexOf
andlastIndexOf
methods are sometimes used to check if a substring occurs at a certain position in a string. However, if the returned index is compared to an expression that might evaluate to -1, the check may pass in some cases where the substring was not found at all. Specifically, this can easily happen when implementingendsWith
usingindexOf
.Recommendation
Use
String.prototype.endsWith
if it is available. Otherwise, explicitly handle the -1 case, either by checking the relative lengths of the strings, or by checking if the returned index is -1.POC
The following uses
lastIndexOf
to determine if the stringx
ends with the stringy
:However, if
y
is one character longer thanx
, the right-hand sidex.length - y.length
becomes -1, which then equals the return value oflastIndexOf
. This will make the test pass, even thoughx
does not end withy
.To avoid this, explicitly check for the -1 case:
References
String.prototype.endsWith
String.prototype.indexOf
CWE-20