Skip to content

idn: avoid allocations and wcslen on Windows - #19798

Closed
vszakats wants to merge 1 commit into
curl:masterfrom
vszakats:widnopt
Closed

vszakats wants to merge 1 commit into
curl:masterfrom
vszakats:widnopt

Conversation

@vszakats

@vszakats vszakats commented Dec 1, 2025 •

Copy link
Copy Markdown
Member

Eliminate a heap buffer in both win32_idn_to_ascii() and
win32_ascii_to_idn(), by replacing it with stack buffer. The maximum
size is fixed in these cases, and small enough to fit there.

Also reuse length returned by the UTF-8 to wchar conversion, allowing
to drop wcslen() call in both functions, and allowing to call
the wchar to UTF-8 conversion API WideCharToMultiByte() with the known
length, saving length calculations within that API too.

Ref: #19748 (comment)

@github-actions github-actions Bot added the name lookup DNS and related tech label Dec 1, 2025
@vszakats vszakats added the Windows Windows-specific label Dec 1, 2025
@vszakats vszakats closed this in 6694a42 Dec 3, 2025
@vszakats
vszakats deleted the widnopt branch December 3, 2025 13:51
@vszakats
vszakats requested a review from Copilot December 14, 2025 22:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR optimizes the Windows IDN (Internationalized Domain Name) conversion functions by eliminating heap allocations and redundant string length calculations. The changes replace dynamically allocated intermediate buffers with fixed-size stack buffers and reuse length information returned by Windows APIs to avoid calling wcslen().

Key changes:

  • Removed the helper function idn_curlx_convert_UTF8_to_wchar() that performed heap allocation
  • Modified idn_curlx_convert_wchar_to_UTF8() to accept an explicit character count parameter
  • Refactored win32_idn_to_ascii() and win32_ascii_to_idn() to use stack-allocated buffers and reuse length values

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

vszakats added a commit to vszakats/curl that referenced this pull request Dec 16, 2025
Add comments to clarify that a terminating null is always present in
the buffers returned to the caller.

The curl APIs `win32_idn_to_ascii()` or `win32_ascii_to_idn()` receive
a null-terminated UTF-8 string as input. They first convert it to wide
chars by first asking `MultiByteToWideChar()` to calculate the length,
by passing -1. This API returns the length with the null char included
(= `strlen() + 1`), does the conversion, with the output also
null-terminated. `IdnTo*()` preserve this null character as documented.
Then we pass this null-terminated, fixed-length buffer ito
`WideCharToMultiByte()`, which keeps preserving the null, ending up in
the buffer returned to the caller.

Refs:
https://learn.microsoft.com/windows/win32/api/stringapiset/nf-stringapiset-multibytetowidechar
https://learn.microsoft.com/windows/win32/api/stringapiset/nf-stringapiset-widechartomultibyte
https://learn.microsoft.com/windows/win32/api/winnls/nf-winnls-idntoascii
https://learn.microsoft.com/windows/win32/api/winnls/nf-winnls-idntounicode

WINE source code:
https://gitlab.winehq.org/wine/wine/-/blob/wine-10.20/dlls/kernelbase/locale.c
https://gitlab.winehq.org/wine/wine/-/blob/wine-10.20/dlls/ntdll/locale.c
https://gitlab.winehq.org/wine/wine/-/blob/wine-10.20/dlls/ntdll/locale_private.h

Ref: curl#19976 (comment)
Follow-up to 6694a42 curl#19798

Closes curl#19980
vszakats added a commit that referenced this pull request Jul 15, 2026
Also:
- curlx: pass this flag to the actual conversion calls, for consistency
  and robustness. (It's not stricly necessary because the initial call
  to determine size, with this flag passed, fails already on bad input.)
- schannel: unfold `MultiByteToWideChar()` line (formatting).

Ref: https://learn.microsoft.com/windows/win32/api/stringapiset/nf-stringapiset-multibytetowidechar

Follow-up to 6694a42 #19798

Closes #22326
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

name lookup DNS and related tech Windows Windows-specific

Development

Successfully merging this pull request may close these issues.

2 participants