-
Notifications
You must be signed in to change notification settings - Fork 731
fix(windows): preserve localized profile paths #1678
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
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| /** | ||
| * Decode bounded text emitted by Windows system tools. | ||
| * | ||
| * [Decision Log] | ||
| * - Purpose: preserve non-ASCII paths when a Windows tool writes the active | ||
| * legacy code page instead of UTF-8. | ||
| * - Existing constraint: generated service assets may be UTF-16, while | ||
| * redirected `schtasks` output follows the Windows locale on affected hosts. | ||
| * - Alternatives considered: replacement-character heuristics and a new | ||
| * iconv dependency. The former can reinterpret valid text; the latter widens | ||
| * the install/security surface for two small, already-supported codecs. | ||
| * - Choice: recognize UTF-16 first, accept only strict UTF-8 next, then use the | ||
| * locale-appropriate WHATWG decoder (CP949 through `euc-kr`, or Windows-1252 | ||
| * only for locales that actually use that family). Unknown/unsupported | ||
| * locales fail back to the old replacement-preserving UTF-8 result instead | ||
| * of guessing another code page or throwing in diagnostics. | ||
| * - Impact: decoding stays dependency-free and bounded, but this deliberately | ||
| * does not guess arbitrary OEM code pages that the runtime cannot identify. | ||
| */ | ||
|
|
||
| function trimWindowsText(value: string): string { | ||
| return value.replace(/^\uFEFF/, "").trim(); | ||
| } | ||
|
|
||
| function currentWindowsLocale(): string { | ||
| try { | ||
| return Intl.DateTimeFormat().resolvedOptions().locale; | ||
| } catch { | ||
| return "en-US"; | ||
| } | ||
| } | ||
|
|
||
| function decodeStrict(buffer: Uint8Array, encoding: string): string | null { | ||
| try { | ||
| return trimWindowsText(new TextDecoder(encoding, { fatal: true }).decode(buffer)); | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| function decodeUtf16Be(buffer: Uint8Array): string { | ||
| const payloadLength = buffer.length - 2; | ||
| const swapped = Buffer.alloc(payloadLength - (payloadLength % 2)); | ||
| for (let i = 2; i + 1 < buffer.length; i += 2) { | ||
| swapped[i - 2] = buffer[i + 1]!; | ||
| swapped[i - 1] = buffer[i]!; | ||
| } | ||
| return trimWindowsText(swapped.toString("utf16le")); | ||
| } | ||
|
|
||
| /** | ||
| * CP949 is exposed by the Encoding Standard under the `euc-kr` label. Keep the | ||
| * Western fallback deliberately narrow: treating CP932, CP1250, or CP1251 | ||
| * bytes as Windows-1252 can fabricate a different valid-looking filesystem | ||
| * path, which is worse than the previous replacement-character refusal. | ||
| */ | ||
| function legacyEncodingForLocale(locale: string): "euc-kr" | "windows-1252" | null { | ||
| const language = locale.trim().split(/[-_]/, 1)[0]?.toLowerCase(); | ||
| if (language === "ko") return "euc-kr"; | ||
| if (language && WINDOWS_1252_LANGUAGES.has(language)) return "windows-1252"; | ||
| return null; | ||
| } | ||
|
|
||
| const WINDOWS_1252_LANGUAGES = new Set([ | ||
| "af", "br", "ca", "co", "cy", "da", "de", "en", "es", "eu", "fi", "fo", "fr", | ||
| "ga", "gd", "gl", "id", "is", "it", "lb", "ms", "nl", "no", "oc", "pt", "sq", | ||
| "sv", "sw", | ||
| ]); | ||
|
|
||
| export interface WindowsTextDecodeOptions { | ||
| /** Test seam and explicit locale override; production uses the active Intl locale. */ | ||
| readonly locale?: string; | ||
| } | ||
|
|
||
| export function decodeWindowsTextBytes( | ||
| buffer: Uint8Array, | ||
| options: WindowsTextDecodeOptions = {}, | ||
| ): string { | ||
| if (buffer.length === 0) return ""; | ||
|
|
||
| const bomUtf16Le = buffer.length >= 2 && buffer[0] === 0xff && buffer[1] === 0xfe; | ||
| const bomUtf16Be = buffer.length >= 2 && buffer[0] === 0xfe && buffer[1] === 0xff; | ||
| const looksUtf16Le = buffer.length >= 4 | ||
| && buffer[1] === 0x00 | ||
| && buffer[3] === 0x00 | ||
| && buffer[0] !== 0x00; | ||
|
|
||
| if (bomUtf16Le || looksUtf16Le) { | ||
| return trimWindowsText(Buffer.from(buffer).toString("utf16le")); | ||
| } | ||
| if (bomUtf16Be) return decodeUtf16Be(buffer); | ||
|
|
||
| const utf8 = decodeStrict(buffer, "utf-8"); | ||
| if (utf8 !== null) return utf8; | ||
|
|
||
| const locale = options.locale ?? currentWindowsLocale(); | ||
| const legacyEncoding = legacyEncodingForLocale(locale); | ||
| if (legacyEncoding !== null) { | ||
| const legacy = decodeStrict(buffer, legacyEncoding); | ||
| if (legacy !== null) return legacy; | ||
| } | ||
|
|
||
| // Preserve the previous fail-soft behavior when the runtime lacks a codec or | ||
| // the bytes are malformed even for the selected Windows code page. | ||
| return trimWindowsText(new TextDecoder("utf-8").decode(buffer)); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not select Windows-1252 when locale detection fails.
If
Intl.DateTimeFormat()throws,currentWindowsLocale()returns"en-US".legacyEncodingForLocale("en-US")then selects"windows-1252". Windows-1252 accepts arbitrary byte values, so CP949 or unsupported-codepage output can be silently corrupted instead of retaining the replacement-preserving UTF-8 fallback described insrc/lib/windows-text.ts.Return
nullfrom the failure path. Skip legacy decoding when no locale is available.Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents