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

Conform to IDNA version 15.1.0 revision 31 #454

Merged
merged 1 commit into from Apr 12, 2024

Conversation

TRowbotham
Copy link
Contributor

This adds the necessary changes to conform to IDNA version 15.1.0 revision 31.

Notable Changes

  • Transitional processing (the default in PHP) is now deprecated. No deprecation notices were added as PHP does not yet report a deprecation notice in this case.
  • An error is no longer recorded for characters with a status of disallowed.
  • When performing code point mapping and transitional processing is enabled the code point U+1E9E capital sharp s (ẞ), is replaced by the string “ss”
  • A new internal option "IgnoreInvalidPunycode" was added, which is supposed to allow for an all-ASCII fast-path, however, the official tests do not test this configuration option.

@derrabus
Copy link
Member

derrabus commented Nov 7, 2023

Can we add a test?

@stof
Copy link
Member

stof commented Nov 7, 2023

I don't think we should add options that don't exist in PHP. This would defeat the purpose of this library.

@TRowbotham
Copy link
Contributor Author

I have decided to remove the "IgnoreInvalidPunycode" option as suggested since the option is not available in PHP.

src/Intl/Idn/Idn.php Outdated Show resolved Hide resolved
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.
Can you please add some tests?

src/Intl/Idn/Idn.php Outdated Show resolved Hide resolved
src/Intl/Idn/Idn.php Outdated Show resolved Hide resolved
src/Intl/Idn/Idn.php Outdated Show resolved Hide resolved
Copy link
Member

@nicolas-grekas nicolas-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 fixed my CS comments by force-pushing on your fork.
Tests would still be welcome. And I have one question (see inline comments).
Are you still interested in finishing this PR?

@@ -280,10 +280,6 @@ private static function mapCodePoints($input, array $options, Info $info)

switch ($data['status']) {
case 'disallowed':
$info->errors |= self::ERROR_DISALLOWED;
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reasoning for this change according to the spec is "Checking for disallowed characters both before and after normalization yielded inconsistent results for unnormalized vs. normalized text".

@TRowbotham
Copy link
Contributor Author

I fixed my CS comments by force-pushing on your fork. Tests would still be welcome. And I have one question (see inline comments). Are you still interested in finishing this PR?

Yes, sorry, haven't had a lot of free time lately. I'll try to put some tests together this week.

Thanks for taking care of the style issues.

@nicolas-grekas
Copy link
Member

Thank you @TRowbotham.

@nicolas-grekas nicolas-grekas merged commit 1100c07 into symfony:1.x Apr 12, 2024
6 of 10 checks passed
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.

None yet

4 participants