GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-137273: Fix debug assertion failure in locale.setlocale() on Windows by serhiy-storchaka · Pull Request #137300 · python/cpython

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented Aug 1, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

It happened when there were at least 16 characters after dot in the locale name.

  • Issue: locale.setlocale() crashes on Windows for long locale name #137273

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@serhiy-storchaka
serhiy-storchaka requested review from malemburg and zooba August 1, 2025 12:13
@serhiy-storchaka serhiy-storchaka added needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes labels Aug 1, 2025
@bedevere-app bedevere-app Bot added the awaiting core review label Aug 1, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Aug 1, 2025
locale.setlocale() crashes on Windows for long locale name #137273
Closed
@serhiy-storchaka
pythongh-137273: Fix debug assertion failure in locale.setlocale() on…
c6dbeb8 
… Windows

It happened when there were at least 16 characters after dot in the
locale name.
@serhiy-storchaka
serhiy-storchaka force-pushed the locale-long-encoding branch from bbff466 to c6dbeb8 Compare August 1, 2025 12:22
malemburg
malemburg requested changes Aug 1, 2025
View reviewed changes

@malemburg malemburg left a comment

Copy link
Copy Markdown
Member

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.

This fix should probably be limited to just a few Windows releases, unless Win11 also shows the same behavior.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Modules/_localemodule.c
size_t len = end ? (size_t)(end - locale) : strlen(locale);
const char *dot = memchr(locale, '.', len);
if (dot && locale + len - dot > 16) {
return -1;

@malemburg malemburg Aug 1, 2025

Copy link
Copy Markdown
Member

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.

Where did you get the "16" from ?
Please include some comments to explain where you got it from and why this check is necessary. Thanks.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@malemburg malemburg Aug 1, 2025

Copy link
Copy Markdown
Member

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.

BTW: It's better to make this limit configurable via a constant.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@zooba zooba Aug 1, 2025

Copy link
Copy Markdown
Member

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.

Comes from https://github.com/huangqinjin/ucrt/blob/d6e817a4cc90f6f1fe54f8a0aa4af4fff0bb647d/inc/corecrt_internal.h#L401, though I don't think that constant is available for us at compile time (the internal headers for the CRT are, well, internal).

A constant at least gives it a name though, and using MAX_CP_LEN may help someone find an updated definition in the future.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gpshead reacted with heart emoji
All reactions
  • ❤️ 1 reaction

@malemburg malemburg Aug 1, 2025

Copy link
Copy Markdown
Member

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.

Looks good now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting changes and removed awaiting core review labels Aug 1, 2025
@bedevere-app

bedevere-app Bot commented Aug 1, 2025

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka
Add more tests and comments.
3031948
@serhiy-storchaka

serhiy-storchaka commented Aug 1, 2025

Copy link
Copy Markdown
Member Author

How to check the Windows version? I do not have Windows 11, can you please test this PR on Windows 11?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@zooba

zooba commented Aug 1, 2025

Copy link
Copy Markdown
Member

How to check the Windows version?

I wouldn't bother, this limit has been around forever. The bigger challenge would be how to handle @<extras> if/when support is added, because right now there's no support for them in UCRT at all.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@malemburg

malemburg commented Aug 1, 2025

Copy link
Copy Markdown
Member

How to check the Windows version? I do not have Windows 11, can you please test this PR on Windows 11?

Sorry, but I don't have a Windows 11 build system available at the moment (not even a working Win10 system, after the VM crashed some time ago).

Let's not worry about this now. We can disable things again selectively, once the Windows SDK provides better support for handling locale names (perhaps in a few years).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka

serhiy-storchaka commented Aug 1, 2025

Copy link
Copy Markdown
Member Author

The bigger challenge would be how to handle @<extras> if/when support is added, because right now there's no support for them in UCRT at all.

I am planning to handle this in Python code. Translate "zh_HK.UTF-8@hans" to "zh-Hans-HK.utf-8", etc.
zooba reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka
MAX_ENCODING_SIZE -> MAX_CP_LEN
eae0bc4
@malemburg

malemburg commented Aug 1, 2025

Copy link
Copy Markdown
Member

I am planning to handle this in Python code. Translate "zh_HK.UTF-8@hans" to "zh-Hans-HK.utf-8", etc.

Is that how Windows handles modifiers ?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka

serhiy-storchaka commented Aug 1, 2025

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting change review and removed awaiting changes labels Aug 1, 2025
@bedevere-app

bedevere-app Bot commented Aug 1, 2025

Copy link
Copy Markdown

Thanks for making the requested changes!

@malemburg: please review the changes made to this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app
bedevere-app Bot requested a review from malemburg August 1, 2025 13:34
malemburg
malemburg approved these changes Aug 1, 2025
View reviewed changes
Comment thread Modules/_localemodule.c
size_t len = end ? (size_t)(end - locale) : strlen(locale);
const char *dot = memchr(locale, '.', len);
if (dot && locale + len - dot > 16) {
return -1;

@malemburg malemburg Aug 1, 2025

Copy link
Copy Markdown
Member

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.

Looks good now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting change review labels Aug 1, 2025
@zooba

zooba commented Aug 1, 2025

Copy link
Copy Markdown
Member

Is that how Windows handles modifiers ?

No - Windows UCRT treats them as part of the code page name and overruns its own buffers, leading to a crash 😉

I don't think there's any support for modifiers in locales at all. The OS itself has always used a different system for those AFAICT, it's just the POSIX emulation in the CRT that's trying to fudge over it.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@malemburg

malemburg commented Aug 1, 2025

Copy link
Copy Markdown
Member

I don't think there's any support for modifiers in locales at all.

In that case, it's probably best to simply strip from them locale strings passed to setlocale() on Windows. Esp. since the meaning of those modifiers is not standardized anywhere, AFAIK.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka

serhiy-storchaka commented Aug 1, 2025

Copy link
Copy Markdown
Member Author

Is that how Windows handles modifiers ?

Yes. It supports format language[-script][-region][.codepage] for BCP-47 locale names.

I seen also sr_Latn_RS instead of sr_RS@latin on some of Posix platforms, so we should support this format not only on Windows. setlocale() should try several variants.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka
serhiy-storchaka merged commit 718e0c8 into python:main Aug 1, 2025
@miss-islington-app

miss-islington-app Bot commented Aug 1, 2025

Copy link
Copy Markdown

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting merge label Aug 1, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 1, 2025
@serhiy-storchaka @miss-islington
pythongh-137273: Fix debug assertion failure in locale.setlocale() on…
150fd4f 
… Windows (pythonGH-137300)

It happened when there were at least 16 characters after dot in the
locale name.
(cherry picked from commit 718e0c8)

Co-authored-by: Serhiy Storchaka <[email protected]>
@serhiy-storchaka
serhiy-storchaka deleted the locale-long-encoding branch August 1, 2025 14:44
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 1, 2025
@serhiy-storchaka @miss-islington
pythongh-137273: Fix debug assertion failure in locale.setlocale() on…
c6f0f5f 
… Windows (pythonGH-137300)

It happened when there were at least 16 characters after dot in the
locale name.
(cherry picked from commit 718e0c8)

Co-authored-by: Serhiy Storchaka <[email protected]>
@bedevere-app

bedevere-app Bot commented Aug 1, 2025

Copy link
Copy Markdown

GH-137305 is a backport of this pull request to the 3.14 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Aug 1, 2025
@bedevere-app

bedevere-app Bot commented Aug 1, 2025

Copy link
Copy Markdown

GH-137306 is a backport of this pull request to the 3.13 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Aug 1, 2025
serhiy-storchaka added a commit that referenced this pull request Aug 1, 2025
@miss-islington @serhiy-storchaka
[3.13] gh-137273: Fix debug assertion failure in locale.setlocale() o…
a32bd11 
…n Windows (GH-137300) (GH-137306)

It happened when there were at least 16 characters after dot in the
locale name.
(cherry picked from commit 718e0c8)

Co-authored-by: Serhiy Storchaka <[email protected]>
@serhiy-storchaka

serhiy-storchaka commented Aug 1, 2025

Copy link
Copy Markdown
Member Author

Thank you for your review, @malemburg and @zooba.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
@serhiy-storchaka @Agent-Hellboy
pythongh-137273: Fix debug assertion failure in locale.setlocale() on…
9882a99 
… Windows (pythonGH-137300)

It happened when there were at least 16 characters after dot in the
locale name.
vstinner pushed a commit that referenced this pull request Oct 7, 2025
@miss-islington @serhiy-storchaka
[3.14] gh-137273: Fix debug assertion failure in locale.setlocale() o…
14e8dec 
…n Windows (GH-137300) (#137305)

gh-137273: Fix debug assertion failure in locale.setlocale() on Windows (GH-137300)

It happened when there were at least 16 characters after dot in the
locale name.
(cherry picked from commit 718e0c8)

Co-authored-by: Serhiy Storchaka <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malemburg malemburg malemburg approved these changes
@zooba zooba Awaiting requested review from zooba

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@serhiy-storchaka @zooba @malemburg
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.