X Tutup
Skip to content

gh-101828: Fix jisx0213 codecs removing null characters#139340

Merged
corona10 merged 4 commits intopython:mainfrom
StanFromIreland:shift_jis
Oct 14, 2025
Merged

gh-101828: Fix jisx0213 codecs removing null characters#139340
corona10 merged 4 commits intopython:mainfrom
StanFromIreland:shift_jis

Conversation

@StanFromIreland
Copy link
Member

@StanFromIreland StanFromIreland commented Sep 25, 2025 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request Sep 25, 2025
Closed
@corona10 corona10 self-assigned this Sep 26, 2025
@corona10
Copy link
Member

corona10 commented Sep 26, 2025

@StanFromIreland Thank you for the investigation. Let me take a look at this weekend :)

@corona10 corona10 added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Sep 28, 2025
corona10
corona10 approved these changes Sep 28, 2025
View reviewed changes
Copy link
Member

@corona10 corona10 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
This patch prevents NULL bytes from being consumed as part of character pair encoding in the string. While this fixes the data loss bug, it does change existing behavior, so backporting needs discussion.

cc @methane

@corona10
Copy link
Member

corona10 commented Oct 4, 2025

I will wait Naoki san for a week and plan to merge this PR.

methane
methane reviewed Oct 6, 2025
View reviewed changes
@methane
Copy link
Member

methane commented Oct 6, 2025 •
edited
Loading

euc_jis_2004 has same logic. would you update it too?

>>> "\u00e6abc".encode('euc_jis_2004')
b'\xa9\xdcabc'
>>> "\u00e6\0abc".encode('euc_jis_2004')
b'\xa9\xdcabc'

@StanFromIreland
Copy link
Member Author

StanFromIreland commented Oct 6, 2025

euc_jis_2004 has same logic. would you update it too?

Is done.

@methane methane changed the title gh-101828: Fix shift_jisx0213 & shift_jis_2004 codecs removing null characters gh-101828: Fix jisx0213 codecs removing null characters Oct 7, 2025
@methane
Copy link
Member

methane commented Oct 7, 2025

iso2022_jp_3 and iso2022_jp_2004 have same issue.
Would you add this patch?

diff --git a/Modules/cjkcodecs/_codecs_iso2022.c b/Modules/cjkcodecs/_codecs_iso2022.c
index ef6faeb7127..83afdd0a1ee 100644
--- a/Modules/cjkcodecs/_codecs_iso2022.c
+++ b/Modules/cjkcodecs/_codecs_iso2022.c
@@ -802,10 +802,12 @@ jisx0213_encoder(const MultibyteCodec *codec, const Py_UCS4 *data,
         return coded;

     case 2: /* second character of unicode pair */
-        coded = find_pairencmap((ucs2_t)data[0], (ucs2_t)data[1],
-                                jisx0213_pair_encmap, JISX0213_ENCPAIRS);
-        if (coded != DBCINV)
-            return coded;
+        if (data[1] != 0) { /* Don't consume null char as part of pair */
+            coded = find_pairencmap((ucs2_t)data[0], (ucs2_t)data[1],
+                                    jisx0213_pair_encmap, JISX0213_ENCPAIRS);
+            if (coded != DBCINV)
+                return coded;
+        }
         _Py_FALLTHROUGH;

methane
methane reviewed Oct 7, 2025
View reviewed changes
@corona10 corona10 merged commit 87eadce into python:main Oct 14, 2025
45 checks passed
@miss-islington-app
Copy link

miss-islington-app bot commented Oct 14, 2025

Thanks @StanFromIreland for the PR, and @corona10 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 14, 2025
@StanFromIreland @miss-islington
pythongh-101828: Fix jisx0213 codecs removing null characters (pyth…
ba47d32 
…ongh-139340)

(cherry picked from commit 87eadce)

Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
@miss-islington-app
Copy link

miss-islington-app bot commented Oct 14, 2025

Sorry, @StanFromIreland and @corona10, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 87eadce3e0309d80a95e85d70a00028b5dca9907 3.13

@bedevere-app
Copy link

bedevere-app bot commented Oct 14, 2025

GH-140110 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Oct 14, 2025
@StanFromIreland StanFromIreland deleted the shift_jis branch October 14, 2025 14:03
@StanFromIreland
Copy link
Member Author

StanFromIreland commented Oct 14, 2025

I can backport.

StanFromIreland added a commit to StanFromIreland/cpython that referenced this pull request Oct 14, 2025
@StanFromIreland
[3.13] pythongh-101828: Fix jisx0213 codecs removing null characters (
8211139 
pythongh-139340)

(cherry picked from commit 87eadce)

Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented Oct 14, 2025

GH-140112 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Oct 14, 2025
corona10 pushed a commit that referenced this pull request Oct 14, 2025
@miss-islington @StanFromIreland
[3.14] gh-101828: Fix jisx0213 codecs removing null characters (gh-…
99e42ab 
…139340) (gh-140110)

gh-101828: Fix `jisx0213` codecs removing null characters (gh-139340)
(cherry picked from commit 87eadce)

Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
corona10 pushed a commit that referenced this pull request Oct 14, 2025
@StanFromIreland
[3.13] gh-101828: Fix jisx0213 codecs removing null characters (gh-…
c5ec267 
…139340) (gh-140112)

* [3.13] gh-101828: Fix `jisx0213` codecs removing null characters (gh-139340)
(cherry picked from commit 87eadce)

Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>

* Accidentally removed line
StanFromIreland added a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
@StanFromIreland
pythongh-101828: Fix jisx0213 codecs removing null characters (pyth…
48cc311 
…ongh-139340)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@methane methane methane approved these changes

@corona10 corona10 corona10 approved these changes

Assignees

@StanFromIreland StanFromIreland

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@StanFromIreland @corona10 @methane
X Tutup