Skip to content

Commit

Permalink
ensureSlash: Fix accidental string-to-NaN coercion (#4424)
Browse files Browse the repository at this point in the history
Summary:
The `hasSlash` method uses `path.substr(path, path.length - 1)` to
remove the last character from `path`. Clearly, the first parameter is
suspect; it should be `0`. The code works as written, but only very
accidentally: the first parameter is coerced by `ToNumber` to `NaN`,
which is then coerced by `ToInteger` to `+0`, per [the spec][1].

[1]: https://www.ecma-international.org/ecma-262/6.0/#sec-string.prototype.substr

Test Plan:
Reading the spec should be sufficient. To verify in the Real World:
```js
const path = "has-slash-but-does-not-need-slash/"
const a = path.substr(path, path.length - 1);
const b = path.substr(0, path.length - 1);
console.log(a === b);  // true
console.log(a);        // has-slash-but-does-not-need-slash
```

wchargin-branch: ensureslash-accidental-coercion
  • Loading branch information
wchargin authored and Timer committed May 8, 2018
1 parent e5e9f59 commit 3aaddef
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion packages/react-scripts/config/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const envPublicUrl = process.env.PUBLIC_URL;
function ensureSlash(path, needsSlash) {
const hasSlash = path.endsWith('/');
if (hasSlash && !needsSlash) {
return path.substr(path, path.length - 1);
return path.substr(0, path.length - 1);
} else if (!hasSlash && needsSlash) {
return `${path}/`;
} else {
Expand Down

0 comments on commit 3aaddef

Please sign in to comment.