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

Allow any protocol in the duplicate slashes normalization #115

Conversation

gcox
Copy link
Contributor

@gcox gcox commented Jul 29, 2020

Fixes #114

Prior to this, duplicate slashes would be removed unless preceded by "http" or "https" protocols. This was too strict.

The regex was updated to allow for any 2+ character alphanumeric protocol. Went with requiring at least 2 characters since every scheme listed here (https://en.wikipedia.org/wiki/List_of_URI_schemes) is at least 2 characters.

gcox added 2 commits July 29, 2020 07:37
Prior to this, duplicate slashes would be removed unless preceeded by "http" or "https" protocols. This was too strict.
The regex was updated to allow for any 2+ character alphanumeric protocol. Went with requiring at least 2 characters since every scheme listed here (https://en.wikipedia.org/wiki/List_of_URI_schemes) is at least 2 characters.
@sindresorhus
Copy link
Owner

I think it should have a length limit too.

index.js Outdated Show resolved Hide resolved
@gcox
Copy link
Contributor Author

gcox commented Jul 29, 2020

@sindresorhus Length limit set to 30. The longest one I found on the above referenced Wiki was 27, so 30 seemed reasonable. However, given the completely arbitrary nature of such a limit, I'm thinking it should either be much larger or configurable. What do you think?

* Adds two new options, for min/max length of what the dupe slash removal regex recognizes as a protocol.
* Updates tests to exercise the new options
* Updates readme to document the new options
@gcox
Copy link
Contributor Author

gcox commented Jul 30, 2020

@sindresorhus Arbitrary bounds felt wrong so I went ahead and made this configurable.

@sindresorhus
Copy link
Owner

I'm not interested in having this configurable. The point is to prevent possible abuse. We can make it 50.

@sindresorhus sindresorhus changed the title Relaxes regex for removing duplicate slashes Allow any protocol in the duplicate slashes normalization Aug 2, 2020
@sindresorhus sindresorhus merged commit 14b79c6 into sindresorhus:master Aug 2, 2020
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

Successfully merging this pull request may close these issues.

Regex for removal of duplicate slashes not preceded by a protocol is too strict
2 participants