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

Type issue leading to NotOTRMessage exception when it should have been UnencryptedError on python 3 #47

Open
mathieui opened this issue Dec 28, 2013 · 9 comments
Assignees
Milestone

Comments

@mathieui
Copy link
Contributor

On python 3:

In context.py, the basestring type is an alias to str; however, everything in and out of the library is bytes (otherwise it does not work), so when receiving a plaintext message during an encrypted session it never enters the test for unencryptedmessage and raises a nototrmessage (which does not provoke a warning because it is supposed to be raised only outside an OTR session).

Specifically, at the end of receiveMessage, message is a bytes object and basestring is “str”.

        if isinstance(message, basestring):
            if self.state != STATE_PLAINTEXT or \
                    self.getPolicy('REQUIRE_ENCRYPTION'):
                raise UnencryptedMessage(message)

        raise NotOTRMessage(messageData)

The fix should be to change,

basestring = str

to

basestring = bytes

at the beginning of potr/context.py, it seems to work fine.

@koolfy koolfy self-assigned this Jan 15, 2015
@koolfy koolfy added this to the 1.0.2 milestone Oct 25, 2015
@koolfy
Copy link
Contributor

koolfy commented Oct 25, 2015

Wouldn't it be confusing to leave

    isinstance(message, basestring)

when we are not testing for basestring intance, but really bytes?

Maybe it would be better to have a better named variable in there instead? Something along the lines of:

    try:
        type(basestring)
        rawMessageFormat = basestring
    except NameError:
        # all strings are unicode in python3k
        rawMessageFormat = bytes
        unicode = str

and then using:

    if isinstance(message, rawMessageFormat):

?

My concern is how having it changed to bytes accross the lib could affect other parts of the logic, and whether unicode should be left alone or could raise similar issues elsewhere? (I don't think so but we should doublecheck)

@mathieui
Copy link
Contributor Author

If we’re testing for bytes, wouldn’t it be better to only do

rawMessageFormat = bytes

(or does that break python versions older than 2.7 that you want to support?)

@koolfy
Copy link
Contributor

koolfy commented Jul 20, 2016

I don't think supporting python 2.7 and older is reasonable at this point.
Thoughts?

@tribut
Copy link
Contributor

tribut commented Jul 20, 2016

2.7 should probably still be supported for a while, but I don't think older versions are used by reasonable people any more (except in old centos releases maybe).

@azrdev
Copy link

azrdev commented Jul 20, 2016

@koolfy older is EOL afaik, but since gajim itself is on python2.7 you cannot move to python3 atm

@koolfy
Copy link
Contributor

koolfy commented Jul 20, 2016

good point.

@mathieui
Copy link
Contributor Author

@azrdev the gajim hg default branch is python3, but it’s not yet usable iirc

@koolfy koolfy modified the milestones: 1.0.2, 1.0.3 Apr 2, 2018
@koolfy
Copy link
Contributor

koolfy commented Apr 2, 2018

I reinstalled a rasperry pi as an IRC client and discovered raspbian-stretch still ships python2.7 by default :)

So I guess this settles the debate, python2.7 is still very much alive, this should be fixed for 1.0.3

@azrdev
Copy link

azrdev commented Apr 2, 2018

With the release of gajim 1.0.0 last month, they finally moved to python3.
As the official wiki says, python2.x is legacy. And only 2.7 is maintained, older 2.x releases are not, as I wrote before.

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

4 participants