Skip to content
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

Potential problem on stripEndingSlash function #62

Open
MCTaylor17 opened this issue Nov 14, 2019 · 0 comments
Open

Potential problem on stripEndingSlash function #62

MCTaylor17 opened this issue Nov 14, 2019 · 0 comments

Comments

@MCTaylor17
Copy link

MCTaylor17 commented Nov 14, 2019

Problem

Note the following code found in /src/index.js

const stripLeadingSlash = s => s.indexOf('/') === 0 ? s.substring(1) : s
const stripEndingSlash = s => s.indexOf('/') === (s.length - 1) ? s.substring(0, s.length - 1) : s

The expression s.indexOf("/") returns the first index in the string. That means stripEndingSlash will only strip ending slashes from strings with exactly one slash. A string containing two slashes will return with the trailing slash still in place.

Example

Consider the following cases:

const singleSlash = "index.js/"
// singleSlash.indexOf('/') equals 8
// 8 equals (singleSlash.length -1);
// Slash is removed
console.log(stripEndingSlash(singleSlash)); 
// "index.js"
const multiSlash = "/src/index.js/";
// multiSlash.indexOf('/') equals 0
// 0 doesn't equal (multiSlash.length -1)
// Slash is not removed
console.log(stripEndingSlash(multiSlash)); 
// "/src/index.js/"

Suggestion

You could fix this by changing indexOf to lastIndexOf, however, using charAt provides a more readable and performant expression:

const stripLeadingSlash = s => s.charAt(0) === "/" ? s.substring(1) : s
const stripEndingSlash = s => s.charAt(s.length - 1) === "/" ? s.substring(0, s.length - 1) : s
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant