This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: imghdr.what doesn't accept bytes paths
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: wont fix
Dependencies: 19990 Superseder:
Assigned To: Nosy List: Claudiu.Popa, pitrou, serhiy.storchaka, vajrasky, vstinner
Priority: normal Keywords: patch

Created on 2013-12-16 12:48 by Claudiu.Popa, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
imghdr_bytes.patch Claudiu.Popa, 2013-12-16 12:48
imghdr_files.patch Claudiu.Popa, 2013-12-17 11:14
imghdr_files_2.patch Claudiu.Popa, 2013-12-18 17:17
imghdr_bytes.patch Claudiu.Popa, 2014-01-19 19:09
imghdr_bytes_2.patch Claudiu.Popa, 2014-02-22 09:21 review
Messages (12)
msg206299 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2013-12-16 12:48
imghdr.what check explicitly for string path, while `open` happily accepts bytes paths, as seen below:

>>> x
b'\xc2\xba'
>>> imghdr.what(x)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/tank/libs/cpython/Lib/imghdr.py", line 15, in what
    location = file.tell()
AttributeError: 'bytes' object has no attribute 'tell'
>>> open(x)
<_io.TextIOWrapper name=b'\xc2\xba' mode='r' encoding='UTF-8'>

A reason why this should be supported can be found in this message: http://bugs.python.org/msg191691.
The following patch fixes this. Also, it depends on issue19990 (where test_imghdr.py was added).
msg206394 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-12-17 02:17
Why limit to str and bytes. Open can accept integer as well. I am asking not suggesting.

>>> with open('/tmp/cutecat.txt', 'wb') as f:
...   f.write(b'cutecat')
... 
7
>>> a = open('/tmp/cutecat.txt')
>>> a.fileno()
3
>>> b = open(3)
>>> b.read()
'cutecat'
msg206419 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2013-12-17 11:14
Thanks, Vajrasky, I forgot about the fact that open can open file descriptors. Here's a patch which fixes this, also adds a test for BytesIO and uses os.fsencode instead of .encode().
msg206525 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-12-18 15:40
imghdr_bytes.patch should use os.fsencode() to encode the filename, not filename.encode().

(You may use a valid sound file of the Python suite test instead of generating an invalid sounf file.)

imghdr_files.patch looks overkill. I don't think that it's useful to support integer file descriptor. Being able to pass an open file is enough. imghdr_bytes.patch is fine.
msg206526 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2013-12-18 15:41
Hi, Victor! The second patch does use os.fsencode. I'll update the first one with this change.
msg206542 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2013-12-18 17:17
Added the new version.

> (You may use a valid sound file of the Python suite test instead of generating an invalid sounf file.)

Oh, that's sndhdr, currently imghdr doesn't have valid image files in the Python suite test (there is another issue for adding a test file for this module). If it's required, sure, I'll add a few.
msg208492 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-01-19 19:09
Updated patch to use real image files from issue #19990.
msg211916 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-02-22 09:21
Patch updated. It removes the check added in http://hg.python.org/cpython/rev/94813eab5a58 and simplifies the test for bytes file path.
msg211940 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-02-22 19:11
I'm not sure imghdr.what() should support bytes path. The open() builtin, most os and os.path functions support string and bytes paths, but many other modules (including pathlib) support only string paths.
msg219557 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-06-02 09:00
There are other modules with support for bytes filenames in their API:

bz2                          
codecs                       
gzip                         
lzma                         
pipes.Template               
tarfile
tokenize
fileinput
filecmp
sndhdr
configparser

Also, given the fact that sndhdr supports them and its purpose is similar with imghdr, it would be a surprise
for a beginner to find out that imghdr.what(b"img") is not working, while sndhdr.what(b"snd") works.
msg225605 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-08-21 08:29
See also a discussion at Python-Dev:
http://comments.gmane.org/gmane.comp.python.devel/149048

Looks as there are no need to add bytes path support in such high-level API. In any case you can call os.fsdecode() on path argument.
msg225636 - (view) Author: PCManticore (Claudiu.Popa) * (Python triager) Date: 2014-08-22 02:07
Right, I've read the thread you posted and I agree. My use cases aren't strong enough and it's enough for me to call os.fsdecode.
History
Date User Action Args
2022-04-11 14:57:55adminsetgithub: 64196
2014-08-22 02:07:45Claudiu.Popasetstatus: pending -> closed
resolution: wont fix
messages: + msg225636

stage: patch review -> resolved
2014-08-21 08:29:40serhiy.storchakasetstatus: open -> pending

messages: + msg225605
2014-06-02 09:00:37Claudiu.Popasetmessages: + msg219557
2014-02-22 19:11:46serhiy.storchakasetnosy: + pitrou
messages: + msg211940
2014-02-22 10:17:14pitrousetnosy: + serhiy.storchaka

stage: patch review
2014-02-22 09:21:45Claudiu.Popasetfiles: + imghdr_bytes_2.patch

messages: + msg211916
2014-01-19 19:09:59Claudiu.Popasetfiles: + imghdr_bytes.patch

messages: + msg208492
2013-12-19 16:56:41vstinnersetdependencies: + Add unittests for imghdr module
2013-12-18 17:17:27Claudiu.Popasetfiles: + imghdr_files_2.patch

messages: + msg206542
2013-12-18 15:41:59Claudiu.Popasetmessages: + msg206526
2013-12-18 15:40:05vstinnersetnosy: + vstinner
messages: + msg206525
2013-12-17 11:14:26Claudiu.Popasetfiles: + imghdr_files.patch

messages: + msg206419
2013-12-17 02:17:19vajraskysetnosy: + vajrasky
messages: + msg206394
2013-12-16 12:48:13Claudiu.Popacreate