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 — Use non-BOM encodings by filmor · Pull Request #2370 · pythonnet/pythonnet

Conversation

@filmor

@filmor filmor commented May 4, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Use non-BOM encodings for both C#->Python and Python->C#, as the byteorder is always the native one and the BOM is neither never or always used.

Fixes #2369.

Sorry, something went wrong.

Uh oh!

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

All reactions
@filmor
filmor force-pushed the fix-bom-strings branch 2 times, most recently from 49db3bf to 07f65c7 Compare May 5, 2024 18:41
filmor added 2 commits May 5, 2024 20:42
@filmor
Use non-BOM encodings
4c46c6d
@filmor
Copy potential BOM to the output of PyString_FromString
dc6f5ef
The documentation of the used `PyUnicode_DecodeUTF16` states that not
passing `*byteorder` or passing a 0 results in the first two bytes, if
they are the BOM (U+FEFF, zero-width no-break space), to be interpreted
and skipped, which is incorrect when we convert a known "non BOM"
string, which all strings from C# are.
@filmor
filmor force-pushed the fix-bom-strings branch from 07f65c7 to dc6f5ef Compare May 5, 2024 18:42
@filmor
filmor marked this pull request as ready for review May 5, 2024 18:42
@filmor
filmor requested a review from lostmsu May 5, 2024 18:44
@lostmsu

lostmsu commented May 6, 2024

Copy link
Copy Markdown
Member

@filmor can you ELI5? For someone not familiar with intricacies of BOM, but aware of byte order issues.

My biggest question is if this change has any potential to introduce bugs to handling strings that actually have BOM? E.g. imagine a scenario when someone serialized and persisted something with BOM using 3.0.3, but after this change in 3.0.4 if they read it back BOM will be in their string data.

All reactions

Sorry, something went wrong.

Uh oh!

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

@filmor

filmor commented May 6, 2024

Copy link
Copy Markdown
Member Author

It's the other way round. Strings that are being passed between Python and .NET are UTF16 in the respective native byte order (usually LE), without a BOM. The functions that we were using for the conversions (in particular PyUnicode_DecodeUTF16 and the defaulr encoding objects from Encoding) try to be "smart" and will interpret a leading set of FE FF or FF FE as the byte order mark, removing it from the converted string. By passing the correct endian-ness explicitly, this behaviour is disabled.

All reactions

Sorry, something went wrong.

Uh oh!

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

@filmor filmor self-assigned this May 7, 2024
@lostmsu
lostmsu merged commit 195cde6 into pythonnet:master May 10, 2024
@filmor
filmor deleted the fix-bom-strings branch May 10, 2024 19:55
@dependabot dependabot Bot mentioned this pull request Jan 16, 2026
Bump pythonnet from 3.0.3 to 3.0.5 mattemangia/Deep3DStudio#99
Closed
Martin-Molinero pushed a commit to QuantConnect/pythonnet that referenced this pull request Jun 19, 2026
@filmor @Martin-Molinero
Use non-BOM encodings (pythonnet#2370)
73389fc
* Use non-BOM encodings

The documentation of the used `PyUnicode_DecodeUTF16` states that not passing `*byteorder` or passing a 0 results in the first two bytes, if
they are the BOM (U+FEFF, zero-width no-break space), to be interpreted and skipped, which is incorrect when we convert a known "non BOM" string, which all strings from C# are.

(cherry picked from commit 195cde6)
@Martin-Molinero Martin-Molinero mentioned this pull request Jun 19, 2026
Add Python 3.12 / 3.13 / 3.14 support QuantConnect/pythonnet#122
Merged
jhonabreul pushed a commit to QuantConnect/pythonnet that referenced this pull request Jul 7, 2026
@Martin-Molinero @filmor @claude
Add Python 3.12 / 3.13 / 3.14 support (#122)
cea3c89
* Initial 3.14 commit

(cherry picked from commit caac33d)

* Apply alignment fix

(cherry picked from commit e10d333)

* Disable problematic GC tests

(cherry picked from commit e976558)

* Set ht_token to NULL in Python 3.14

(cherry picked from commit 65af098)

* Workaround for blocked PyObject_GenericSetAttr in metatypes

Python 3.14 introduced a new assertion that prevents us from using
PyObject_GenericSetAttr directly in our meta type. To work around
this, we manipulate the type dict directly.

This workaround is a simplified variant of Cython's workaround from
cython/cython#6325.

The relevant Python change is in
python/cpython#118454

(cherry picked from commit 08550d0)

* Use PyThreadState_GetUnchecked on Python 3.13

(cherry picked from commit f3face0)

* Remove deprecated function call

(cherry picked from commit 8dfe408)

* Assign True instead of None to __clear_reentry_guard__

Not at all sure why this helps, but when assigning `None` instead, the
object is gone at the time of garbage collection.

(cherry picked from commit 8e0333d)

* Move tp_clear workaround to .NET

In Python 3.14, the objects __dict__ seems to already be half
deconstructed, leading to crashes during garbage collection.

Since gc in Python is single-threaded (I think :)), it should
be fine to have a single static for this. If that is not true,
we can always use a thread-local instead.

(cherry picked from commit 908e13b)

* Use non-BOM encodings (pythonnet#2370)

* Use non-BOM encodings

The documentation of the used `PyUnicode_DecodeUTF16` states that not passing `*byteorder` or passing a 0 results in the first two bytes, if
they are the BOM (U+FEFF, zero-width no-break space), to be interpreted and skipped, which is incorrect when we convert a known "non BOM" string, which all strings from C# are.

(cherry picked from commit 195cde6)

* Preserve SyntaxError source line in message on Python 3.12+

Python 3.12 eagerly normalizes the error indicator, so PyErr_Fetch now
hands us the SyntaxError instance (whose str() omits the offending source
line) instead of the raw args tuple (whose str() included it). Callers that
surface PythonException.Message for compile diagnostics therefore lost the
offending source text on 3.12+.

GetMessage now re-appends the SyntaxError 'text' attribute when present.
This is a no-op on <=3.11 (there the fetched value is a tuple without the
SyntaxError attributes) and only affects SyntaxError messages.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* Make embed tests compatible with Python 3.12+ behavior changes

Three CPython behavior changes surfaced as failures/crashes once the
overload-resolution crash was fixed, all on 3.12+:

- ClassManagerTests.BindsCorrectOverloadForClassName crashed the host with
  "Python memory allocator called without holding the GIL". TestClass2's
  Get(PyObject o) re-enters Python via ToPython() while MethodBinder has
  released the GIL (allow_threads) around the managed call. A managed
  callback that re-enters Python must re-acquire the GIL; tolerated on
  <=3.11, fatal on 3.12+. Wrap the body in using (Py.GIL()).

- TestGetsPythonCodeInfoInStackTrace[ForNestedInterop]: 3.12+ adds caret
  indicator lines (e.g. "~~~^^^") under source lines in tracebacks, shifting
  the positional assertions. Drop caret-only lines before asserting
  (no-op on <=3.11).

- Codecs.ExceptionDecodedNoInstance: 3.12 eagerly normalizes exceptions, so
  the error indicator always carries an instance ("value"); the instanceless
  scenario this decoder targets can no longer be produced. Guard the test to
  <3.12.

Verified: full embed suite green on 3.11 (910/910) with these changes; the
three previously-failing tests pass on 3.14.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* Add Python 3.12 / 3.13 ABI offsets and CI jobs

The fork resolves PyTypeObject field offsets from the hardcoded
TypeOffset{major}{minor} tables (it does not run geninterop at build),
so a missing table makes ABI.Initialize throw "Python ABI v... is not
supported" and every test on that version fails at PythonEngine init.
Only 3.6-3.11 and 3.14 tables were present.

Vendor the 3.12 and 3.13 tables from pythonnet/pythonnet upstream
(byte-identical to upstream master; same source as the already-present
TypeOffset314) and add 3.12 + 3.13 to the CI matrix.

Local verification (uv standalone CPython 3.13, this branch's fixes):
embed suite ABI-initializes correctly and runs 847 passed / 0 failed
(parity with 3.14). 3.12 table is vendored from the same authoritative
source; CI exercises it.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

* Support Python 3.13/3.14 re-init in tests; drop obsolete TestDomainReload

The embed-test suite re-initializes the interpreter per fixture. On CPython
3.13/3.14 the suite aborted with "Failed to import encodings module" - not a
filesystem problem (strace shows the file opens fine) but interpreter import
state corrupted across re-initialization.

Root cause isolated to a single test: TestPythonEngineProperties.SetPythonPath.
It uses PythonEngine.PythonPath, which pins a fixed module search path via the
deprecated Py_SetPath. CPython 3.13+ keeps that path config in _PyRuntime across
Py_Finalize and offers no way to reset it back to auto-computation without the
PyConfig API, so once this test runs every later re-initialization in the same
process is forced onto the pinned path and eventually cannot bootstrap encodings.
All other fixtures - including the normal Initialize/Shutdown cycles in
pyinitialize and TestFinalizer - run fine in a single process.

Run only SetPythonPath in its own test process so it cannot pollute the rest of
the suite. Verified locally: full embed suite green on 3.11, 3.13 and 3.14
(main run 908 / SetPythonPath 1, 0 failures); no regression.

Also delete TestDomainReload: AppDomain reload is not supported on modern .NET
(single-domain), so those tests (MarshalByRefObject / AppDomain.CreateDomain)
are obsolete.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

---------

Co-authored-by: Benedikt Reinartz <[email protected]>
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lostmsu lostmsu Awaiting requested review from lostmsu

Uh oh!

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

Assignees

@filmor filmor

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Possible bug in reading zero width no-break space character

Uh oh!

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

2 participants

@filmor @lostmsu
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.