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.

Sign up for GitHub

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

Jump to bottom

Maximum length IDNs are not usable #554

Closed
Avamander opened this issue Nov 20, 2022 · 2 comments
Closed

Maximum length IDNs are not usable #554

Avamander opened this issue Nov 20, 2022 · 2 comments
Labels
bug Something isn't working. ready-for-release Feature is implemented and available for testing in dev branch. It will be included in the next rele

Comments

@Avamander
Copy link
Contributor

Describe the bug

When an IDN is maximum length allowed, it will be rejected by maddy, even if it's otherwise technically usable and allowed.

For example xn--oaoaaaoaoaoaooaoaoiuaiauiuaiauaaa-f1cadccdcmd01eddchqcbe07a.tld, this is not a real domain just reproduces the problem. It just reaches the maximum length of 63 characters allowed for a label in punycode.

Steps to reproduce

Add a maximum length IDN to local_domains

Log files

/data/maddy.conf:95: invalid source routing rule: äõäoaõoäaõaäõaoäaoaäõoaäooaoaoiuaiauäõiuüõaõäiauõaaa.tld

Environment information

  • maddy version: 0.6.2
@Avamander Avamander added the bug Something isn't working. label Nov 20, 2022
@foxcpp
Copy link
Owner

foxcpp commented Nov 26, 2022 •
edited
Loading

Interesting. Looks like maddy incorrectly checks the length in bytes for U-labels (that is, UTF-8) form instead of encoding to Punycode and checking that instead.

Byte length of UTF-8 encoded string 'äõäoaõoäaõaäõaoäaoaäõoaäooaoaoiuaiauäõiuüõaõäiauõaaa' is 71.

foxcpp added a commit that referenced this issue Jan 8, 2023
@foxcpp
Improve domain validation function
e6518a0 
Now FQDNs are accepted as well (with trailing dot).
Empty strings are not considered valid.
Label length for IDNs is checked using Punycode form as it should.

See #554
@foxcpp foxcpp added the ready-for-release Feature is implemented and available for testing in dev branch. It will be included in the next rele label Jan 8, 2023
@foxcpp
Copy link
Owner

foxcpp commented Jan 8, 2023

Pushed e6518a0 to fix this.

@foxcpp foxcpp closed this as completed Jan 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working. ready-for-release Feature is implemented and available for testing in dev branch. It will be included in the next rele
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Avamander @foxcpp