From 85f412864fe7734d1f477205c56ec363de8012b6 Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Tue, 24 Dec 2024 10:51:03 -0300 Subject: [PATCH] fix: render library v2 assets with whitespace (#35974) Assets that contain whitespace fail to be rendered when uploaded to library v2 --- .../tests/test_static_assets.py | 7 +++++++ .../djangoapps/content_libraries/views.py | 1 + .../xblock/runtime/learning_core_runtime.py | 20 +++++++++++++++---- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py b/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py index 69f5cd2d797c..91d9556b01f1 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py @@ -63,6 +63,13 @@ def test_asset_filenames(self): file_name = "a////////b" self._set_library_block_asset(block_id, file_name, SVG_DATA, expect_response=400) + # Names with spaces are allowed but replaced with underscores + file_name_with_space = "o w o.svg" + self._set_library_block_asset(block_id, file_name_with_space, SVG_DATA) + file_name = "o_w_o.svg" + assert self._get_library_block_asset(block_id, file_name)['path'] == file_name + assert self._get_library_block_asset(block_id, file_name)['size'] == file_size + def test_video_transcripts(self): """ Test that video blocks can read transcript files out of learning core. diff --git a/openedx/core/djangoapps/content_libraries/views.py b/openedx/core/djangoapps/content_libraries/views.py index 94197f508775..c8c91d50331f 100644 --- a/openedx/core/djangoapps/content_libraries/views.py +++ b/openedx/core/djangoapps/content_libraries/views.py @@ -799,6 +799,7 @@ def put(self, request, usage_key_str, file_path): """ Replace a static asset file belonging to this block. """ + file_path = file_path.replace(" ", "_") # Messes up url/name correspondence due to URL encoding. usage_key = LibraryUsageLocatorV2.from_string(usage_key_str) api.require_permission_for_library_key( usage_key.lib_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, diff --git a/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py b/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py index dde2084e54cd..44dedcf42874 100644 --- a/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py @@ -6,6 +6,7 @@ import logging from collections import defaultdict from datetime import datetime, timezone +from urllib.parse import unquote from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.db.transaction import atomic @@ -446,9 +447,20 @@ def _lookup_asset_url(self, block: XBlock, asset_path: str) -> str | None: .get(key=f"static/{asset_path}") ) except ObjectDoesNotExist: - # This means we see a path that _looks_ like it should be a static - # asset for this Component, but that static asset doesn't really - # exist. - return None + try: + # Retry with unquoted path. We don't always unquote because it would not + # be backwards-compatible, but we need to try both. + asset_path = unquote(asset_path) + content = ( + component_version + .componentversioncontent_set + .filter(content__has_file=True) + .get(key=f"static/{asset_path}") + ) + except ObjectDoesNotExist: + # This means we see a path that _looks_ like it should be a static + # asset for this Component, but that static asset doesn't really + # exist. + return None return self._absolute_url_for_asset(component_version, asset_path)