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 — bpo-34481: Fix surrogate-handling in strftime by pganssle · Pull Request #8983 · python/cpython

Conversation

@pganssle

@pganssle pganssle commented Aug 28, 2018 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

This is intended to be re-based against #8878 - that PR adds tests, this one adds the fix.

I am making this PR before that one is merged mainly because this is some crazy platform-specific behavior, and I want to test it on the full cross-platform test suite.

https://bugs.python.org/issue34481

https://bugs.python.org/issue34481

Sorry, something went wrong.

Uh oh!

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

All reactions
Alexey Izbyshev and others added 6 commits August 23, 2018 20:46
Alexey Izbyshev
bpo-34482: Add tests for proper handling of non-UTF-8-encodable strin…
97037bd
…gs in datetime classes

A follow-up of bpo-34454.
@pganssle
Fix error with surrogate characters in strftime
ee6227b
@pganssle
Update strftime surrogate tests
bc13193
Now that the bug is fixed, the try/catch can be removed.
@pganssle
Fix surrogate handling in Z-replacement
8029dd1
This is now also a more complete fix for bpo-6697.
@pganssle
Specify caught error when wrapping
6494aae
@pganssle
Encode to utf-8 rather than locale
3973be1
This will cut down on some of the per-environment variability of this
function.
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Aug 28, 2018
@bedevere-bot bedevere-bot added the awaiting review label Aug 28, 2018
pganssle added 4 commits August 29, 2018 12:12
@pganssle
Revert "Encode to utf-8 rather than locale"
667a9c6
This reverts commit 3973be1.
@pganssle
Disable wcsftime
c1381d5
@pganssle
Fall back to unicode escape after locale encoding
b643da8
This passes a backslash-escaped unicode string to strftime in the event
that locale encoding fails. To ensure that the relevant string is
round-trippable, all backslashes in the original string are
double-escaped, and then unescaped when the result is decoded.
@pganssle
Revert "Disable wcsftime"
9d34c69
This reverts commit c1381d5.
@pganssle

pganssle commented Aug 30, 2018

Copy link
Copy Markdown
Member Author

I have no idea how to reproduce this failure. It works fine on my home computer (Arch Linux), on a debian VPS and a debian WSL. All are succeeding.

I think that this will start succeeding on Linux as well if I re-enable wcsftime, but I'm quite concerned that it's failing in a way that I don't understand and can't reproduce.

All reactions

Sorry, something went wrong.

Uh oh!

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

@pganssle

pganssle commented Sep 4, 2018

Copy link
Copy Markdown
Member Author

I'm able to reproduce this issue only on Ubuntu 14. For some reason with '\ud800', this function is not raising an error in Ubuntu 14. However, a simple POC:

#include <stdlib.h>
#include <stdio.h>

int main() {
    const wchar_t str[] = L"\xd800";
    char buffer[32];

    int rv = wcstombs(NULL, str, 0);

    printf("%d\n", rv);
}

Works fine on both Ubuntu 14 and other Linux (returns -1). I am honestly deeply puzzled at this. I also am not sure how to make a minimal triggering example, because I'm not sure what the pure python equivalent of PyUnicode_EncodeLocale is. '\ud800'.encode(errors="surrogateescape") doesn't hit this code path. Anyone have an idea?

All reactions

Sorry, something went wrong.

Uh oh!

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

@pganssle

pganssle commented Sep 5, 2018

Copy link
Copy Markdown
Member Author

This is too annoying for me to continue working on it. Anyone else is free to pick up where I left off.

All reactions

Sorry, something went wrong.

Uh oh!

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

@pganssle pganssle closed this Sep 5, 2018
@izbyshev izbyshev mannequin mentioned this pull request Apr 10, 2022
Different behavior of C and Python impls of datetime.strftime with non-UTF-8-encodable strings #78662
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

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

Assignees

No one assigned

Labels

awaiting review

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

@pganssle @the-knights-who-say-ni @bedevere-bot
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.