Skip to content

Commit

Permalink
fix: auto-strip file paths starting with '/'
Browse files Browse the repository at this point in the history
We don't allow keys that would be absolute paths. Storing thes causes
headaches with buildilng relative paths, and because of mismatches where
people are inconsistent about using leading slashes or not.
  • Loading branch information
ormsbee committed Sep 30, 2024
1 parent 98cfb10 commit 397878b
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 2 deletions.
14 changes: 14 additions & 0 deletions openedx_learning/apps/authoring/components/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,21 @@ def create_component_version_content(
) -> ComponentVersionContent:
"""
Add a Content to the given ComponentVersion
We don't allow keys that would be absolute paths, e.g. ones that start with
'/'. Storing these causes headaches with building relative paths and because
of mismatches with things that expect a leading slash and those that don't.
So for safety and consistency, we strip off leading slashes and emit a
warning when we do.
"""
if key.startswith('/'):
logger.warn(
"Absolute paths are not supported: "
f"removed leading '/' from ComponentVersion {component_version_id} "
f"content key: {repr(key)} (content_id: {content_id})"
)
key = key.lstrip('/')

cvrc, _created = ComponentVersionContent.objects.get_or_create(
component_version_id=component_version_id,
content_id=content_id,
Expand Down
21 changes: 19 additions & 2 deletions tests/openedx_learning/apps/authoring/components/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ def test_add(self):
components_api.create_component_version_content(
new_version.pk,
new_content.pk,
key="hello.txt",
key="my/path/to/hello.txt",
learner_downloadable=False,
)
# re-fetch from the database to check to see if we wrote it correctly
Expand All @@ -390,9 +390,26 @@ def test_add(self):
.get(publishable_entity_version__version_num=1)
assert (
new_content ==
new_version.contents.get(componentversioncontent__key="hello.txt")
new_version.contents.get(componentversioncontent__key="my/path/to/hello.txt")
)

# Write the same content again, but to an absolute path (should auto-
# strip) the leading '/'s.
components_api.create_component_version_content(
new_version.pk,
new_content.pk,
key="//nested/path/hello.txt",
learner_downloadable=False,
)
new_version = components_api.get_component(self.problem.pk) \
.versions \
.get(publishable_entity_version__version_num=1)
assert (
new_content ==
new_version.contents.get(componentversioncontent__key="nested/path/hello.txt")
)


def test_multiple_versions(self):
hello_content = contents_api.get_or_create_text_content(
self.learning_package.id,
Expand Down

0 comments on commit 397878b

Please sign in to comment.