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

string vs bytes type checking/handling #66

Open
oberstet opened this issue Jun 7, 2017 · 1 comment
Open

string vs bytes type checking/handling #66

oberstet opened this issue Jun 7, 2017 · 1 comment

Comments

@oberstet
Copy link

oberstet commented Jun 7, 2017

Split out from #11

The way the type checking is done in the code base problematic. Take this line:

if not isinstance(var, six.string_types):
    raise MarshallingError('Required string. Received: ' + repr(var))

The problem is, on Python 2 this will always evaluate to False! For both bytes and unicode strings.

(cpy361_7) oberstet@thinkpad-t430s:~/scm/crossbario/crossbar-fabric-device-agent$ python2
Python 2.7.12 (default, Nov 19 2016, 06:48:10) 
[GCC 5.4.0 20160609] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import six
>>> six.text_type
<type 'unicode'>
>>> six.binary_type
<type 'str'>
>>> type(b'hello') == six.binary_type
True
>>> type(b'hello') == six.text_type
False
>>> six.string_types
(<type 'basestring'>,)
>>> isinstance(u'hello', six.string_types)
True
>>> isinstance(b'hello', six.string_types)
True
>>> 
(cpy361_7) oberstet@thinkpad-t430s:~/scm/crossbario/crossbar-fabric-device-agent$ python
Python 3.6.1 (default, Apr  2 2017, 18:19:20) 
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import six
>>> six.text_type
<class 'str'>
>>> six.binary_type
<class 'bytes'>
>>> type(b'hello') == six.binary_type
True
>>> type(b'hello') == six.text_type
False
>>> six.string_types
(<class 'str'>,)
>>> isinstance(u'hello', six.string_types)
True
>>> isinstance(b'hello', six.string_types)
False
>>> 

The reason is, that on Python 2, both str (bytes) and unicode derive of basestring (https://pythonhosted.org/six/#six.string_types, https://docs.python.org/2/library/functions.html#basestring).

The right way to check is via six.text_type and six.binary_type. See above.

@WhyNotHugo
Copy link
Collaborator

Makes perfect sense. Given the comment right above the method, I'd say that six.text_type should expected here.

I think it makes sense for this to be:

    if not isinstance(var, six.text_type):
        raise MarshallingError(
            'Required string. Received: %' % type(var).__name__
        )

Note that the error message now shows the type, rather than repr as well (does this make sense too?).

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

No branches or pull requests

2 participants