Skip to content

Adopt Punycode in HttpUrl on non-JVM platforms#7805

Merged
swankjesse merged 2 commits intomasterfrom
jwilson.0507.hook_up_punycode
May 8, 2023
Merged

Adopt Punycode in HttpUrl on non-JVM platforms#7805
swankjesse merged 2 commits intomasterfrom
jwilson.0507.hook_up_punycode

Conversation

@swankjesse
Copy link
Collaborator

No description provided.

@swankjesse swankjesse force-pushed the jwilson.0507.hook_up_punycode branch from 779e4b8 to 0770155 Compare May 7, 2023 15:13
Copy link
Collaborator

@yschimke yschimke left a comment

Choose a reason for hiding this comment

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

LGTM

Take it with a grain of salt, few beers in watching arsenal game with my son.

val result = idnToAscii(host) ?: return null
if (result.isEmpty()) return null

return if (result.containsInvalidHostnameAsciiCodes()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these two check needed?

Or should be postconditions via check?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, they’re needed. Java’s IDN ToAscii returns non-ascii characters if it doesn’t have a mapping!


@Test
fun hostnameLowercaseCharactersMappedDirectly() {
if (!isJvm) return // TODO: this test is broken on non-JVM platforms.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

@swankjesse swankjesse merged commit 5b5d312 into master May 8, 2023
@swankjesse swankjesse deleted the jwilson.0507.hook_up_punycode branch May 8, 2023 02:12
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.

2 participants