From d72e456918db99ee8ca78e92bb2c7b9ea273270f Mon Sep 17 00:00:00 2001 From: Mauro Amico Date: Mon, 28 Oct 2024 01:28:15 +0100 Subject: [PATCH 1/7] feat: set canonical header for file --- plone/namedfile/browser.py | 13 +++++++++++-- plone/namedfile/tests/test_display_file.py | 15 +++++++++++++++ plone/namedfile/utils/__init__.py | 4 +++- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/plone/namedfile/browser.py b/plone/namedfile/browser.py index 0930b752..0b753cb3 100644 --- a/plone/namedfile/browser.py +++ b/plone/namedfile/browser.py @@ -127,6 +127,13 @@ def handle_request_range(self, file): except ValueError: return default + def get_canonical(self, file): + filename = getattr(file, "filename", None) + if filename is None: + return f"{self.context.absolute_url()}/@@download/{self.fieldname}" + else: + return f"{self.context.absolute_url()}/@@download/{self.fieldname}/filename" + def set_headers(self, file): # With filename None, set_headers will not add the download headers. if not self.filename: @@ -135,7 +142,8 @@ def set_headers(self, file): self.filename = self.fieldname if self.filename is None: self.filename = "file.ext" - set_headers(file, self.request.response, filename=self.filename) + canonical = self.get_canonical(file) + set_headers(file, self.request.response, filename=self.filename, canonical=canonical) def _getFile(self): if not self.fieldname: @@ -185,4 +193,5 @@ def set_headers(self, file): if mimetype not in self.allowed_inline_mimetypes: # Let the Download view handle this. return super().set_headers(file) - set_headers(file, self.request.response) + canonical = self.get_canonical(file) + set_headers(file, self.request.response, canonical=canonical) diff --git a/plone/namedfile/tests/test_display_file.py b/plone/namedfile/tests/test_display_file.py index 4ab467d3..88e41518 100644 --- a/plone/namedfile/tests/test_display_file.py +++ b/plone/namedfile/tests/test_display_file.py @@ -49,6 +49,15 @@ def get_disposition_header(browser): return browser.headers.get(name, None) +def get_canonical_header(browser): + name = "Link" + for header, value in browser.headers.items(): + if header == name or header == name.lower(): + if 'rel="canonical"' in map(str.strip, value.split(";")): + return value + return None + + def custom_available_sizes(): # Define available image scales. return {"custom": (10, 10)} @@ -96,12 +105,16 @@ def assert_download_works(self, base_url): self.assertIsNotNone(header) self.assertIn("attachment", header) self.assertIn("filename", header) + header = get_canonical_header(browser) + self.assertTrue(header.startswith(f"<{base_url}/@@download/{self.field_name}>")) def assert_display_inline_works(self, base_url): # Test that displaying this file inline works. browser = self.get_anon_browser() browser.open(base_url + f"/@@display-file/{self.field_name}") self.assertIsNone(get_disposition_header(browser)) + header = get_canonical_header(browser) + self.assertTrue(header.startswith(f"<{base_url}/@@download/{self.field_name}>")) def assert_display_inline_is_download(self, base_url): # Test that displaying this file inline turns into a download. @@ -111,6 +124,8 @@ def assert_display_inline_is_download(self, base_url): self.assertIsNotNone(header) self.assertIn("attachment", header) self.assertIn("filename", header) + header = get_canonical_header(browser) + self.assertTrue(header.startswith(f"<{base_url}/@@download/{self.field_name}>")) def assert_scale_view_works(self, base_url): # Test that accessing a scale view shows the image inline. diff --git a/plone/namedfile/utils/__init__.py b/plone/namedfile/utils/__init__.py index 2f17ead9..6509aec7 100644 --- a/plone/namedfile/utils/__init__.py +++ b/plone/namedfile/utils/__init__.py @@ -130,7 +130,7 @@ def get_contenttype(file=None, filename=None, default="application/octet-stream" return default -def set_headers(file, response, filename=None): +def set_headers(file, response, filename=None, canonical=None): """Set response headers for the given file. If filename is given, set the Content-Disposition to attachment. """ @@ -149,6 +149,8 @@ def set_headers(file, response, filename=None): "Content-Disposition", f"attachment; filename*=UTF-8''{filename}" ) + if canonical is not None: + response.setHeader("Link", f'<{canonical}>; rel="canonical"') def stream_data(file, start=0, end=None): """Return the given file as a stream if possible.""" From 5a9fc2f369e50c826ada3939294900325d30ea83 Mon Sep 17 00:00:00 2001 From: Mauro Amico Date: Mon, 28 Oct 2024 01:35:22 +0100 Subject: [PATCH 2/7] fix test --- plone/namedfile/usage.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plone/namedfile/usage.rst b/plone/namedfile/usage.rst index 759afe38..bec73255 100644 --- a/plone/namedfile/usage.rst +++ b/plone/namedfile/usage.rst @@ -32,6 +32,9 @@ These store data with the following types:: ... self.image = namedfile.NamedImage() ... self.blob = namedfile.NamedBlobFile() ... self.blobimage = namedfile.NamedBlobImage() + ... + ... def absolute_url(self): + ... return "http://foo/bar" File data and content type From 36fe1a40779787bbd025b4ce82b792473a7014f9 Mon Sep 17 00:00:00 2001 From: Mauro Amico Date: Mon, 28 Oct 2024 01:48:30 +0100 Subject: [PATCH 3/7] fix test --- plone/namedfile/browser.py | 2 +- plone/namedfile/tests/test_display_file.py | 15 --------------- plone/namedfile/usage.rst | 8 ++++++++ 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/plone/namedfile/browser.py b/plone/namedfile/browser.py index 0b753cb3..538b5d86 100644 --- a/plone/namedfile/browser.py +++ b/plone/namedfile/browser.py @@ -132,7 +132,7 @@ def get_canonical(self, file): if filename is None: return f"{self.context.absolute_url()}/@@download/{self.fieldname}" else: - return f"{self.context.absolute_url()}/@@download/{self.fieldname}/filename" + return f"{self.context.absolute_url()}/@@download/{self.fieldname}/{filename}" def set_headers(self, file): # With filename None, set_headers will not add the download headers. diff --git a/plone/namedfile/tests/test_display_file.py b/plone/namedfile/tests/test_display_file.py index 88e41518..4ab467d3 100644 --- a/plone/namedfile/tests/test_display_file.py +++ b/plone/namedfile/tests/test_display_file.py @@ -49,15 +49,6 @@ def get_disposition_header(browser): return browser.headers.get(name, None) -def get_canonical_header(browser): - name = "Link" - for header, value in browser.headers.items(): - if header == name or header == name.lower(): - if 'rel="canonical"' in map(str.strip, value.split(";")): - return value - return None - - def custom_available_sizes(): # Define available image scales. return {"custom": (10, 10)} @@ -105,16 +96,12 @@ def assert_download_works(self, base_url): self.assertIsNotNone(header) self.assertIn("attachment", header) self.assertIn("filename", header) - header = get_canonical_header(browser) - self.assertTrue(header.startswith(f"<{base_url}/@@download/{self.field_name}>")) def assert_display_inline_works(self, base_url): # Test that displaying this file inline works. browser = self.get_anon_browser() browser.open(base_url + f"/@@display-file/{self.field_name}") self.assertIsNone(get_disposition_header(browser)) - header = get_canonical_header(browser) - self.assertTrue(header.startswith(f"<{base_url}/@@download/{self.field_name}>")) def assert_display_inline_is_download(self, base_url): # Test that displaying this file inline turns into a download. @@ -124,8 +111,6 @@ def assert_display_inline_is_download(self, base_url): self.assertIsNotNone(header) self.assertIn("attachment", header) self.assertIn("filename", header) - header = get_canonical_header(browser) - self.assertTrue(header.startswith(f"<{base_url}/@@download/{self.field_name}>")) def assert_scale_view_works(self, base_url): # Test that accessing a scale view shows the image inline. diff --git a/plone/namedfile/usage.rst b/plone/namedfile/usage.rst index bec73255..c18cf075 100644 --- a/plone/namedfile/usage.rst +++ b/plone/namedfile/usage.rst @@ -229,6 +229,8 @@ We will test this with a dummy request, faking traversal:: 'text/plain' >>> request.response.getHeader('Content-Disposition') "attachment; filename*=UTF-8''test.txt" + >>> request.response.getHeader('Link') + '; rel="canonical"' >>> request = TestRequest() >>> download = Download(container, request).publishTraverse(request, 'blob') @@ -241,6 +243,8 @@ We will test this with a dummy request, faking traversal:: 'text/plain' >>> request.response.getHeader('Content-Disposition') "attachment; filename*=UTF-8''test.txt" + >>> request.response.getHeader('Link') + '; rel="canonical"' >>> request = TestRequest() >>> download = Download(container, request).publishTraverse(request, 'image') @@ -253,6 +257,8 @@ We will test this with a dummy request, faking traversal:: 'image/foo' >>> request.response.getHeader('Content-Disposition') "attachment; filename*=UTF-8''zpt.gif" + >>> request.response.getHeader('Link') + '; rel="canonical"' >>> request = TestRequest() >>> download = Download(container, request).publishTraverse(request, 'blobimage') @@ -265,6 +271,8 @@ We will test this with a dummy request, faking traversal:: 'image/foo' >>> request.response.getHeader('Content-Disposition') "attachment; filename*=UTF-8''zpt.gif" + >>> request.response.getHeader('Link') + '; rel="canonical"' Range support ------------- From 5a808ba73b98be70ce0786f5f4fe87937248761c Mon Sep 17 00:00:00 2001 From: Mauro Amico Date: Mon, 28 Oct 2024 14:13:31 +0100 Subject: [PATCH 4/7] changelog --- news/163.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/163.feature diff --git a/news/163.feature b/news/163.feature new file mode 100644 index 00000000..08cf3f9e --- /dev/null +++ b/news/163.feature @@ -0,0 +1 @@ +set canonical header for file. @mamico From 6296627099f9524d545d4e2de8d195a6bddea6d6 Mon Sep 17 00:00:00 2001 From: David Glick Date: Mon, 28 Oct 2024 21:59:49 -0700 Subject: [PATCH 5/7] Update news/163.feature --- news/163.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/163.feature b/news/163.feature index 08cf3f9e..3891be30 100644 --- a/news/163.feature +++ b/news/163.feature @@ -1 +1 @@ -set canonical header for file. @mamico +Set `Link` header with `rel="canonical"` for file downloads. @mamico From 39abbc0d7af268fe443603886ce8bf24335b2963 Mon Sep 17 00:00:00 2001 From: Mauro Amico Date: Sat, 16 Nov 2024 23:39:13 +0100 Subject: [PATCH 6/7] The URI (absolute or relative) must percent-encode character codes greater than 255 (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Link) --- plone/namedfile/utils/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plone/namedfile/utils/__init__.py b/plone/namedfile/utils/__init__.py index 6509aec7..dd9da38e 100644 --- a/plone/namedfile/utils/__init__.py +++ b/plone/namedfile/utils/__init__.py @@ -150,7 +150,7 @@ def set_headers(file, response, filename=None, canonical=None): ) if canonical is not None: - response.setHeader("Link", f'<{canonical}>; rel="canonical"') + response.setHeader("Link", f'<{quote(canonical, safe='')}>; rel="canonical"') def stream_data(file, start=0, end=None): """Return the given file as a stream if possible.""" From 2e54f717898c9b717a5100682e3cb0f43a91b68f Mon Sep 17 00:00:00 2001 From: Mauro Amico Date: Sat, 16 Nov 2024 23:50:00 +0100 Subject: [PATCH 7/7] fix quote --- plone/namedfile/utils/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plone/namedfile/utils/__init__.py b/plone/namedfile/utils/__init__.py index dd9da38e..a2ccf851 100644 --- a/plone/namedfile/utils/__init__.py +++ b/plone/namedfile/utils/__init__.py @@ -150,7 +150,7 @@ def set_headers(file, response, filename=None, canonical=None): ) if canonical is not None: - response.setHeader("Link", f'<{quote(canonical, safe='')}>; rel="canonical"') + response.setHeader("Link", f'<{quote(canonical, safe="/:&?=@")}>; rel="canonical"') def stream_data(file, start=0, end=None): """Return the given file as a stream if possible."""