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

Handle unicode characters gracefully in python 2 #317

Merged
merged 1 commit into from
Dec 27, 2022

Conversation

ananduri9
Copy link
Contributor

Hello there! We've come across some bugs like the following using this library's add_sign function:

File /opt/virtualenv/monolith/local/lib/python2.7/site-packages/onelogin/saml2/utils.py line 740 in add_sign
elem = OneLogin_Saml2_XML.to_etree(xml)
File /opt/virtualenv/monolith/local/lib/python2.7/site-packages/onelogin/saml2/xml_utils.py line 68 in to_etree
return OneLogin_Saml2_XML._parse_etree(compat.to_bytes(xml), forbid_dtd=True)
File /opt/virtualenv/monolith/local/lib/python2.7/site-packages/onelogin/saml2/compat.py line 42 in to_bytes
return str(data)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xc9' in position 2593: ordinal not in range(128)

It looks like we're getting this error when a user's name or other data has a non-ascii character that goes through this function - and we're still using Python 2 for this.

I believe my fix should allow for gracefully handling unicode characters in Python 2. Let me know if you have any questions/concerns.

@akx
Copy link
Collaborator

akx commented Oct 27, 2022

This is python3-saml – you should use https://github.com/onelogin/python-saml/ for Python 2...

@paulsaccount
Copy link

python3-saml is still installable via pip with Python 2 because it isn't enforcing a minimum Python. In addition, this library python3-saml wrote a compatibility file compat.py to handle issues like the one mentioned in the original issue description.

This bug is still valid for users who are using Python 2.7 and the latest version of python3-saml.

If there is a desire to stop supporting Python 2.7 support for this library, you could deprecate support and version the python3-saml so it will not install on Python 2.7 environments. However, shouldn't prevent the fix presented from being added.

@alimpon
Copy link

alimpon commented Nov 21, 2022

@akx would you be able to merge this fix in for us?

@akx
Copy link
Collaborator

akx commented Nov 22, 2022

@alimpon No, I'm not a maintainer of this project.

@alimpon
Copy link

alimpon commented Nov 22, 2022

oh sorry. I think @eriktalvi is a maintainer, could you please help merge this in?

@alimpon
Copy link

alimpon commented Dec 1, 2022

@eriktalvi bump again to merge this please

@pitbulk pitbulk force-pushed the master branch 2 times, most recently from 4fa3934 to acef8eb Compare December 26, 2022 01:53
@pitbulk pitbulk merged commit e188936 into SAML-Toolkits:master Dec 27, 2022
@ananduri9 ananduri9 deleted the fix-py2-to-bytes-unicode branch January 4, 2023 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants