From 5cddf822e5874a222bfbd2828a65f424e7253383 Mon Sep 17 00:00:00 2001 From: Akhilesh Sharma <akhilsharma3332@gmail.com> Date: Fri, 22 Nov 2024 23:12:50 +0530 Subject: [PATCH 01/11] Fixes: Handle importing UTF-16 encoded CSV files (#550) The issue was that all CSV files were being read in UTF-8 format, causing failures when importing UTF-16 encoded files. - Added support for detecting file encoding using the popular Python library chardet. - If chardet fails to recognize the encoding, explicitly handle the files as UTF-16. This change ensures proper handling of both UTF-8 and UTF-16 encoded CSV files. Fixes #550 --- openwisp_radius/base/models.py | 4 +++- openwisp_radius/utils.py | 20 +++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/openwisp_radius/base/models.py b/openwisp_radius/base/models.py index a27d9c63..c9a88881 100644 --- a/openwisp_radius/base/models.py +++ b/openwisp_radius/base/models.py @@ -7,6 +7,7 @@ from datetime import timedelta from io import StringIO +import chardet import phonenumbers import swapper from django.conf import settings @@ -951,7 +952,8 @@ def csvfile_upload( if not csvfile: csvfile = self.csvfile csv_data = csvfile.read() - csv_data = csv_data.decode('utf-8') if isinstance(csv_data, bytes) else csv_data + if isinstance(csv_data, bytes): + csv_data = csv_data.decode(chardet.detect(csv_data)['encoding']) reader = csv.reader(StringIO(csv_data), delimiter=',') self.full_clean() self.save() diff --git a/openwisp_radius/utils.py b/openwisp_radius/utils.py index 499059fb..ca7862e9 100644 --- a/openwisp_radius/utils.py +++ b/openwisp_radius/utils.py @@ -4,6 +4,7 @@ from datetime import timedelta from io import BytesIO, StringIO +import chardet import swapper from django.conf import settings from django.contrib.auth import get_user_model @@ -129,10 +130,27 @@ def find_available_username(username, users_list, prefix=False): return tmp +def decode_csv_data(csv_data): + if isinstance(csv_data, bytes): + # Detect encoding using chardet + detected = chardet.detect(csv_data) + detected_encoding = detected.get('encoding') + + # Explicit handling for UTF-16 encodings (check for BOM) + if csv_data.startswith(b'\xff\xfe') or csv_data.startswith(b'\xfe\xff'): + detected_encoding = 'utf-16' + detected_encoding = detected_encoding or 'utf-8' + try: + return csv_data.decode(detected_encoding) + except UnicodeDecodeError as e: + raise + return csv_data + + def validate_csvfile(csvfile): csv_data = csvfile.read() try: - csv_data = csv_data.decode('utf-8') if isinstance(csv_data, bytes) else csv_data + csv_data = decode_csv_data(csv_data) except UnicodeDecodeError: raise ValidationError( _( From e02605d8b3f1c3a8de43584f29c3b3f26ef522d0 Mon Sep 17 00:00:00 2001 From: Akhilesh Sharma <akhilsharma3332@gmail.com> Date: Tue, 26 Nov 2024 00:23:25 +0530 Subject: [PATCH 02/11] Added unit testing for "get_encoding_format()" function testing for "utf-8", "utf-16", "empty data", and invalid data. --- openwisp_radius/tests/test_utils.py | 19 ++++++++++++++- openwisp_radius/utils.py | 38 +++++++++++++++++------------ 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/openwisp_radius/tests/test_utils.py b/openwisp_radius/tests/test_utils.py index ce4d84d7..26a2f38a 100644 --- a/openwisp_radius/tests/test_utils.py +++ b/openwisp_radius/tests/test_utils.py @@ -2,7 +2,7 @@ from django.core.exceptions import ValidationError from django.test import override_settings -from ..utils import find_available_username, get_one_time_login_url, validate_csvfile +from ..utils import find_available_username, get_one_time_login_url, validate_csvfile, get_encoding_format from . import FileMixin from .mixins import BaseTestCase @@ -34,6 +34,23 @@ def test_validate_csvfile(self): validate_csvfile(open(improper_csv_path, 'rt')) self.assertTrue('Improper CSV format' in error.exception.message) + def test_get_encoding_format(self): + test_cases = [ + # UTF-8 encoded data + (b'hello world', 'utf-8', "UTF-8 encoded data"), + # UTF-16 encoded data with BOM + (b'\xff\xfeh\x00e\x00l\x00l\x00o\x00 \x00w\x00o\x00r\x00l\x00d\x00', 'utf-16', "UTF-16 encoded data with BOM"), + # Empty data + (b'', 'utf-8', "Empty byte data"), + # Invalid encoding data + (b'\x80\x81\x82', 'utf-8', "Invalid encoding data") + ] + + for data, expected, description in test_cases: + with self.subTest(description=description): + result = get_encoding_format(data) + self.assertEqual(result, expected) + @override_settings(AUTHENTICATION_BACKENDS=[]) def test_get_one_time_login_url(self): login_url = get_one_time_login_url(None, None) diff --git a/openwisp_radius/utils.py b/openwisp_radius/utils.py index ca7862e9..2ef8ac59 100644 --- a/openwisp_radius/utils.py +++ b/openwisp_radius/utils.py @@ -130,27 +130,35 @@ def find_available_username(username, users_list, prefix=False): return tmp -def decode_csv_data(csv_data): - if isinstance(csv_data, bytes): - # Detect encoding using chardet - detected = chardet.detect(csv_data) - detected_encoding = detected.get('encoding') - - # Explicit handling for UTF-16 encodings (check for BOM) - if csv_data.startswith(b'\xff\xfe') or csv_data.startswith(b'\xfe\xff'): - detected_encoding = 'utf-16' - detected_encoding = detected_encoding or 'utf-8' + +def get_encoding_format(csv_data): + + # Explicit handling for UTF-16 encodings (check for BOM) + if csv_data.startswith(b'\xff\xfe') or csv_data.startswith(b'\xfe\xff'): + return 'utf-16' + + # Detect encoding using chardet + detected = chardet.detect(csv_data) + detected_encoding = detected.get('encoding') + + if detected_encoding == 'ascii': + return 'utf-8' + + if detected_encoding == 'utf-16le': try: - return csv_data.decode(detected_encoding) - except UnicodeDecodeError as e: - raise - return csv_data + csv_data.decode('utf-16le') # Test if decoding works + return detected_encoding + except UnicodeDecodeError: + pass + + return detected_encoding or 'utf-8' def validate_csvfile(csvfile): csv_data = csvfile.read() try: - csv_data = decode_csv_data(csv_data) + if isinstance(csv_data, bytes): + csv_data = csv_data.decode(get_encoding_format(csv_data)) except UnicodeDecodeError: raise ValidationError( _( From bf3947c7765731cef3e4c0617682d0f6ad21aeb8 Mon Sep 17 00:00:00 2001 From: Akhilesh Sharma <akhilsharma3332@gmail.com> Date: Wed, 27 Nov 2024 16:34:29 +0530 Subject: [PATCH 03/11] Add unit test, reproducing the bug#550 in unit testing, added files (which was producing error) earlier. - test_batch_utf16_file2.csv = Eap.G720.accountstest.csv - test_batch_utf16_file1.csv = import-bug-utf16.csv --- .../tests/static/test_batch_utf16_file1.csv | Bin 0 -> 132 bytes .../tests/static/test_batch_utf16_file2.csv | 1 + openwisp_radius/tests/test_utils.py | 8 +++++++- requirements-test.txt | 1 + 4 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 openwisp_radius/tests/static/test_batch_utf16_file1.csv create mode 100644 openwisp_radius/tests/static/test_batch_utf16_file2.csv diff --git a/openwisp_radius/tests/static/test_batch_utf16_file1.csv b/openwisp_radius/tests/static/test_batch_utf16_file1.csv new file mode 100644 index 0000000000000000000000000000000000000000..4ea3e163568ae3181f58102da05abf22210da459 GIT binary patch literal 132 zcmZQDWiVxMVK8EFV{io0E)3=j#z1Vzpu>>Nki(G5kjPNPPy(bYfTRjg-hiPT$n#+^ q17ZuHDia`dVKBtd?*P_S3?%h{Cgn5a0@a2x1T(lY1OZhev$+7@WD?x~ literal 0 HcmV?d00001 diff --git a/openwisp_radius/tests/static/test_batch_utf16_file2.csv b/openwisp_radius/tests/static/test_batch_utf16_file2.csv new file mode 100644 index 00000000..57961928 --- /dev/null +++ b/openwisp_radius/tests/static/test_batch_utf16_file2.csv @@ -0,0 +1 @@ +44D1FADD7379,cleartext$D0weL6L8,44D1FADD7379@umoja.com,EAPUSER1,USER1 \ No newline at end of file diff --git a/openwisp_radius/tests/test_utils.py b/openwisp_radius/tests/test_utils.py index 26a2f38a..bc940b1a 100644 --- a/openwisp_radius/tests/test_utils.py +++ b/openwisp_radius/tests/test_utils.py @@ -23,7 +23,13 @@ def test_validate_file_format(self): 'Unrecognized file format, the supplied file does not look like a CSV file.' in error.exception.message ) - + + utf_16_file_1_format_path = self._get_path('static/test_batch_utf16_file1.csv') + assert validate_csvfile(open(utf_16_file_1_format_path, 'rb')) is None + + utf_16_file_2_format_path = self._get_path('static/test_batch_utf16_file2.csv') + assert validate_csvfile(open(utf_16_file_2_format_path, 'rb')) is None + def test_validate_csvfile(self): invalid_csv_path = self._get_path('static/test_batch_invalid.csv') improper_csv_path = self._get_path('static/test_batch_improper.csv') diff --git a/requirements-test.txt b/requirements-test.txt index c5c0d8c3..4b6c3a96 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -13,3 +13,4 @@ openwisp-monitoring @ https://github.com/openwisp/openwisp-monitoring/tarball/ma django-redis~=5.4.0 mock-ssh-server~=0.9.1 channels_redis~=4.2.1 +chardet From 01fe5c4e2273cfd915d331825ca0686c636880b9 Mon Sep 17 00:00:00 2001 From: Akhilesh Sharma <akhilsharma3332@gmail.com> Date: Wed, 27 Nov 2024 19:38:27 +0530 Subject: [PATCH 04/11] Refactored code (defined a seperate function for the assertion), sepecified the "chardet" dependency version --- openwisp_radius/tests/test_utils.py | 6 +++--- requirements-test.txt | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openwisp_radius/tests/test_utils.py b/openwisp_radius/tests/test_utils.py index bc940b1a..f8f9d207 100644 --- a/openwisp_radius/tests/test_utils.py +++ b/openwisp_radius/tests/test_utils.py @@ -23,13 +23,13 @@ def test_validate_file_format(self): 'Unrecognized file format, the supplied file does not look like a CSV file.' in error.exception.message ) - + + def test_validate_utf16_file_format(self): utf_16_file_1_format_path = self._get_path('static/test_batch_utf16_file1.csv') assert validate_csvfile(open(utf_16_file_1_format_path, 'rb')) is None - utf_16_file_2_format_path = self._get_path('static/test_batch_utf16_file2.csv') assert validate_csvfile(open(utf_16_file_2_format_path, 'rb')) is None - + def test_validate_csvfile(self): invalid_csv_path = self._get_path('static/test_batch_invalid.csv') improper_csv_path = self._get_path('static/test_batch_improper.csv') diff --git a/requirements-test.txt b/requirements-test.txt index 4b6c3a96..6e85ad54 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -13,4 +13,4 @@ openwisp-monitoring @ https://github.com/openwisp/openwisp-monitoring/tarball/ma django-redis~=5.4.0 mock-ssh-server~=0.9.1 channels_redis~=4.2.1 -chardet +chardet~=4.0.0 From a45fd96b4c30421da7243cd8b8d3367ada654d5a Mon Sep 17 00:00:00 2001 From: Akhilesh Sharma <akhilsharma3332@gmail.com> Date: Thu, 28 Nov 2024 23:48:02 +0530 Subject: [PATCH 05/11] Improved logic of the `get_encoding_format` function. --- openwisp_radius/base/models.py | 5 ++--- openwisp_radius/utils.py | 13 +++++-------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/openwisp_radius/base/models.py b/openwisp_radius/base/models.py index c9a88881..68f1e3fc 100644 --- a/openwisp_radius/base/models.py +++ b/openwisp_radius/base/models.py @@ -7,7 +7,6 @@ from datetime import timedelta from io import StringIO -import chardet import phonenumbers import swapper from django.conf import settings @@ -54,6 +53,7 @@ load_model, prefix_generate_users, validate_csvfile, + get_encoding_format ) from .validators import ipv6_network_validator, password_reset_url_validator @@ -952,8 +952,7 @@ def csvfile_upload( if not csvfile: csvfile = self.csvfile csv_data = csvfile.read() - if isinstance(csv_data, bytes): - csv_data = csv_data.decode(chardet.detect(csv_data)['encoding']) + csv_data = csv_data.decode(get_encoding_format(csv_data)) if isinstance(csv_data, bytes) else csv_data reader = csv.reader(StringIO(csv_data), delimiter=',') self.full_clean() self.save() diff --git a/openwisp_radius/utils.py b/openwisp_radius/utils.py index 2ef8ac59..56e3b8cb 100644 --- a/openwisp_radius/utils.py +++ b/openwisp_radius/utils.py @@ -145,20 +145,17 @@ def get_encoding_format(csv_data): return 'utf-8' if detected_encoding == 'utf-16le': - try: - csv_data.decode('utf-16le') # Test if decoding works - return detected_encoding - except UnicodeDecodeError: - pass - + return "utf-16le" + return detected_encoding or 'utf-8' def validate_csvfile(csvfile): csv_data = csvfile.read() try: - if isinstance(csv_data, bytes): - csv_data = csv_data.decode(get_encoding_format(csv_data)) + # if isinstance(csv_data, bytes): + # csv_data = csv_data.decode(get_encoding_format(csv_data)) + csv_data = csv_data.decode(get_encoding_format(csv_data)) if isinstance(csv_data, bytes) else csv_data except UnicodeDecodeError: raise ValidationError( _( From 1d7a7b33e5fdf0c942251874e3e99a4894aa6f8d Mon Sep 17 00:00:00 2001 From: Akhilesh Sharma <akhilsharma3332@gmail.com> Date: Fri, 29 Nov 2024 00:05:33 +0530 Subject: [PATCH 06/11] removed unwanted comments --- openwisp_radius/utils.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/openwisp_radius/utils.py b/openwisp_radius/utils.py index 56e3b8cb..caa29727 100644 --- a/openwisp_radius/utils.py +++ b/openwisp_radius/utils.py @@ -137,13 +137,10 @@ def get_encoding_format(csv_data): if csv_data.startswith(b'\xff\xfe') or csv_data.startswith(b'\xfe\xff'): return 'utf-16' - # Detect encoding using chardet - detected = chardet.detect(csv_data) - detected_encoding = detected.get('encoding') + detected_encoding = chardet.detect(csv_data).get('encoding') if detected_encoding == 'ascii': return 'utf-8' - if detected_encoding == 'utf-16le': return "utf-16le" @@ -153,8 +150,6 @@ def get_encoding_format(csv_data): def validate_csvfile(csvfile): csv_data = csvfile.read() try: - # if isinstance(csv_data, bytes): - # csv_data = csv_data.decode(get_encoding_format(csv_data)) csv_data = csv_data.decode(get_encoding_format(csv_data)) if isinstance(csv_data, bytes) else csv_data except UnicodeDecodeError: raise ValidationError( From 6fa896f4a229951e046ab3e83860cd3444e03ac1 Mon Sep 17 00:00:00 2001 From: Akhilesh Sharma <akhilsharma3332@gmail.com> Date: Wed, 4 Dec 2024 10:51:30 +0530 Subject: [PATCH 07/11] [change] Refactored code by running openwisp-qa-format --- CHANGES.rst | 4 ++-- README.rst | 4 ++-- docs/developer/extending.rst | 8 ++------ docs/user/settings.rst | 8 ++------ openwisp_radius/api/views.py | 6 +++--- openwisp_radius/base/forms.py | 4 +++- openwisp_radius/base/models.py | 8 ++++++-- openwisp_radius/receivers.py | 1 + .../tests/static/test_batch_utf16_file2.csv | 2 +- openwisp_radius/tests/test_saml/utils.py | 6 +++--- openwisp_radius/tests/test_utils.py | 19 ++++++++++++++----- openwisp_radius/utils.py | 12 +++++++----- 12 files changed, 46 insertions(+), 36 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 5e88d17c..fdd3aeae 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -8,8 +8,8 @@ Features ~~~~~~~~ - Added integration with `OpenWISP Monitoring - <https://openwisp.io/docs/stable/radius/user/radius_monitoring.html>`_ to - collect and visualize metrics for user-signups and RADIUS traffic. + <https://openwisp.io/docs/stable/radius/user/radius_monitoring.html>`_ + to collect and visualize metrics for user-signups and RADIUS traffic. - Added support for `Change of Authorization (CoA) <https://openwisp.io/docs/stable/radius/user/change_of_authorization.html>`_. - Added `MonthlyTrafficCounter diff --git a/README.rst b/README.rst index 2232c150..9e6bd574 100644 --- a/README.rst +++ b/README.rst @@ -46,8 +46,8 @@ verification `generation of new users for events <https://openwisp.io/docs/stable/radius/user/generating_users.html>`_, `social login -<https://openwisp.io/docs/stable/radius/user/social_login.html>`_, and much -more. +<https://openwisp.io/docs/stable/radius/user/social_login.html>`_, and +much more. It can be used as a standalone application or integrated with the rest of `OpenWISP <https://openwisp.org>`_. It can also be used as a `base system diff --git a/docs/developer/extending.rst b/docs/developer/extending.rst index 95f0cb54..5927e07c 100644 --- a/docs/developer/extending.rst +++ b/docs/developer/extending.rst @@ -206,9 +206,7 @@ Once you have created the models, add the following to your OPENWISP_RADIUS_RADIUSGROUP_MODEL = "myradius.RadiusGroup" OPENWISP_RADIUS_RADIUSTOKEN_MODEL = "myradius.RadiusToken" OPENWISP_RADIUS_PHONETOKEN_MODEL = "myradius.PhoneToken" - OPENWISP_RADIUS_ORGANIZATIONRADIUSSETTINGS_MODEL = ( - "myradius.OrganizationRadiusSettings" - ) + OPENWISP_RADIUS_ORGANIZATIONRADIUSSETTINGS_MODEL = "myradius.OrganizationRadiusSettings" OPENWISP_RADIUS_REGISTEREDUSER_MODEL = "myradius.RegisteredUser" # You will need to change AUTH_USER_MODEL if you are extending openwisp_users @@ -315,9 +313,7 @@ resort to monkey patching, you can proceed as follows: RadiusGroupCheck = load_model("openwisp_radius", "RadiusGroupCheck") RadiusGroupReply = load_model("openwisp_radius", "RadiusGroupReply") RadiusUserGroup = load_model("openwisp_radius", "RadiusUserGroup") - OrganizationRadiusSettings = load_model( - "openwisp_radius", "OrganizationRadiusSettings" - ) + OrganizationRadiusSettings = load_model("openwisp_radius", "OrganizationRadiusSettings") User = get_user_model() admin.site.unregister(RadiusCheck) diff --git a/docs/user/settings.rst b/docs/user/settings.rst index 1c8dd4d7..c29f4242 100644 --- a/docs/user/settings.rst +++ b/docs/user/settings.rst @@ -539,9 +539,7 @@ means that the global setting specified in ``settings.py`` will be used. .. code-block:: python - { - "__all__": "https://{site}/{organization}/password/reset/confirm/{uid}/{token}" - } + {"__all__": "https://{site}/{organization}/password/reset/confirm/{uid}/{token}"} A dictionary representing the frontend URLs through which end users can complete the password reset operation. @@ -871,9 +869,7 @@ type in the API response for ``ChilliSpot-Max-Input-Octets`` attribute: .. code-block:: python - OPENWISP_RADIUS_RADIUS_ATTRIBUTES_TYPE_MAP = { - "ChilliSpot-Max-Input-Octets": "bytes" - } + OPENWISP_RADIUS_RADIUS_ATTRIBUTES_TYPE_MAP = {"ChilliSpot-Max-Input-Octets": "bytes"} .. _radius_social_login_settings: diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index 6bb21916..30190cad 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -155,9 +155,9 @@ def get(self, request, *args, **kwargs): if radbatch.strategy == 'prefix': pdf = generate_pdf(radbatch.pk) response = HttpResponse(content_type='application/pdf') - response[ - 'Content-Disposition' - ] = f'attachment; filename="{radbatch.name}.pdf"' + response['Content-Disposition'] = ( + f'attachment; filename="{radbatch.name}.pdf"' + ) response.write(pdf) return response else: diff --git a/openwisp_radius/base/forms.py b/openwisp_radius/base/forms.py index d6d28637..fadcee56 100644 --- a/openwisp_radius/base/forms.py +++ b/openwisp_radius/base/forms.py @@ -46,7 +46,9 @@ def clean(self): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) if 'csvfile' in self.fields: - docs_link = 'https://openwisp.io/docs/stable/radius/user/importing_users.html' + docs_link = ( + 'https://openwisp.io/docs/stable/radius/user/importing_users.html' + ) help_text = f"Refer to the <b><u><a href='{docs_link}'>docs</a></u></b> \ for more details on importing users from a CSV" self.fields['csvfile'].help_text = help_text diff --git a/openwisp_radius/base/models.py b/openwisp_radius/base/models.py index 68f1e3fc..5e816851 100644 --- a/openwisp_radius/base/models.py +++ b/openwisp_radius/base/models.py @@ -49,11 +49,11 @@ SmsMessage, find_available_username, generate_sms_token, + get_encoding_format, get_sms_default_valid_until, load_model, prefix_generate_users, validate_csvfile, - get_encoding_format ) from .validators import ipv6_network_validator, password_reset_url_validator @@ -952,7 +952,11 @@ def csvfile_upload( if not csvfile: csvfile = self.csvfile csv_data = csvfile.read() - csv_data = csv_data.decode(get_encoding_format(csv_data)) if isinstance(csv_data, bytes) else csv_data + csv_data = ( + csv_data.decode(get_encoding_format(csv_data)) + if isinstance(csv_data, bytes) + else csv_data + ) reader = csv.reader(StringIO(csv_data), delimiter=',') self.full_clean() self.save() diff --git a/openwisp_radius/receivers.py b/openwisp_radius/receivers.py index a3a999be..c19299d9 100644 --- a/openwisp_radius/receivers.py +++ b/openwisp_radius/receivers.py @@ -1,6 +1,7 @@ """ Receiver functions for django signals (eg: post_save) """ + import logging from celery.exceptions import OperationalError diff --git a/openwisp_radius/tests/static/test_batch_utf16_file2.csv b/openwisp_radius/tests/static/test_batch_utf16_file2.csv index 57961928..dcd4a3fc 100644 --- a/openwisp_radius/tests/static/test_batch_utf16_file2.csv +++ b/openwisp_radius/tests/static/test_batch_utf16_file2.csv @@ -1 +1 @@ -44D1FADD7379,cleartext$D0weL6L8,44D1FADD7379@umoja.com,EAPUSER1,USER1 \ No newline at end of file +44D1FADD7379,cleartext$D0weL6L8,44D1FADD7379@umoja.com,EAPUSER1,USER1 diff --git a/openwisp_radius/tests/test_saml/utils.py b/openwisp_radius/tests/test_saml/utils.py index 3c173780..27a57099 100644 --- a/openwisp_radius/tests/test_saml/utils.py +++ b/openwisp_radius/tests/test_saml/utils.py @@ -25,6 +25,6 @@ def add_outstanding_query(self, session_id, came_from): came_from, ) self.saml_session.save() - self.client.cookies[ - settings.SESSION_COOKIE_NAME - ] = self.saml_session.session_key + self.client.cookies[settings.SESSION_COOKIE_NAME] = ( + self.saml_session.session_key + ) diff --git a/openwisp_radius/tests/test_utils.py b/openwisp_radius/tests/test_utils.py index f8f9d207..0e5b60c7 100644 --- a/openwisp_radius/tests/test_utils.py +++ b/openwisp_radius/tests/test_utils.py @@ -2,7 +2,12 @@ from django.core.exceptions import ValidationError from django.test import override_settings -from ..utils import find_available_username, get_one_time_login_url, validate_csvfile, get_encoding_format +from ..utils import ( + find_available_username, + get_encoding_format, + get_one_time_login_url, + validate_csvfile, +) from . import FileMixin from .mixins import BaseTestCase @@ -23,7 +28,7 @@ def test_validate_file_format(self): 'Unrecognized file format, the supplied file does not look like a CSV file.' in error.exception.message ) - + def test_validate_utf16_file_format(self): utf_16_file_1_format_path = self._get_path('static/test_batch_utf16_file1.csv') assert validate_csvfile(open(utf_16_file_1_format_path, 'rb')) is None @@ -45,18 +50,22 @@ def test_get_encoding_format(self): # UTF-8 encoded data (b'hello world', 'utf-8', "UTF-8 encoded data"), # UTF-16 encoded data with BOM - (b'\xff\xfeh\x00e\x00l\x00l\x00o\x00 \x00w\x00o\x00r\x00l\x00d\x00', 'utf-16', "UTF-16 encoded data with BOM"), + ( + b'\xff\xfeh\x00e\x00l\x00l\x00o\x00 \x00w\x00o\x00r\x00l\x00d\x00', + 'utf-16', + "UTF-16 encoded data with BOM", + ), # Empty data (b'', 'utf-8', "Empty byte data"), # Invalid encoding data - (b'\x80\x81\x82', 'utf-8', "Invalid encoding data") + (b'\x80\x81\x82', 'utf-8', "Invalid encoding data"), ] for data, expected, description in test_cases: with self.subTest(description=description): result = get_encoding_format(data) self.assertEqual(result, expected) - + @override_settings(AUTHENTICATION_BACKENDS=[]) def test_get_one_time_login_url(self): login_url = get_one_time_login_url(None, None) diff --git a/openwisp_radius/utils.py b/openwisp_radius/utils.py index caa29727..701626ba 100644 --- a/openwisp_radius/utils.py +++ b/openwisp_radius/utils.py @@ -130,27 +130,29 @@ def find_available_username(username, users_list, prefix=False): return tmp - def get_encoding_format(csv_data): - # Explicit handling for UTF-16 encodings (check for BOM) if csv_data.startswith(b'\xff\xfe') or csv_data.startswith(b'\xfe\xff'): return 'utf-16' detected_encoding = chardet.detect(csv_data).get('encoding') - if detected_encoding == 'ascii': + if detected_encoding == 'ascii': return 'utf-8' if detected_encoding == 'utf-16le': return "utf-16le" - + return detected_encoding or 'utf-8' def validate_csvfile(csvfile): csv_data = csvfile.read() try: - csv_data = csv_data.decode(get_encoding_format(csv_data)) if isinstance(csv_data, bytes) else csv_data + csv_data = ( + csv_data.decode(get_encoding_format(csv_data)) + if isinstance(csv_data, bytes) + else csv_data + ) except UnicodeDecodeError: raise ValidationError( _( From e3417357a29aca9b46bd37bec207c6f407fba7b0 Mon Sep 17 00:00:00 2001 From: Akhilesh Sharma <akhilsharma3332@gmail.com> Date: Wed, 4 Dec 2024 21:15:49 +0530 Subject: [PATCH 08/11] [changes] refactored using `black==23.12.1" --- docs/developer/extending.rst | 8 ++++++-- docs/user/settings.rst | 8 ++++++-- openwisp_radius/api/views.py | 6 +++--- openwisp_radius/tests/test_saml/utils.py | 6 +++--- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/docs/developer/extending.rst b/docs/developer/extending.rst index 5927e07c..95f0cb54 100644 --- a/docs/developer/extending.rst +++ b/docs/developer/extending.rst @@ -206,7 +206,9 @@ Once you have created the models, add the following to your OPENWISP_RADIUS_RADIUSGROUP_MODEL = "myradius.RadiusGroup" OPENWISP_RADIUS_RADIUSTOKEN_MODEL = "myradius.RadiusToken" OPENWISP_RADIUS_PHONETOKEN_MODEL = "myradius.PhoneToken" - OPENWISP_RADIUS_ORGANIZATIONRADIUSSETTINGS_MODEL = "myradius.OrganizationRadiusSettings" + OPENWISP_RADIUS_ORGANIZATIONRADIUSSETTINGS_MODEL = ( + "myradius.OrganizationRadiusSettings" + ) OPENWISP_RADIUS_REGISTEREDUSER_MODEL = "myradius.RegisteredUser" # You will need to change AUTH_USER_MODEL if you are extending openwisp_users @@ -313,7 +315,9 @@ resort to monkey patching, you can proceed as follows: RadiusGroupCheck = load_model("openwisp_radius", "RadiusGroupCheck") RadiusGroupReply = load_model("openwisp_radius", "RadiusGroupReply") RadiusUserGroup = load_model("openwisp_radius", "RadiusUserGroup") - OrganizationRadiusSettings = load_model("openwisp_radius", "OrganizationRadiusSettings") + OrganizationRadiusSettings = load_model( + "openwisp_radius", "OrganizationRadiusSettings" + ) User = get_user_model() admin.site.unregister(RadiusCheck) diff --git a/docs/user/settings.rst b/docs/user/settings.rst index c29f4242..1c8dd4d7 100644 --- a/docs/user/settings.rst +++ b/docs/user/settings.rst @@ -539,7 +539,9 @@ means that the global setting specified in ``settings.py`` will be used. .. code-block:: python - {"__all__": "https://{site}/{organization}/password/reset/confirm/{uid}/{token}"} + { + "__all__": "https://{site}/{organization}/password/reset/confirm/{uid}/{token}" + } A dictionary representing the frontend URLs through which end users can complete the password reset operation. @@ -869,7 +871,9 @@ type in the API response for ``ChilliSpot-Max-Input-Octets`` attribute: .. code-block:: python - OPENWISP_RADIUS_RADIUS_ATTRIBUTES_TYPE_MAP = {"ChilliSpot-Max-Input-Octets": "bytes"} + OPENWISP_RADIUS_RADIUS_ATTRIBUTES_TYPE_MAP = { + "ChilliSpot-Max-Input-Octets": "bytes" + } .. _radius_social_login_settings: diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index 30190cad..6bb21916 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -155,9 +155,9 @@ def get(self, request, *args, **kwargs): if radbatch.strategy == 'prefix': pdf = generate_pdf(radbatch.pk) response = HttpResponse(content_type='application/pdf') - response['Content-Disposition'] = ( - f'attachment; filename="{radbatch.name}.pdf"' - ) + response[ + 'Content-Disposition' + ] = f'attachment; filename="{radbatch.name}.pdf"' response.write(pdf) return response else: diff --git a/openwisp_radius/tests/test_saml/utils.py b/openwisp_radius/tests/test_saml/utils.py index 27a57099..3c173780 100644 --- a/openwisp_radius/tests/test_saml/utils.py +++ b/openwisp_radius/tests/test_saml/utils.py @@ -25,6 +25,6 @@ def add_outstanding_query(self, session_id, came_from): came_from, ) self.saml_session.save() - self.client.cookies[settings.SESSION_COOKIE_NAME] = ( - self.saml_session.session_key - ) + self.client.cookies[ + settings.SESSION_COOKIE_NAME + ] = self.saml_session.session_key From ee0b2cf7101a499828bcf1c56e7d82205396bb57 Mon Sep 17 00:00:00 2001 From: Akhilesh Sharma <akhilsharma3332@gmail.com> Date: Thu, 5 Dec 2024 16:04:21 +0530 Subject: [PATCH 09/11] [changes] Removed use of chardet because it was unstable on utf-8 blob files, improved logic, more clean code --- openwisp_radius/base/models.py | 8 ++--- ...file2.csv => test_batch_utf8Sig_file2.csv} | 0 openwisp_radius/tests/test_utils.py | 34 ++++-------------- openwisp_radius/utils.py | 35 ++++++++++--------- 4 files changed, 27 insertions(+), 50 deletions(-) rename openwisp_radius/tests/static/{test_batch_utf16_file2.csv => test_batch_utf8Sig_file2.csv} (100%) diff --git a/openwisp_radius/base/models.py b/openwisp_radius/base/models.py index 5e816851..1103ccca 100644 --- a/openwisp_radius/base/models.py +++ b/openwisp_radius/base/models.py @@ -47,9 +47,9 @@ ) from ..utils import ( SmsMessage, + decode_byte_data, find_available_username, generate_sms_token, - get_encoding_format, get_sms_default_valid_until, load_model, prefix_generate_users, @@ -952,11 +952,7 @@ def csvfile_upload( if not csvfile: csvfile = self.csvfile csv_data = csvfile.read() - csv_data = ( - csv_data.decode(get_encoding_format(csv_data)) - if isinstance(csv_data, bytes) - else csv_data - ) + csv_data = decode_byte_data(csv_data) reader = csv.reader(StringIO(csv_data), delimiter=',') self.full_clean() self.save() diff --git a/openwisp_radius/tests/static/test_batch_utf16_file2.csv b/openwisp_radius/tests/static/test_batch_utf8Sig_file2.csv similarity index 100% rename from openwisp_radius/tests/static/test_batch_utf16_file2.csv rename to openwisp_radius/tests/static/test_batch_utf8Sig_file2.csv diff --git a/openwisp_radius/tests/test_utils.py b/openwisp_radius/tests/test_utils.py index 0e5b60c7..8626632c 100644 --- a/openwisp_radius/tests/test_utils.py +++ b/openwisp_radius/tests/test_utils.py @@ -2,12 +2,7 @@ from django.core.exceptions import ValidationError from django.test import override_settings -from ..utils import ( - find_available_username, - get_encoding_format, - get_one_time_login_url, - validate_csvfile, -) +from ..utils import find_available_username, get_one_time_login_url, validate_csvfile from . import FileMixin from .mixins import BaseTestCase @@ -32,7 +27,11 @@ def test_validate_file_format(self): def test_validate_utf16_file_format(self): utf_16_file_1_format_path = self._get_path('static/test_batch_utf16_file1.csv') assert validate_csvfile(open(utf_16_file_1_format_path, 'rb')) is None - utf_16_file_2_format_path = self._get_path('static/test_batch_utf16_file2.csv') + + def test_validate_utf8Sig_file_format(self): + utf_16_file_2_format_path = self._get_path( + 'static/test_batch_utf8Sig_file2.csv' + ) assert validate_csvfile(open(utf_16_file_2_format_path, 'rb')) is None def test_validate_csvfile(self): @@ -45,27 +44,6 @@ def test_validate_csvfile(self): validate_csvfile(open(improper_csv_path, 'rt')) self.assertTrue('Improper CSV format' in error.exception.message) - def test_get_encoding_format(self): - test_cases = [ - # UTF-8 encoded data - (b'hello world', 'utf-8', "UTF-8 encoded data"), - # UTF-16 encoded data with BOM - ( - b'\xff\xfeh\x00e\x00l\x00l\x00o\x00 \x00w\x00o\x00r\x00l\x00d\x00', - 'utf-16', - "UTF-16 encoded data with BOM", - ), - # Empty data - (b'', 'utf-8', "Empty byte data"), - # Invalid encoding data - (b'\x80\x81\x82', 'utf-8', "Invalid encoding data"), - ] - - for data, expected, description in test_cases: - with self.subTest(description=description): - result = get_encoding_format(data) - self.assertEqual(result, expected) - @override_settings(AUTHENTICATION_BACKENDS=[]) def test_get_one_time_login_url(self): login_url = get_one_time_login_url(None, None) diff --git a/openwisp_radius/utils.py b/openwisp_radius/utils.py index 701626ba..1e79df9a 100644 --- a/openwisp_radius/utils.py +++ b/openwisp_radius/utils.py @@ -4,7 +4,6 @@ from datetime import timedelta from io import BytesIO, StringIO -import chardet import swapper from django.conf import settings from django.contrib.auth import get_user_model @@ -130,29 +129,32 @@ def find_available_username(username, users_list, prefix=False): return tmp -def get_encoding_format(csv_data): - # Explicit handling for UTF-16 encodings (check for BOM) - if csv_data.startswith(b'\xff\xfe') or csv_data.startswith(b'\xfe\xff'): - return 'utf-16' +def get_encoding_format(byte_data): + # Explicitly handle some common encodings, including utf-16le + common_encodings = ['utf-8-sig', 'utf-16', 'utf-16be', 'utf-16le', 'ascii'] - detected_encoding = chardet.detect(csv_data).get('encoding') + for enc in common_encodings: + try: + byte_data.decode(enc) + return enc + except (UnicodeDecodeError, TypeError): + continue - if detected_encoding == 'ascii': - return 'utf-8' - if detected_encoding == 'utf-16le': - return "utf-16le" + return 'utf-8' - return detected_encoding or 'utf-8' + +def decode_byte_data(data): + if isinstance(data, bytes): + data = data.decode(get_encoding_format(data)) + data = data.replace('\x00', '') # Removing null bytes + return data def validate_csvfile(csvfile): csv_data = csvfile.read() + try: - csv_data = ( - csv_data.decode(get_encoding_format(csv_data)) - if isinstance(csv_data, bytes) - else csv_data - ) + csv_data = decode_byte_data(csv_data) except UnicodeDecodeError: raise ValidationError( _( @@ -160,6 +162,7 @@ def validate_csvfile(csvfile): 'does not look like a CSV file.' ) ) + reader = csv.reader(StringIO(csv_data), delimiter=',') error_message = 'The CSV contains a line with invalid data,\ line number {} triggered the following error: {}' From 1fa28944d6251b358763814a8c9e71590dc920bd Mon Sep 17 00:00:00 2001 From: Akhilesh <akhilsharma3332@gmail.com> Date: Mon, 27 Jan 2025 14:47:31 +0530 Subject: [PATCH 10/11] [changes] Removed unused dependency chardet --- requirements-test.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements-test.txt b/requirements-test.txt index 6e85ad54..c5c0d8c3 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -13,4 +13,3 @@ openwisp-monitoring @ https://github.com/openwisp/openwisp-monitoring/tarball/ma django-redis~=5.4.0 mock-ssh-server~=0.9.1 channels_redis~=4.2.1 -chardet~=4.0.0 From ae29c396ca8aebbd3f43bf9e0fbf61c15257db0e Mon Sep 17 00:00:00 2001 From: Federico Capoano <f.capoano@openwisp.io> Date: Fri, 31 Jan 2025 11:48:14 -0300 Subject: [PATCH 11/11] [tests] Added one more utf16 encoded test file --- .../tests/static/test_batch_utf16_file2.csv | Bin 0 -> 134 bytes openwisp_radius/tests/test_utils.py | 3 +++ 2 files changed, 3 insertions(+) create mode 100644 openwisp_radius/tests/static/test_batch_utf16_file2.csv diff --git a/openwisp_radius/tests/static/test_batch_utf16_file2.csv b/openwisp_radius/tests/static/test_batch_utf16_file2.csv new file mode 100644 index 0000000000000000000000000000000000000000..0907bf990449542650bd069e21375e830a9531d6 GIT binary patch literal 134 zcmezW&y>NG!G*zy!HvNYNV_nYGZ+K0C4&w_GD8kSDnlYe5km=(t^krMKzRd(av;x# t!3>BkfT~P@(1pPeL%#!9S22*(1Dcf2kPB2B$`H)p$`Axpjm+j^008^J6xsj) literal 0 HcmV?d00001 diff --git a/openwisp_radius/tests/test_utils.py b/openwisp_radius/tests/test_utils.py index 8626632c..1503a4d7 100644 --- a/openwisp_radius/tests/test_utils.py +++ b/openwisp_radius/tests/test_utils.py @@ -28,6 +28,9 @@ def test_validate_utf16_file_format(self): utf_16_file_1_format_path = self._get_path('static/test_batch_utf16_file1.csv') assert validate_csvfile(open(utf_16_file_1_format_path, 'rb')) is None + utf_16_file_2_format_path = self._get_path('static/test_batch_utf16_file2.csv') + assert validate_csvfile(open(utf_16_file_2_format_path, 'rb')) is None + def test_validate_utf8Sig_file_format(self): utf_16_file_2_format_path = self._get_path( 'static/test_batch_utf8Sig_file2.csv'