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

UnicodeEncodeError when calling puts function with unicode u'\xff' and the stream.encoding is None #1303

Open
zhaoguixu opened this issue Apr 8, 2015 · 5 comments

Comments

@zhaoguixu
Copy link

Hi,
When replacing the variable s to another value u'\xff', this test would fail.

[test-case]

@mock_streams('stdout')
def test_puts_with_encoding_type_none_output():
    """
    puts() should print unicode output without a stream encoding
    """
    s = u"string!"       #<====== replace to u"\xff"
    output.user = True
    sys.stdout.encoding = None
    puts(s, show_prefix=False)
    eq_(sys.stdout.getvalue(), s + "\n")

[traceback]
UnicodeEncodeError: 'ascii' codec can't encode characters in position 0-4: ordinal not in range(128)

[comments]
The buggy point locates in the function '_encode' in utils.py, you should not simply cast the msg into str by using str(msg).

def _encode(msg, stream):
    if isinstance(msg, unicode) and hasattr(stream, 'encoding') and not stream.encoding is None:
        return msg.encode(stream.encoding)
    else:
        return str(msg)         #<========== buggy point
@zhaoguixu
Copy link
Author

Anyone cares?

@ppkt
Copy link

ppkt commented Jul 30, 2015

Yup, it seems that changing return str(msg) to return msg solves the problem without breaking tests. But why this cast was necessary at the first place?

@kaaveland
Copy link

I'm not absolutely sure it makes sense to change this now. It's not going to absolutely break anyone's code, but it basically lets you puts things which aren't strings and get something sensible out:

>>> class Foo(object):
...   def __str__(self): return "The best Foo in the world"
...
>>> from fabric.api import puts
>>> puts(Foo())
The best Foo in the world

I'm guessing there's code out there that uses this (hopeless to search for this in github, so not going to try to look for examples).

I'm not sure I think it's right that not having an encoding set and using unicode will cause puts to crash us, though.

@bitprophet
Copy link
Member

Suspect the least-worst way to address this for now is to simply expand the offending line into this block:

try:
    return str(msg) # Original behavior added for 'reasons'
except UnicodeDecodeError:
    return msg # Best-effort fallback

This way, whatever antediluvian reasons we had for casting-to-string will continue to function, but the buggy case under discussion would fall back to "well, I dunno what this is, but I can't str() it, so I'll just shove it down the pipe as-is".

@dclong
Copy link

dclong commented Apr 15, 2017

I encountered a similar issue while running the sudo method, and the exception happened at self.stream.write(text) in io.py. Is there any progress on the unicode issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants