Skip to content

Commit

Permalink
Redirects user to a path that ends in /
Browse files Browse the repository at this point in the history
This patch returns a 301 redirect when a requested path does not end in a / but should end in a /.

closes: pulp#3173
closes: pulp#3459
  • Loading branch information
dkliban committed Sep 22, 2023
1 parent 2032e64 commit 7767d2e
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 20 deletions.
1 change: 1 addition & 0 deletions CHANGES/3173.feature
@@ -0,0 +1 @@
The content app now returns a 301 redirect when a requested path does not end in a / but should end in a /.
1 change: 1 addition & 0 deletions CHANGES/plugin_api/3459.feature
@@ -0,0 +1 @@
Handler._match_distribution() method now accepts `add_trailing_slash` keyword argument. When set to False, the content app will not try to append a '/' to the path before trying to match it to a distribution. Plugin code that calls this method directly needs to be updated to account for the desired behavior.
3 changes: 3 additions & 0 deletions pulpcore/app/models/publication.py
Expand Up @@ -533,6 +533,9 @@ class Distribution(MasterModel):
# If distribution serves publications, set by subclasses for proper handling in content app
SERVE_FROM_PUBLICATION = False

# If distribution supports redirecting clients when a trailing / is missing
REDIRECT_ON_MISSING_SLASH = True

name = models.TextField(db_index=True)
pulp_labels = HStoreField(default=dict)
base_path = models.TextField()
Expand Down
1 change: 1 addition & 0 deletions pulpcore/content/__init__.py
Expand Up @@ -94,6 +94,7 @@ async def server(*args, **kwargs):
path_prefix = settings.CONTENT_PATH_PREFIX
if settings.DOMAIN_ENABLED:
path_prefix = path_prefix + "{pulp_domain}/"
app.add_routes([web.get(path_prefix[:-1], Handler().list_distributions)])
app.add_routes([web.get(path_prefix, Handler().list_distributions)])
app.add_routes([web.get(path_prefix + "{path:.+}", Handler().stream_content)])
app.cleanup_ctx.append(_heartbeat_ctx)
Expand Down
60 changes: 40 additions & 20 deletions pulpcore/content/handler.py
Expand Up @@ -11,6 +11,7 @@
HTTPError,
HTTPForbidden,
HTTPFound,
HTTPMovedPermanently,
HTTPNotFound,
HTTPRequestRangeNotSatisfiable,
)
Expand Down Expand Up @@ -173,6 +174,9 @@ async def list_distributions(self, request):
"""
domain = get_domain()

if not request.path.endswith("/"):
raise HTTPMovedPermanently(f"{request.path}/")

def get_base_paths_blocking():
distro_model = self.distribution_model or Distribution
raise DistroListings(path="", distros=distro_model.objects.filter(pulp_domain=domain))
Expand Down Expand Up @@ -270,12 +274,13 @@ def _base_paths(path):
return tree

@classmethod
def _match_distribution(cls, path):
def _match_distribution(cls, path, add_trailing_slash=True):
"""
Match a distribution using a list of base paths and return its detail object.
Args:
path (str): The path component of the URL.
add_trailing_slash (bool): If true, a missing trailing '/' will be appended to the path.
Returns:
The detail object of the matched distribution.
Expand All @@ -284,6 +289,9 @@ def _match_distribution(cls, path):
DistroListings: when multiple matches are possible.
PathNotResolved: when not matched.
"""
path_ends_in_slash = path.endswith("/")
if not path_ends_in_slash and add_trailing_slash:
path = f"{path}/"
base_paths = cls._base_paths(path)
distro_model = cls.distribution_model or Distribution
domain = get_domain()
Expand All @@ -308,7 +316,16 @@ def _match_distribution(cls, path):
pulp_domain=domain, base_path__startswith=path
)
if distros.count():
raise DistroListings(path=path, distros=distros)
if path_ends_in_slash:
raise DistroListings(path=path, distros=distros)
else:
# The list of a subset of distributions was requested without a trailing /
if settings.DOMAIN_ENABLED:
raise HTTPMovedPermanently(
f"{settings.CONTENT_PATH_PREFIX}{domain.name}/{path}"
)
else:
raise HTTPMovedPermanently(f"{settings.CONTENT_PATH_PREFIX}{path}")

log.debug(
_("Distribution not matched for {path} using: {base_paths}").format(
Expand Down Expand Up @@ -482,24 +499,21 @@ def list_directory_blocking():
else:
artifacts_to_find[ca.pk] = name

if directory_list:
# Find the dates the content got added to the repository
dates.update(
{
content_to_find[rc.content_id]: rc.pulp_created
for rc in content_repo_ver._content_relationships()
if rc.content_id in content_to_find
}
)
# Find the sizes for on_demand artifacts
r_artifacts = RemoteArtifact.objects.filter(
content_artifact__in=artifacts_to_find.keys()
).values_list("content_artifact_id", "size")
sizes.update({artifacts_to_find[ra_ca_id]: size for ra_ca_id, size in r_artifacts})
# Find the dates the content got added to the repository
dates.update(
{
content_to_find[rc.content_id]: rc.pulp_created
for rc in content_repo_ver._content_relationships()
if rc.content_id in content_to_find
}
)
# Find the sizes for on_demand artifacts
r_artifacts = RemoteArtifact.objects.filter(
content_artifact__in=artifacts_to_find.keys()
).values_list("content_artifact_id", "size")
sizes.update({artifacts_to_find[ra_ca_id]: size for ra_ca_id, size in r_artifacts})

return directory_list, dates, sizes
else:
raise PathNotResolved(path)
return directory_list, dates, sizes

return await sync_to_async(list_directory_blocking)()

Expand Down Expand Up @@ -544,6 +558,13 @@ async def _match_and_stream(self, path, request):
rel_path = rel_path[len(distro.base_path) :]
rel_path = rel_path.lstrip("/")

if rel_path == "" and not path.endswith("/"):
# The root of a distribution base_path was requested without a slash
if distro.REDIRECT_ON_MISSING_SLASH:
raise HTTPMovedPermanently(f"{request.path}/")
else:
raise HTTPNotFound()

content_handler_result = await sync_to_async(distro.content_handler)(rel_path)
if content_handler_result is not None:
return content_handler_result
Expand Down Expand Up @@ -592,7 +613,6 @@ async def _match_and_stream(self, path, request):
dir_list, path=request.path, dates=dates, sizes=sizes
),
)

# published artifact
try:
ca = (
Expand Down

0 comments on commit 7767d2e

Please sign in to comment.