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

fix: use more permissive URL pattern for Abide #9180 #11116

Merged

Conversation

ncoden
Copy link
Contributor

@ncoden ncoden commented Apr 1, 2018

Use a more permissive URL regex in Abide to allow all escaped characters and various others valid URLs.

Changes:

  • Use CommonRegexJS URL regex in Abide
  • Add FTPS and SFTP to accepted protocols in Abide url
  • Update Abide documentation

Note: the new RegExp is close to an "Standard URL validation" and allow a lot of valid URLs that may have to be forbidden on the end website (intrants, local domains, utf8 & unicode characters...).
For more restrictive RegExps, see https://mathiasbynens.be/demo/url-regex.

Closes #9180

Verified

This commit was signed with the committer’s verified signature.
thecsw Sandy
Note: the new RegExp is close to an "Standard URL validation" and allow a lot of valid URLs that may have to be forbidden on the end website (intrants, local domains, utf8 & unicode characters...).
For more restrictive RegExps, see https://mathiasbynens.be/demo/url-regex.

Closes foundation#9180
From https://github.com/talyssonoc/CommonRegexJS/blob/e2901b9f57222bc14069dc8f0598d5f412555411/lib/commonregex.js#L76
@@ -321,8 +321,13 @@ cvv : /^([0-9]){3,4}$/,

// http://www.whatwg.org/specs/web-apps/current-work/multipage/states-of-the-type-attribute.html#valid-e-mail-address
email : /^[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)+$/,
}[a-zA-Z0-9])?)+$/,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this line added by accident?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess so

// From CommonRegexJS (@talyssonoc)
// https://github.com/talyssonoc/CommonRegexJS/blob/e2901b9f57222bc14069dc8f0598d5f412555411/lib/commonregex.js#L76
// For more restrictive URL Regexs, see https://mathiasbynens.be/demo/url-regex.
url: /^((?:(https?|ftp|file|ssh):\/\/|www\d{0,3}[.]|[a-z0-9.\-]+[.][a-z]{2,4}\/)(?:[^\s()<>]+|\((?:[^\s()<>]+|(?:\([^\s()<>]+\)))*\))+(?:\((?:[^\s()<>]+|(?:\([^\s()<>]+\)))*\)|[^\s`!()\[\]{};:\'".,<>?\xab\xbb\u201c\u201d\u2018\u2019]))$/,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about sftp and ftps? Or should these not be covered here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were not accepted before. but we can add them

@DanielRuf
Copy link
Contributor

Rest looks good so far accoding to some regex tests.

ncoden added 2 commits April 3, 2018 11:26

Verified

This commit was signed with the committer’s verified signature.
thecsw Sandy

Verified

This commit was signed with the committer’s verified signature.
thecsw Sandy
@ncoden ncoden merged commit ab24073 into foundation:develop Apr 3, 2018
@ncoden ncoden deleted the fix/abide-permissive-url-regexp-9180 branch April 7, 2018 17:44
ncoden added a commit to ncoden/foundation-sites that referenced this pull request Jun 16, 2018

Verified

This commit was signed with the committer’s verified signature.
thecsw Sandy
…l-regexp-9180 for v6.5.0

724545c fix: use more permissive URL pattern for Abide foundation#9180
01dde90 docs: remove line added by accident in Abide regex docs
db8d81f feat: add SFTP & FTPS to Abide valid urls protocols

Signed-off-by: Nicolas Coden <nicolas@ncoden.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Abide] URL validation fails when URL encoded commas are in the URL.
2 participants