Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Validator] Fix validation for single level domains#43876

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:4.4fromHypeMC:url-validator-fix
Nov 29, 2021

Conversation

@HypeMC
Copy link
Member

@HypeMCHypeMC commentedNov 1, 2021
edited by nicolas-grekas
Loading

QA
Branch?4.4
Bug fix?yes
New feature?no
Deprecations?no
TicketsFix#43868,#44252
LicenseMIT
Doc PR-

Not sure if there are any other test cases that could be added, but this seems to fix the problem.

@nicolas-grekas
Copy link
Member

This looks strange to me because this means that['http://☎a'] is a valid domain name, while['http://☎'] is invalid...

@welcoMattic
Copy link
Member

This looks strange to me because this means that['http://☎a'] is a valid domain name, while['http://☎'] is invalid...

@nicolas-grekas wasn't this case fixed by#44119?

@nicolas-grekas
Copy link
Member

#44119 is on HttpClient+Mime, this one is on Validator, so I'm not sure.

@welcoMattic
Copy link
Member

Yes you are right, at the Validator level maybe we don't have to worry about thexn-- URL format?
If so, your concern is still legitimate

@HypeMC
Copy link
MemberAuthor

@nicolas-grekas Sorry, I missed your earlier comment, will check it out later today.

@nicolas-grekas
Copy link
Member

Looks like#44252 could provide us with another test case, can you please incorporate it here also?

HypeMC reacted with thumbs up emoji

@HypeMCHypeMCforce-pushed theurl-validator-fix branch 2 times, most recently from829372b to6dc7c87CompareNovember 29, 2021 07:34
@HypeMC
Copy link
MemberAuthor

I've split the domain part into two cases:

  1. single-level domain, the assumption here is that since there's no TLD, it's an internal domain, allows symbols, hyphens etc..
  2. multi-level domain, has a TLD. Since I haven't been able to find any clear information on which characters are allowed in a TLD I went with hyphens allowed, but not at the end, no symbols

Of course, this won't work if you have a multi-level internal domain, egsubdomain.internal☎. Any suggestions would be appreciated.

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

I've update the regexp to not allow unicode chars in local domain names
I've also split it to deal with punycode separately from unicode global domain names,
and ensured thathttp://বিডিআইএ.বাংলা is accepted (adding\pM to the regexp) for unicode global domain names, since this host exists.

HypeMC reacted with thumbs up emoji
@nicolas-grekasnicolas-grekasforce-pushed theurl-validator-fix branch 2 times, most recently from8b29f2c to5cbd90eCompareNovember 29, 2021 14:59
@nicolas-grekas
Copy link
Member

Thank you@HypeMC.

@nicolas-grekasnicolas-grekas merged commit382cde2 intosymfony:4.4Nov 29, 2021
@HypeMCHypeMC deleted the url-validator-fix branchNovember 29, 2021 15:10
This was referencedNov 29, 2021
This was referencedDec 29, 2021
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@stofstofstof left review comments

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@GromNaNGromNaNGromNaN approved these changes

@welcoMatticwelcoMatticwelcoMattic approved these changes

@derrabusderrabusderrabus approved these changes

@chalasrchalasrchalasr approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

8 participants

@HypeMC@nicolas-grekas@welcoMattic@GromNaN@stof@derrabus@chalasr@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp