Ascii decoding in OTR messages might make downstream plugins crash

[koolfy]

It has been reported by @mathieui that some OTR implementations (Jitsi, for example), do localize even OTR errors in ?OTR messages.

The issue lies at line https://github.com/python-otr/pure-python-otr/blob/master/src/potr/proto.py#L55

For example, an implementation throwing a message such as

?OTR Error: ééààùù

at a potr OTR plugin, will effecively make it attempt to interpret non-ascii chars as ascii, and will raise an exception that the downstream OTR plugin should not have to handle. This is our job.

The suggested fix would be to interpret it as UTF8, thus conforming with the rest of our string decoding.

Funny how this ties in with issue #54 :)

[koolfy]

From what I can see, this staged commit/branch ( 6eb66ae ) should suffice in preventing this from being an issue in the future.

Adding an exception handler around the conversion might be a good addition too.

[mathieui]

This commit prevents the issue as long as the other client sends the error message in UTF-8.

Sadly, while the spec says that data messages must be utf-8, there is nothing said about error messages. We can reasonably suppose that most will be in ascii or unicode (for my use case, XMPP, it certainly is).

But we can’t be sure of that, and it might be worth looking into implementations in the future to see if they localize their error messages with other encodings than utf-8.

I think this solution is satisfactory, if it is improved by adding 'replace' (or possibly 'ignore') to the arguments given to .decode(), in order to avoid tracebacks on failed encodings. Adding a try/except block and possibly raising an exception will in my opinion only deport the issue to the people using potr, which is not a good idea (if it is shown that some error messages are in latin1 or whatever, it might be worth adding some clever encoding detection, but I hope it won’t come to this).

[koolfy]

I haven't tested this in the current state. It should be very safe but will wait confirmation that I haven't left out something stupid before meging to the staging branch.

[koolfy]

Merged to staging, closing