tacacs_plus icon indicating copy to clipboard operation
tacacs_plus copied to clipboard

API inconsistencies between Python 2 & 3 (string handling)

Open kevinboulain opened this issue 6 years ago • 2 comments

Hi there,

This isn't a major issue as user code may work around it but it may be nice to look into it (because this library seems to be the only serious one available for the TACACS+ protocol) and a compatible API between different Python versions may be a nice thing to have (maybe it was simply overlooked because the code makes heavy use of six).

I think there is a small inconsistency with the handling of strings between Python 2 & 3, a simple and idiomatic user code that could be executed under both major versions would be something like:

from __future__ import print_function, unicode_literals
import tacacs_plus.client

c = tacacs_plus.client.TACACSClient('tacacs', 49, 'testing123')
print(c.authenticate('test', 'toto').valid)

This would work in Python 3 but would fails in Python 2 (in multiple places) due to unicode_literals as the library is expecting str and not unicode strings.

Regards.

kevinboulain avatar Sep 15 '17 08:09 kevinboulain

Yeah, I think your analisys is right. There is a lot going on deep in the structure packing and unpacking with strings, bytes and unicode strings. I think we need to make some sort of sanitisation. The code works better in py3 thou. For python 2 there is need of some sort of user adjustment .

crisidev avatar Sep 15 '17 08:09 crisidev

Okay, so unless I am really misunderstanding something, the use of six.b in the code may produce unexpected results for those using the library:

Python 3 code:

# coding: utf-8
import tacacs_plus.client

# I have to give the library a latin-1 string so the unicode string is correctly interpreted
user = password = 'é'.encode('utf-8').decode('latin-1') 

c = tacacs_plus.client.TACACSClient('tacacs', 49, 'testing123')
print(c.authenticate(user, password).valid)

TACACS+ configuration:

user = é {
  login = cleartext "é"
  service = exec {
    priv-lvl = 15
  }
}

I'm not sure why six.b defaults to latin-1 but this should maybe given a thought.

kevinboulain avatar Sep 26 '17 14:09 kevinboulain