Skip to content

decode non-UTF-8 strings as Latin-1 instead of panicking - #25

Merged
evmar merged 2 commits into
evmar:mainfrom
LinusU:lu-latin1-strings
Sep 17, 2026
Merged

evmar merged 2 commits into
evmar:mainfrom
LinusU:lu-latin1-strings

Conversation

@LinusU

@LinusU LinusU commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

This fixes Moto Racer which has a copyright symbol in the window title 😁

I think that using a Cow was quite smart, and it is a quite small patch. But potentially we'd want to always read the strings as Windows-1252 maybe? 🤔

From Claude Fable 5.1 (high):

Programs of this era use Windows-1252, and a window title with a copyright sign in it took read_str's from_utf8 unwrap down. read_str now returns a Cow: borrowed for valid UTF-8, an owned Latin-1 decode otherwise.

The first commit adds Memory::read_cstr and moves the byte-oriented callers onto it (lstrlen/lstrcpy/lstrcat, wsprintfA, the NLS length computations), so decoding never changes a length or re-encodes a copy.

@evmar

evmar commented Sep 16, 2026

Copy link
Copy Markdown
Owner

I think having read_cstr return &[u8] is right. The read_str thing returning a UTF-8 String was always a hack, and the .unwrap() in there was intended exactly to help notice when this case came up.

My understanding is that the fully-correct thing to do is obey the current locale. I doubt we'd ever care to implement such a thing though. So maybe the main concern is making it easy to find this if anyone ever does care? With that framing, I think the .read_str() is sort of a "this is a convenient but technically wrong" method, which means having it return a String is probably better, since any caller that really cares about correctness will use some locale-aware method.

This is all an incredibly tiny detail and what you have is probably fine too. If it's easy to switch to non-Cow I'd prefer it, what do you think?

(Also, somehow I woke up this morning and thought "I haven't heard from Linus in a while, wonder what's going on", then opened GitHub and saw I'd somehow missed all of these, sorry for the delay!)

LinusU and others added 2 commits September 16, 2026 21:16
lstrlenA, lstrcpyA, lstrcatA, wsprintfA and the NLS length computations
work on bytes, so hand them the bytes instead of going through a &str
and back. This keeps them exact once read_str starts decoding text that
isn't UTF-8, which would otherwise change lengths and re-encode copies.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Programs of this era use the Windows-1252 code page; a window title with
a copyright sign in it took read_str's from_utf8 unwrap down. Decode
such bytes as Latin-1 instead.

read_str returns a String: it is the convenient reading, not the
correct one, and a caller that cares about the encoding has read_cstr.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@LinusU

LinusU commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Yeah, makes sense! I pushed an update.

Let me know if you want me to log something in the Err-path, e.g. when a program has a copyright-symbol in the titlebar 😁

From Fable 5.1:

Switched read_str to return a String and dropped the into_owned calls, with a doc comment saying it's the convenient reading and pointing at read_cstr for anyone who cares about the encoding. One thing the fallback loses is the unwrap's "notice when this happens" signal; happy to add a log::debug on the Latin-1 path if you'd like that kept.

(No worries on the delay! Glad you thought to check in 🙌)

@evmar
evmar merged commit 8ad1b7d into evmar:main Sep 17, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants