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

Handle empty domain segments correctly in idna #256

Closed
wants to merge 1 commit into from

Conversation

@Manishearth
Copy link
Member

Manishearth commented Dec 18, 2016

Spec asks us to "Otherwise join the labels using U+002E FULL STOP as a separator". The current code checking if the result was non-empty would fail if the segments themself were non-empty. It should be equivalent to .split('.').map(.. processing ..).join('.'). Implementing it procedurally since the code is already pretty procedural and it avoids extra allocations.

Should processing() return an iterator? We seem to be needlessly deconstructing and reconstructing the same string twice. OTOH that would mean more allocations.

r? @SimonSapin

fixes #255


This change is Reviewable

Spec asks us to "Otherwise join the labels using U+002E FULL STOP as a
separator". The current code checking if the result was non-empty would
fail if the segments themself were non-empty. It should be equivalent to
`.split('.').map(.. processing ..).join('.')`. Implementing it
procedurally since the code is already pretty procedural.
@SimonSapin
Copy link
Member

SimonSapin commented Dec 19, 2016

Closing in favor of #171, I’ll comment there.

@SimonSapin SimonSapin closed this Dec 19, 2016
@Manishearth Manishearth deleted the Manishearth:dotdot branch Dec 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.