Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PhotoImage(data=...) apparently has to be UTF-8 or Base-64 encoded #65779

Closed
vadmium opened this issue May 26, 2014 · 11 comments
Closed

PhotoImage(data=...) apparently has to be UTF-8 or Base-64 encoded #65779

vadmium opened this issue May 26, 2014 · 11 comments
Assignees
Labels
docs Documentation in the Doc dir topic-tkinter type-bug An unexpected behavior, bug, or error

Comments

@vadmium
Copy link
Member

vadmium commented May 26, 2014

BPO 21580
Nosy @loewis, @terryjreedy, @vadmium, @serhiy-storchaka
Dependencies
  • bpo-21605: Add tests for Tkinter images
  • Files
  • tkinter_bytes.patch
  • tkinter_bytes-2.7.patch: Patch for 2.7.
  • tkinter_bytes-2.7_2.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/serhiy-storchaka'
    closed_at = <Date 2014-07-31.07:33:13.889>
    created_at = <Date 2014-05-26.05:52:24.420>
    labels = ['type-bug', 'expert-tkinter', 'docs']
    title = 'PhotoImage(data=...) apparently has to be UTF-8 or Base-64 encoded'
    updated_at = <Date 2014-07-31.07:33:13.888>
    user = 'https://github.com/vadmium'

    bugs.python.org fields:

    activity = <Date 2014-07-31.07:33:13.888>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2014-07-31.07:33:13.889>
    closer = 'serhiy.storchaka'
    components = ['Documentation', 'Tkinter']
    creation = <Date 2014-05-26.05:52:24.420>
    creator = 'martin.panter'
    dependencies = ['21605']
    files = ['35431', '35432', '36167']
    hgrepos = []
    issue_num = 21580
    keywords = ['patch']
    message_count = 11.0
    messages = ['219133', '219287', '219317', '219346', '219499', '223811', '224295', '224315', '224319', '224321', '224380']
    nosy_count = 7.0
    nosy_names = ['loewis', 'terry.reedy', 'gpolo', 'docs@python', 'python-dev', 'martin.panter', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue21580'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @vadmium
    Copy link
    Member Author

    vadmium commented May 26, 2014

    At the bottom of the “tkinter” doc page it mentions the “data” option is available. However I was unable to get it to work well when passing a plain bytes() string in. It seems the bytes() string gets interpreted as UTF-8 somewhere along the line, although the underlying TCL library apparenly handles byte strings fine itself. Passing binary GIF and PNG data in Python would usually produce strange exceptions, crashes, and blank images for me.

    I found this message with a simple patch which might be useful, though I’m not familiar with the code involved to understand the implications:
    https://mail.python.org/pipermail/tkinter-discuss/2012-April/003108.html

    Even if that fix is not appropriate, can I suggest adding to the documentation to say the data should be encoded with one of these options that seem to work? The Base-64 one is probably better.

    PhotoImage(data=data.decode("latin-1).encode("utf-8"))
    PhotoImage(data=base64.encodebytes(data))

    @vadmium vadmium added docs Documentation in the Doc dir topic-tkinter labels May 26, 2014
    @serhiy-storchaka
    Copy link
    Member

    All works to me.

    >>> import tkinter
    >>> b = tkinter.Button()
    >>> with open('Lib/test/imghdrdata/python.gif', 'rb') as f: data = f.read()
    ... 
    >>> img = tkinter.PhotoImage(data=data)
    >>> b['image'] = img
    >>> b.pack()

    Could you please provide an example which demonstrates the issue?

    @vadmium
    Copy link
    Member Author

    vadmium commented May 28, 2014

    Thanks for looking at this. Originally the issue was found by reading the GIF and PNG images on various web pages, such as <http://www.weatherzone.com.au/vic/north-central/castlemaine\>. However I was also able to produce the problem with the other formats of that Python logo:

    $ python3 -Wall -bt
    Python 3.4.1 (default, May 19 2014, 17:40:19) 
    [GCC 4.9.0 20140507 (prerelease)] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import tkinter; tk = tkinter.Tk(); text = tkinter.Text(tk); text.pack()
    >>> with open("/lib/python3.4/test/imghdrdata/python.png", "rb") as file: data = file.read()
    ... 
    >>> image = tkinter.PhotoImage(format="png", data=data)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python3.4/tkinter/__init__.py", line 3384, in __init__
        Image.__init__(self, 'photo', name, cnf, master, **kw)
      File "/usr/lib/python3.4/tkinter/__init__.py", line 3340, in __init__
        self.tk.call(('image', 'create', imgtype, name,) + options)
    _tkinter.TclError: CRC check failed
    >>> u8png = tkinter.PhotoImage(format="png", data=data.decode("latin-1").encode("utf-8"))
    >>> text.image_create(tkinter.END, image=u8png)
    'pyimage2'

    The same problem occurs with the PGM and PPM logos, and the Base-64 encoding trick does not work with those; only UTF-8 encoding. However as you discovered, the GIF format logo works no problem when passed unencoded, although it continues to work if I use my UTF-8 encoding trick, which is a bit strange. Perhaps the internal UTF-8 decoding step is passing the invalid UTF-8 straight through?

    @serhiy-storchaka
    Copy link
    Member

    Here is a patch which fixes passing Python bytes to Tcl in Python 3. However it will be not easy to fix this issue in Python 2.

    See also bpo-21605 which adds tests for Tkinter images (some of them fails without this patch).

    @serhiy-storchaka serhiy-storchaka added the type-bug An unexpected behavior, bug, or error label May 29, 2014
    @serhiy-storchaka
    Copy link
    Member

    Unfortunately we can't use this straightforward and universal solution in Python 2. Here is a patch which adds special workarounds to fix this issue in 2.7.

    @terryjreedy
    Copy link
    Member

    I will try test the problem and fix on Windows within a day.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Jul 30, 2014

    The 3.4 patch looks fine, please apply.

    I'm -1 on the 2.7 patch. I think it would be better to add a _tkinter helper function to create Tcl byte array objects. Alternatively, the "binary format" command might help. OTOH, I don't care about 2.7, so feel free to do whatever you consider appropriate.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 30, 2014

    New changeset 9474f2971855 by Serhiy Storchaka in branch '3.4':
    Issue bpo-21580: Now Tkinter correctly handles bytes arguments passed to Tk.
    http://hg.python.org/cpython/rev/9474f2971855

    New changeset b9d249316f29 by Serhiy Storchaka in branch 'default':
    Issue bpo-21580: Now Tkinter correctly handles bytes arguments passed to Tk.
    http://hg.python.org/cpython/rev/b9d249316f29

    @serhiy-storchaka
    Copy link
    Member

    Thank you Martin for your reviews. Here is updated 2.7 patch which implements your suggestion.

    The disadvantage of this patch in comparison with first version is that it will not work with statically compiled embedded Python, when only stdlib is updated. I once broke re in bugfix release in such manner (by moving one constant from Python sources to C sources). On other hand, it is very unlikely that anyone uses Tkinter in such circumstances, and in any case this part of code is broken for now, so patch should not introduce new regression.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Jul 30, 2014

    The 2.7_2 patch looks good to me. I won't rule on the backwards compatibility implications, although I agree that this is unlikely to cause a regression (it would only if somebody updated the standard library only, *and* would use data= for PhotoImage). I'm unsure whether it was possible at all so far to use data= (with whatever argument); if you could have passed a non-buffer object successfully, this would break now.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 31, 2014

    New changeset 818989a48e96 by Serhiy Storchaka in branch '2.7':
    Issue bpo-21580: Now Tkinter correctly handles binary "data" and "maskdata"
    http://hg.python.org/cpython/rev/818989a48e96

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    docs Documentation in the Doc dir topic-tkinter type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants