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

[5.8] Accept underscores in the URL validator, as the URL will load #30417

Merged
merged 1 commit into from Oct 24, 2019

Conversation

@sgilberg
Copy link
Contributor

sgilberg commented Oct 24, 2019

We came across an issue in one of our Laravel apps (currently using 5.8) in which a valid URL was being treated as invalid by the 'url' validation rule, due to the presence of an underscore in the domain name (which, while unconventional, still works). Laravel's validateUrl() method documentation states that it is based on Symfony's UrlValidator. Symfony recently released a fix for this same issue here: symfony/symfony@ad89564 (see related discussion here: symfony/symfony#32506).

I copied the pattern update into Laravel's validateUrl() method and updated the corresponding tests accordingly.

(I realize we are just out of the published window for bug fixes on 5.8, but I'm hoping we can squeeze in one this small anyway, as it is compatible with the current release. I can resubmit to the 6.x branch if necessary.)

@taylorotwell taylorotwell merged commit 0cf3871 into laravel:5.8 Oct 24, 2019
2 checks passed
2 checks passed
continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@taylorotwell

This comment has been minimized.

Copy link
Member

taylorotwell commented Oct 24, 2019

Hello!

Could you send this to 6.x branch as well?

@sgilberg

This comment has been minimized.

Copy link
Contributor Author

sgilberg commented Oct 24, 2019

Gladly! #30418 Thanks so much @taylorotwell appreciate all the good work you do

@sgilberg

This comment has been minimized.

Copy link
Contributor Author

sgilberg commented Nov 5, 2019

Would it be possible to include this in a 5.8.36 release?

@driesvints

This comment has been minimized.

Copy link
Member

driesvints commented Nov 11, 2019

@sgilberg 5.8 isn't supported anymore unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.