From 59782fa625cb863e2beb1eaf80d1c22219f15d4f Mon Sep 17 00:00:00 2001 From: Usama Sadiq Date: Thu, 17 Aug 2023 16:59:23 +0500 Subject: [PATCH] fix: fix is_safe_url and urlquote warnings (#33041) * fix: fix is_safe_url and urlquote warnings * fix: replace urlquote with quote --- .../contentstore/views/tests/test_container_page.py | 8 ++++---- cms/djangoapps/contentstore/views/tests/test_helpers.py | 6 +++--- lms/templates/courseware/progress.html | 2 +- lms/templates/dashboard/_dashboard_course_listing.html | 7 ++++--- lms/templates/main.html | 4 ++-- openedx/core/djangoapps/user_authn/tests/test_utils.py | 2 +- openedx/core/djangoapps/user_authn/utils.py | 4 ++-- 7 files changed, 17 insertions(+), 16 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_container_page.py b/cms/djangoapps/contentstore/views/tests/test_container_page.py index 986d2c4485..7bc2616515 100644 --- a/cms/djangoapps/contentstore/views/tests/test_container_page.py +++ b/cms/djangoapps/contentstore/views/tests/test_container_page.py @@ -9,8 +9,8 @@ from unittest.mock import Mock, patch from django.http import Http404 from django.test.client import RequestFactory -from django.utils import http from pytz import UTC +from urllib.parse import quote import cms.djangoapps.contentstore.views.component as views from cms.djangoapps.contentstore.tests.test_libraries import LibraryTestCase @@ -67,10 +67,10 @@ class ContainerPageTestCase(StudioPageTestCase, LibraryTestCase): 'Lesson 1' ).format( course=re.escape(str(self.course.id)), - section_parameters=re.escape('?show={}'.format(http.urlquote( + section_parameters=re.escape('?show={}'.format(quote( str(self.chapter.location).encode() ))), - subsection_parameters=re.escape('?show={}'.format(http.urlquote( + subsection_parameters=re.escape('?show={}'.format(quote( str(self.sequential.location).encode() ))), ), @@ -97,7 +97,7 @@ class ContainerPageTestCase(StudioPageTestCase, LibraryTestCase): ).format( course=re.escape(str(self.course.id)), unit_parameters=re.escape(str(self.vertical.location)), - subsection_parameters=re.escape('?show={}'.format(http.urlquote( + subsection_parameters=re.escape('?show={}'.format(quote( str(self.sequential.location).encode() ))), ), diff --git a/cms/djangoapps/contentstore/views/tests/test_helpers.py b/cms/djangoapps/contentstore/views/tests/test_helpers.py index 10e35710da..17ed8ae2e6 100644 --- a/cms/djangoapps/contentstore/views/tests/test_helpers.py +++ b/cms/djangoapps/contentstore/views/tests/test_helpers.py @@ -3,7 +3,7 @@ Unit tests for helpers.py. """ -from django.utils import http +from urllib.parse import quote from cms.djangoapps.contentstore.tests.utils import CourseTestCase from xmodule.modulestore.tests.factories import BlockFactory, LibraryFactory # lint-amnesty, pylint: disable=wrong-import-order @@ -27,7 +27,7 @@ class HelpersTestCase(CourseTestCase): display_name="Week 1") self.assertEqual( xblock_studio_url(chapter), - f'{course_url}?show={http.urlquote(str(chapter.location).encode())}' + f'{course_url}?show={quote(str(chapter.location).encode())}' ) # Verify sequential URL @@ -35,7 +35,7 @@ class HelpersTestCase(CourseTestCase): display_name="Lesson 1") self.assertEqual( xblock_studio_url(sequential), - f'{course_url}?show={http.urlquote(str(sequential.location).encode())}' + f'{course_url}?show={quote(str(sequential.location).encode())}' ) # Verify unit URL diff --git a/lms/templates/courseware/progress.html b/lms/templates/courseware/progress.html index fd087493d1..26f1bb59cb 100644 --- a/lms/templates/courseware/progress.html +++ b/lms/templates/courseware/progress.html @@ -7,7 +7,7 @@ from datetime import datetime from django.conf import settings from django.urls import reverse -from django.utils.http import urlquote_plus +from urllib.parse import quote_plus from django.utils.translation import gettext as _ from pytz import UTC diff --git a/lms/templates/dashboard/_dashboard_course_listing.html b/lms/templates/dashboard/_dashboard_course_listing.html index 55718cb92f..ac34e4cbc3 100644 --- a/lms/templates/dashboard/_dashboard_course_listing.html +++ b/lms/templates/dashboard/_dashboard_course_listing.html @@ -4,7 +4,8 @@ import datetime from django.conf import settings -from django.utils.http import urlencode, urlquote_plus +from django.utils.http import urlencode +from urllib.parse import quote_plus from django.utils.translation import gettext as _ from django.utils.translation import ungettext from django.urls import reverse @@ -232,8 +233,8 @@ from lms.djangoapps.experiments.utils import UPSELL_TRACKING_FLAG <% twitter_share_url = u"{url}?{utm_params}".format(url=share_url, utm_params=encoded_utm_parameters['twitter']) default_share_text = _("I'm taking {course_name} online with {twitter_brand}. Check it out!").format(course_name=course_overview.display_name_with_default, twitter_brand=share_settings.get('TWITTER_BRAND', '@edxonline')) - share_text = urlquote_plus(share_settings.get('DASHBOARD_TWITTER_TEXT', default_share_text)) - twitter_url = u'https://twitter.com/intent/tweet?text=' + share_text + u'%20' + urlquote_plus(twitter_share_url) + share_text = quote_plus(share_settings.get('DASHBOARD_TWITTER_TEXT', default_share_text)) + twitter_url = u'https://twitter.com/intent/tweet?text=' + share_text + u'%20' + quote_plus(twitter_share_url) share_msg = _("Share {course_name} on Twitter").format(course_name=course_overview.display_name_with_default) %> ${ u"?next={next}".format( - next=urlquote_plus(login_redirect_url if login_redirect_url else request.path) + next=quote_plus(login_redirect_url if login_redirect_url else request.path) ) if (login_redirect_url or (request and not request.path.startswith("/logout"))) else "" } diff --git a/openedx/core/djangoapps/user_authn/tests/test_utils.py b/openedx/core/djangoapps/user_authn/tests/test_utils.py index 6f10560d9d..72eaaf8265 100644 --- a/openedx/core/djangoapps/user_authn/tests/test_utils.py +++ b/openedx/core/djangoapps/user_authn/tests/test_utils.py @@ -41,7 +41,7 @@ class TestRedirectUtils(TestCase): RedirectCase('http://edx.org/courses', 'edx.org', req_is_secure=False, expected_is_safe=True), RedirectCase('http://edx.org/courses', 'edx.org', req_is_secure=True, expected_is_safe=False), - # Django's is_safe_url protects against "///" + # Django's url_has_allowed_host_and_scheme protects against "///" RedirectCase('http:///edx.org/courses', 'edx.org', req_is_secure=True, expected_is_safe=False), ) @ddt.unpack diff --git a/openedx/core/djangoapps/user_authn/utils.py b/openedx/core/djangoapps/user_authn/utils.py index 5b70d24278..07eb5b491a 100644 --- a/openedx/core/djangoapps/user_authn/utils.py +++ b/openedx/core/djangoapps/user_authn/utils.py @@ -56,11 +56,11 @@ def is_safe_login_or_logout_redirect(redirect_to, request_host, dot_client_id, r if redirect_to in application.redirect_uris: login_redirect_whitelist.add(urlparse(redirect_to).netloc) - is_safe_url = http.is_safe_url( + url_has_allowed_host_and_scheme = http.url_has_allowed_host_and_scheme( redirect_to, allowed_hosts=login_redirect_whitelist, require_https=require_https ) - return is_safe_url + return url_has_allowed_host_and_scheme def is_registration_api_v1(request):