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 url parse on markdown formatted urls #2036

Merged
merged 2 commits into from
Jan 28, 2016
Merged

Fix url parse on markdown formatted urls #2036

merged 2 commits into from
Jan 28, 2016

Conversation

jf-guillou
Copy link
Contributor

#1857 allowed wider fragment matches, but breaks auto url embed with markdown urls. See #2034

This pr attemps to improve this behaviour.

Previously :

"[stream](https://www.twitch.tv/stream)".match(/([A-Za-z]{3,9}):\/\/([-;:&=\+\$,\w]+@{1})?([-A-Za-z0-9\.]+)+:?(\d+)?((\/[-\+=!:~%\/\.@\,\w]+)?\??([-\+=&!:;%@\/\.\,\w]+)?#?([^\s]+)?)?/)
https://www.twitch.tv/stream)

With this pr :

"[stream](https://www.twitch.tv/stream)".match(/([A-Za-z]{3,9}):\/\/([-;:&=\+\$,\w]+@{1})?([-A-Za-z0-9\.]+)+:?(\d+)?((\/[-\+=!:~%\/\.@\,\w]+)?\??([-\+=&!:;%@\/\.\,\w]+)?(?:#([^\s]+))?)?/)
https://www.twitch.tv/stream

Fragments still works :

"https://www.twitch.tv/stream#with/fragment".match(/([A-Za-z]{3,9}):\/\/([-;:&=\+\$,\w]+@{1})?([-A-Za-z0-9\.]+)+:?(\d+)?((\/[-\+=!:~%\/\.@\,\w]+)?\??([-\+=&!:;%@\/\.\,\w]+)?(?:#([^\s]+))?)?/)
https://www.twitch.tv/stream#with/fragment

But markdown format with fragments are still failing :

"[stream](https://www.twitch.tv/stream#with/fragment)".match(/([A-Za-z]{3,9}):\/\/([-;:&=\+\$,\w]+@{1})?([-A-Za-z0-9\.]+)+:?(\d+)?((\/[-\+=!:~%\/\.@\,\w]+)?\??([-\+=&!:;%@\/\.\,\w]+)?(?:#([^\s]+))?)?/)
https://www.twitch.tv/stream#with/fragment)

I cannot find a way to correctly handle this case since parenthesis are allowed in fragments

Allows better matches on markdown formatted urls
@rodrigok
Copy link
Member

I think we can ignore the ) in fragments, so you can change (?:#([^\s]+))? to (?:#([^\s\)]+))?

@jf-guillou
Copy link
Contributor Author

Got to admit that ) in fragments is rare enough to be ignored

@rodrigok
Copy link
Member

LGTM

1 similar comment
@engelgabriel
Copy link
Member

LGTM

engelgabriel added a commit that referenced this pull request Jan 28, 2016
Fix url parse on markdown formatted urls
@engelgabriel engelgabriel merged commit 6d7cbbd into RocketChat:develop Jan 28, 2016
@jf-guillou jf-guillou deleted the urlregex branch January 28, 2016 13:35
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.

3 participants