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 /.

Changes the order in which pulpcore-content looks for published artifacts
The handler now looks for a published artifact before it tries to look for an index.html or listing
the directory.

fixes: pulp#3173
fixes: pulp#3459
  • Loading branch information
dkliban authored and mdellweg committed May 4, 2023
1 parent 14cedb9 commit 23bc5e0
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 33 deletions.
1 change: 1 addition & 0 deletions CHANGES/3173.feature
@@ -0,0 +1 @@
Content app now redirects requests without a trailing slash to a path with the trailing slash.
2 changes: 2 additions & 0 deletions CHANGES/plugin_api/3459.feature
@@ -0,0 +1,2 @@
Added a flag `REDIRECT_ON_MISSING_SLASH` for plugin distrubution models to opt out of automatic
redirecting if a path should end in a slash but does not.
23 changes: 18 additions & 5 deletions pulpcore/app/models/base.py
Expand Up @@ -126,6 +126,24 @@ class MasterModel(BaseModel, metaclass=MasterModelMeta):
class Meta:
abstract = True

@classmethod
def get_pulp_type(cls):
"""Get the "pulp_type" string associated with this MasterModel type."""
return "{app_label}.{type}".format(app_label=cls._meta.app_label, type=cls.TYPE)

@classmethod
def get_model_for_pulp_type(cls, pulp_type):
return cls._pulp_model_map[pulp_type]

def __init_subclass__(cls, /, **kwargs):
super().__init_subclass__(**kwargs)
if hasattr(cls, "_pulp_model_map"):
# This is a Detail Model. Register it with the Master Model.
cls._pulp_model_map[cls.get_pulp_type()] = cls
else:
# This is a Master Model. Initialize the model map.
cls._pulp_model_map = {}

def save(self, *args, **kwargs):
# instances of "detail" models that subclass MasterModel are exposed
# on instances of MasterModel by the string stored in that model's TYPE attr.
Expand All @@ -137,11 +155,6 @@ def save(self, *args, **kwargs):
self.pulp_type = self.get_pulp_type()
return super().save(*args, **kwargs)

@classmethod
def get_pulp_type(cls):
"""Get the "pulp_type" string associated with this MasterModel type."""
return "{app_label}.{type}".format(app_label=cls._meta.app_label, type=cls.TYPE)

def cast(self):
"""Return a "Detail" model instance of this master-detail pair.
Expand Down
5 changes: 4 additions & 1 deletion pulpcore/app/models/publication.py
Expand Up @@ -524,9 +524,12 @@ class Distribution(MasterModel):
pulp_domain (models.ForeignKey): The domain the Distribution is a part of.
"""

# If distribution serves publications, set by subclasses for proper handling in content app
# If distribution serves publications, set by subclasses for proper handling in content app.
SERVE_FROM_PUBLICATION = False

# Plugins can opt out of redirecting by settings this to False.
REDIRECT_ON_MISSING_SLASH = True

name = models.TextField(db_index=True)
pulp_labels = HStoreField(default=dict)
base_path = models.TextField()
Expand Down
76 changes: 49 additions & 27 deletions pulpcore/content/handler.py
Expand Up @@ -9,6 +9,7 @@
from aiohttp.web_exceptions import (
HTTPForbidden,
HTTPFound,
HTTPMovedPermanently,
HTTPNotFound,
HTTPRequestRangeNotSatisfiable,
)
Expand Down Expand Up @@ -280,6 +281,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:
path = f"{path}/"
base_paths = cls._base_paths(path)
distro_model = cls.distribution_model or Distribution
domain = get_domain()
Expand All @@ -298,8 +302,11 @@ 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 /
raise HTTPMovedPermanently(f"{settings.CONTENT_PATH_PREFIX}{path}")
log.debug(
_("Distribution not matched for {path} using: {base_paths}").format(
path=path, base_paths=base_paths
Expand Down Expand Up @@ -478,9 +485,7 @@ def list_directory_blocking():
).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 @@ -525,6 +530,12 @@ 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 not distro.REDIRECT_ON_MISSING_SLASH:
raise ObjectDoesNotExist(request.path)
raise HTTPMovedPermanently(f"{request.path}/")

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 @@ -561,28 +572,6 @@ def get_latest_publication_or_version_blocking():
await sync_to_async(get_latest_publication_or_version_blocking)()

if publication:
if rel_path == "" or rel_path[-1] == "/":
try:
index_path = "{}index.html".format(rel_path)

await sync_to_async(publication.published_artifact.get)(
relative_path=index_path
)

rel_path = index_path
headers = self.response_headers(rel_path)
except ObjectDoesNotExist:
dir_list, dates, sizes = await self.list_directory(None, publication, rel_path)
dir_list.update(
await sync_to_async(distro.content_handler_list_directory)(rel_path)
)
return HTTPOk(
headers={"Content-Type": "text/html"},
body=self.render_html(
dir_list, path=request.path, dates=dates, sizes=sizes
),
)

# published artifact
try:

Expand Down Expand Up @@ -638,6 +627,39 @@ def get_contentartifact_blocking():
request, StreamResponse(headers=headers), ca
)

# Look for index.html or list the directory
ends_in_slash = rel_path == "" or rel_path.endswith("/")
if not ends_in_slash:
rel_path = f"{rel_path}/"
try:
index_path = "{}index.html".format(rel_path)

await sync_to_async(publication.published_artifact.get)(relative_path=index_path)
if ends_in_slash is False:
# index.html found, but user didn't specify a trailing slash
if not distro.REDIRECT_ON_MISSING_SLASH:
raise ObjectDoesNotExist(request.path)
raise HTTPMovedPermanently(f"{request.path}/")
rel_path = index_path
headers = self.response_headers(rel_path)
except ObjectDoesNotExist:
if not distro.REDIRECT_ON_MISSING_SLASH:
raise
dir_list, dates, sizes = await self.list_directory(None, publication, rel_path)
dir_list.update(
await sync_to_async(distro.content_handler_list_directory)(rel_path)
)
if dir_list and not ends_in_slash:
# Directory can be listed, but user did not specify trailing slash
raise HTTPMovedPermanently(f"{request.path}/")
elif dir_list:
return HTTPOk(
headers={"Content-Type": "text/html"},
body=self.render_html(
dir_list, path=request.path, dates=dates, sizes=sizes
),
)

if repo_version and not publication and not distro.SERVE_FROM_PUBLICATION:
if rel_path == "" or rel_path[-1] == "/":
index_path = "{}index.html".format(rel_path)
Expand Down

0 comments on commit 23bc5e0

Please sign in to comment.