You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
PyUnicode_FromWideChar incorrect for characters outside the BMP (unix only) #48724
On systems (Linux, OS X) where sizeof(wchar_t) is 4 and wchar_t arrays are
usually encoded as UTF-32, it looks as though PyUnicode_FromWideChar
simply truncates the 32-bit characters to 16-bits, thus giving incorrect
results for characters outside the BMP. I expected it to convert the UTF-
32 encoding to UTF-16.
Note that PyUnicode_FromWideChar is used to process command-line
arguments, so strange things can happen when passing filenames with non-
BMP characters to a Python script.
Here's an OS X 10.5 Terminal session (current directory is the root of the
py3k tree).
dickinsm$ cat test𐅭.py
from sys import argv
print("My arguments are: ",argv)
dickinsm$ ./python.exe test𐅭.py
My arguments are: ['testŭ.py']
dickinsm$ ./python.exe Lib/tabnanny.py test𐅭.py
'testŭ.py': I/O Error: [Errno 2] No such file or directory: 'testŭ.py'
(In case the character after 'test' and before '.py' isn't showing up
correctly, it's chr(65901), 'GREEK ACROPHONIC TROEZENIAN FIVE HUNDRED'.)
I think you are right. However, conversion to/from wchar_t is/was
rarely used, and so are non-BMP characters; it's very likely that
the problem hasn't occurred in practice (and I doubt it would occur
in 3.0 if not fixed - there are more severe problems around).
rpetrov commented on Nov 30, 2008
rpetrovmannequin
Mannequin
More actions
it is fine on linux (tested with UTF-8 codeset for locale):
$ ./python test𐅭.py
('My arguments are: ', ['test\xf0\x90\x85\xad.py'])
\xf0\x90\x85\xad (UTF-8) = 0001016d (USC-4) = 65901
This is due to the function downcasting the wchar_t values to
Py_UNICODE, which is a 2-byte value if you build Python as UCS2 version
on Unix.
Most Unixes ship with UCS4 builds, so you don't see the problem there.
Mac OS X ships with a UCS2 build, which is why you run into the problem
on that platform.
UCS2 builds are also the default build on Unix, so if you compile Python
yourself, it will result in a UCS2 build, unless you explicitly specify
the UCS4 build configure option or configure happens to find a Tcl/Tk
version installed that uses UCS4 internally.
rpetrov commented on Dec 1, 2008
rpetrovmannequin
Mannequin
More actions
Marc-Andre explain all. For the protocol my version is from trunk,
python is build with default options. Since system tcl limit UTF-8 to 3
bytes, python is build for UCS-2.
In the report output from python is with character 010d(UCS-2).
Looks pretty good at first glance, except that it seems that the UTF-32 to
UTF-16 translation is skipped if HAVE_USABLE_WCHAR_T is defined. Is that
deliberate?
Looks pretty good at first glance, except that it seems that the UTF-32 to
UTF-16 translation is skipped if HAVE_USABLE_WCHAR_T is defined. Is that
deliberate?
I understand this code as: sizeof(wchar_t) == sizeof(Py_UNICODE). If I
misunderstood the code, it's a a heap overflow :-) So there is no not
conversion from UTF-32 to UTF-16 using memcpy if HAVE_USABLE_WCHAR_T is
defined, right?
A test would be good, too.
PyUnicode_FromWideChar() is not a public API. Should I write a function in
_testcapi?
Also note that on platforms with 16-bit wchar_t, the comparison
(0xffff < *w) will always be false, so an additional check for
(Py_UNICODE_SIZE > 2) is needed.
Yes, but the right test is (SIZEOF_WCHAR_T > 2). I wrote a new test:
BTW: Please always use upper-case hex literals, or at leat don't
mix the case within the same function.
I try, but it would be easier if the rule was already respected: they
are many tabs and many lower-case hex literals. I used copy/paste from
existing code of unicodeobject.c...
Patch version 3:
disable the UTF-16 surrogate for 16 bits wchar_t: so my patch is
only used for 16 bits Py_UNICODE and 32 bits wchar_t... which is the
default case for python 2.6 and 3.0 on Linux
@marketdickinson, @lemburg: ping! I updated the patch, does it look
better?
Yes, but there are a few things that still need fixing:
*SIZEOF_WCHAR_TisnotdefinedforWindowsbuilds, soneedstobeaddedtoPC/pyconfig.h (OTOHwchar_tis2bytesonWindows)
*USE_WCHAR_SURROGATEshouldbe#defined just before the
functionand#undef'ed right after it; I'd also use a more
accuratename*pleaseusepre-processorindents, e.g.
#ifdef ...
# define ...
#endif
applies cleanly against newly whitespace-normalized unicodeobject.c
renamed USE_WCHAR_SURROGATE to CONVERT_WCHAR_TO_SURROGATES
add defined(SIZEOF_WCHAR_T) check
I find the patched version of PyUnicode_FromWideChar quite hard to follow
with all the #ifdefery. I wonder whether it might be better to define a
_PyUnicode16_FromWideChar32 helper function instead.
applies cleanly against newly whitespace-normalized unicodeobject.c
renamed USE_WCHAR_SURROGATE to CONVERT_WCHAR_TO_SURROGATES
add defined(SIZEOF_WCHAR_T) check
I find the patched version of PyUnicode_FromWideChar quite hard to follow
with all the #ifdefery. I wonder whether it might be better to define a
_PyUnicode16_FromWideChar32 helper function instead.
Same here. It would be better to have a single #ifdef #else #endif
block with one branch for the CONVERT_WCHAR_TO_SURROGATES case and the
other for the normal operation.
I don't understand why SIZEOF_WCHAR_T could be unset, but the patch
version 6 only checks defined(SIZEOF_WCHAR_T) in unicodeobject.c, not
in _testcapimodule.c (#if SIZEOF_WCHAR_T == 4).
Since this is partway between a bugfix and a new feature, I suggest that
it's not worth merging it to 3.0 (or 2.6). It should be backported to
2.7, however; I'll do this after verifying that the py3k buildbots are
happy.
Activity
mdickinson commentedon Nov 30, 2008
On systems (Linux, OS X) where sizeof(wchar_t) is 4 and wchar_t arrays are
usually encoded as UTF-32, it looks as though PyUnicode_FromWideChar
simply truncates the 32-bit characters to 16-bits, thus giving incorrect
results for characters outside the BMP. I expected it to convert the UTF-
32 encoding to UTF-16.
Note that PyUnicode_FromWideChar is used to process command-line
arguments, so strange things can happen when passing filenames with non-
BMP characters to a Python script.
Here's an OS X 10.5 Terminal session (current directory is the root of the
py3k tree).
dickinsm$ cat test𐅭.py
from sys import argv
print("My arguments are: ",argv)
dickinsm$ ./python.exe test𐅭.py
My arguments are: ['testŭ.py']
dickinsm$ ./python.exe Lib/tabnanny.py test𐅭.py
'testŭ.py': I/O Error: [Errno 2] No such file or directory: 'testŭ.py'
(In case the character after 'test' and before '.py' isn't showing up
correctly, it's chr(65901), 'GREEK ACROPHONIC TROEZENIAN FIVE HUNDRED'.)
mdickinson commentedon Nov 30, 2008
Comments from MvL in bpo-4388:
rpetrov commentedon Nov 30, 2008
it is fine on linux (tested with UTF-8 codeset for locale):
$ ./python test𐅭.py
('My arguments are: ', ['test\xf0\x90\x85\xad.py'])
\xf0\x90\x85\xad (UTF-8) = 0001016d (USC-4) = 65901
rpetrov commentedon Nov 30, 2008
s/USC-4/UCS-4/g
mdickinson commentedon Nov 30, 2008
Interesting. Which version of Python is that? And is PyUNICODE 2 bytes
or 4 bytes for that build of Python?
mdickinson commentedon Dec 1, 2008
Just to be clear, the defect in PyUnicode_FromWideChar is present both in
Python 2.x and Python 3.x.
The problem with command-line arguments only occurs in Python 3.x, since
2.x doesn't use PyUnicode_FromWideChar in converting arguments.
I can reproduce the 'No such file or directory' error on both OS X and
Linux, for Python 3.0.
malemburg commentedon Dec 1, 2008
This is due to the function downcasting the wchar_t values to
Py_UNICODE, which is a 2-byte value if you build Python as UCS2 version
on Unix.
Most Unixes ship with UCS4 builds, so you don't see the problem there.
Mac OS X ships with a UCS2 build, which is why you run into the problem
on that platform.
UCS2 builds are also the default build on Unix, so if you compile Python
yourself, it will result in a UCS2 build, unless you explicitly specify
the UCS4 build configure option or configure happens to find a Tcl/Tk
version installed that uses UCS4 internally.
rpetrov commentedon Dec 1, 2008
Marc-Andre explain all. For the protocol my version is from trunk,
python is build with default options. Since system tcl limit UTF-8 to 3
bytes, python is build for UCS-2.
In the report output from python is with character 010d(UCS-2).
May be issue is not for versions before 3.0.
vstinner commentedon Jan 17, 2009
Patch fixing PyUnicode_FromWideChar() for UCS-2 build: create
surrogates for character > U+FFFF like PyUnicode_FromOrdinal() does.
vstinner commentedon Jan 17, 2009
Note: I wrote my patch against py3k r68646.
mdickinson commentedon Jan 17, 2009
Thanks for the patch, Victor!
Looks pretty good at first glance, except that it seems that the UTF-32 to
UTF-16 translation is skipped if HAVE_USABLE_WCHAR_T is defined. Is that
deliberate?
A test would be good, too.
vstinner commentedon Jan 17, 2009
I understand this code as: sizeof(wchar_t) == sizeof(Py_UNICODE). If I
misunderstood the code, it's a a heap overflow :-) So there is no not
conversion from UTF-32 to UTF-16 using memcpy if HAVE_USABLE_WCHAR_T is
defined, right?
PyUnicode_FromWideChar() is not a public API. Should I write a function in
_testcapi?
mdickinson commentedon Jan 17, 2009
Yep, sorry. You're right.
I was actually thinking of a test for the "No such file or directory"
bug that's mentioned at the start of the discussion.
7 remaining items
vstinner commentedon Jan 21, 2009
Yes, but the right test is (SIZEOF_WCHAR_T > 2). I wrote a new test:
I try, but it would be easier if the rule was already respected: they
are many tabs and many lower-case hex literals. I used copy/paste from
existing code of unicodeobject.c...
Patch version 3:
only used for 16 bits Py_UNICODE and 32 bits wchar_t... which is the
default case for python 2.6 and 3.0 on Linux
vstinner commentedon Jan 26, 2009
@marketdickinson, @lemburg: ping! I updated the patch, does it look
better?
malemburg commentedon Jan 27, 2009
On 2009-01-26 17:56, STINNER Victor wrote:
Yes, but there are a few things that still need fixing:
I'd write
...
#undef CONVERT_WCHAR_TO_SURROGATES
Thanks.
vstinner commentedon Jan 29, 2009
For lemburg, updated patch:
(and indent the defines, sorry)
mdickinson commentedon Feb 24, 2009
Updated Victor's patch:
I find the patched version of PyUnicode_FromWideChar quite hard to follow
with all the #ifdefery. I wonder whether it might be better to define a
_PyUnicode16_FromWideChar32 helper function instead.
malemburg commentedon Feb 24, 2009
On 2009-02-24 20:39, Mark Dickinson wrote:
Same here. It would be better to have a single #ifdef #else #endif
block with one branch for the CONVERT_WCHAR_TO_SURROGATES case and the
other for the normal operation.
No need for a new helper function.
mdickinson commentedon Feb 24, 2009
Yes, of course it would. :)
mdickinson commentedon Feb 24, 2009
New patch, with two separate versions of PyUnicode_FromWideChar.
malemburg commentedon Feb 24, 2009
On 2009-02-24 21:50, Mark Dickinson wrote:
Thanks, much better :-)
vstinner commentedon Feb 26, 2009
I don't understand why SIZEOF_WCHAR_T could be unset, but the patch
version 6 only checks defined(SIZEOF_WCHAR_T) in unicodeobject.c, not
in _testcapimodule.c (#if SIZEOF_WCHAR_T == 4).
mdickinson commentedon Feb 28, 2009
Good catch! Added defined(SIZEOF_WCHAR) to the testcapi code as well,
and removed the change to PC/pyconfig.h, since we don't need it any
more...
mdickinson commentedon Mar 18, 2009
Committed to py3k, r70452.
Since this is partway between a bugfix and a new feature, I suggest that
it's not worth merging it to 3.0 (or 2.6). It should be backported to
2.7, however; I'll do this after verifying that the py3k buildbots are
happy.
mdickinson commentedon Mar 18, 2009
Backported to the trunk in r70454. Thanks, all!