Skip to content

caddytls: skip idna.ToASCII for pure ASCII SNI values - #7770

Merged
steadytao merged 1 commit into
caddyserver:masterfrom
sleet0922:master
May 28, 2026
Merged

caddytls: skip idna.ToASCII for pure ASCII SNI values#7770
steadytao merged 1 commit into
caddyserver:masterfrom
sleet0922:master

Conversation

@sleet0922

@sleet0922 sleet0922 commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

SNI is always ASCII on the wire (RFC 6066), and most config
patterns are also ASCII. For pure ASCII input, idna.ToASCII
only validates and lowercases, which is equivalent to a simple
strings.ToLower. Add a fast path to avoid the overhead of
idna.ToASCII in the common case.

AI Assistance Disclosure

  • AI tools used: Claude
  • Usage scope: Vulnerability scanning and code optimization suggestions only
  • Verification: All code changes in this PR were manually implemented, reviewed, and tested by me. I take full responsibility for the correctness and originality of the work.

@sleet0922
caddytls: skip idna.ToASCII for pure ASCII SNI values
f6cdcf8 
SNI is always ASCII on the wire (RFC 6066), and most config
patterns are also ASCII. For pure ASCII input, idna.ToASCII
only validates and lowercases, which is equivalent to a simple
strings.ToLower. Add a fast path to avoid the overhead of
idna.ToASCII in the common case.
@CLAassistant

CLAassistant commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@steadytao

steadytao commented May 27, 2026

Copy link
Copy Markdown
Member

Weird that the CLA Assistant didn't do that automatically but rechecking PRs is simple enough.

@sleet0922 Please sign our CLA and add an AI assistance disclosure to your PR body (see PR template).

steadytao
steadytao approved these changes May 27, 2026
View reviewed changes

@steadytao steadytao left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks reasonable to me. Preserves the existing IDNA behaviour while avoiding extra work for pure ASCII names.

CLA + Assistance disclosure required before merge.

@steadytao steadytao added the do not merge ⛔ Not ready yet! label May 27, 2026
@sleet0922

sleet0922 commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

Looks reasonable to me. Preserves the existing IDNA behaviour while avoiding extra work for pure ASCII names.

CLA + Assistance disclosure required before merge.

Thanks for the review
I've updated the initial commit message, signed the CLA, and added the AI assistance disclosure to the PR body. The changes are ready for your recheck at your convenience.

@steadytao

steadytao commented May 27, 2026

Copy link
Copy Markdown
Member

Thank you, looks good. Will merge once CI is green.

@steadytao steadytao added optimization 📉 Performance or cost improvements and removed do not merge ⛔ Not ready yet! labels May 27, 2026
@steadytao
steadytao merged commit 86121c8 into caddyserver:master May 28, 2026
31 checks passed
This was referenced Jun 2, 2026
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@steadytao steadytao steadytao approved these changes

Assignees

No one assigned

Labels

optimization 📉 Performance or cost improvements

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sleet0922 @CLAassistant @steadytao