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

Add missing characters to URL regex #4710

Merged
merged 1 commit into from
Apr 21, 2022

Conversation

daniel-stoneuk
Copy link
Contributor

@daniel-stoneuk daniel-stoneuk commented Apr 19, 2022

Added the unreserved character ~ and the reserved characters '(), to the URL matching regex. If I'm understanding RFC 3986 correctly, "reserved" characters are allowed unencoded in URIs (& there already exist some of these characters in the regex) and so are "unreserved" characters like ~.

The tilde looks like its an unsafe character to have unencoded in RFC 1738, though it appears this is now superseded in most applications - and regardless, if this regex is only used to determine if a link is clickable I don't think there's much harm in including it. I'd love to get some more opinions on this.

This should fix #4705 and improve detection of clickable URLs. I have checked that this is the only place this REGEX string is used but since this is my first contribution, please can someone else check this as well.

Changelog(Fixes): Fixed an issue where URLs in JSON responses were not properly clickable if they included certain unreserved characters

Copy link
Contributor

@johnwchadwick johnwchadwick left a comment

Choose a reason for hiding this comment

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

Looks like it works for me.

Before:
screenshot showing a hyperlink cut off in Insomnia (develop)

After:
screenshot showing a hyperlink that is properly linked in Insomnia (#4710)

I don't see a reason why this would break anything either. Obviously, it will break if a URL is followed directly by any of the newly added characters, but that's not a big deal I don't think, especially given that in Insomnia, we're mostly dealing with URLs inside machine-readable formats.

LGTM.

@daniel-stoneuk
Copy link
Contributor Author

Apologies for the force push. I messed up trying to rebase on top of develop now that the directory these changes are in has been renamed 😨

@johnwchadwick
Copy link
Contributor

Apologies for the force push. I messed up trying to rebase on top of develop now that the directory these changes are in has been renamed fearful

Thanks for the rebase. Force pushing on PR branches is fine and not a problem in my opinion.

@johnwchadwick johnwchadwick enabled auto-merge (squash) April 21, 2022 17:56
*Added the unreserved character ~ and the reserved characters '(), to the URL matching regex since these can appear without being URL encoded.

*Added test for tilde in URL
@johnwchadwick johnwchadwick merged commit 874d494 into Kong:develop Apr 21, 2022
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.

URL in JSON response stops early if it contains a tilde (~)
3 participants