Fix file extension extraction for URLs with query parameters (#4225) (#7414)

* Fix file extension extraction for URLs with query parameters (#4225)

* Prioritize parsed path for file extensions and add tests (#4225)

scrapy/pipelines/files.py

@@ -33,6 +33,7 @@ from scrapy.utils.boto import is_botocore_available
 from scrapy.utils.datatypes import CaseInsensitiveDict
 from scrapy.utils.defer import deferred_from_coro, ensure_awaitable
 from scrapy.utils.ftp import ftp_store_file
+from scrapy.utils.httpobj import urlparse_cached
 from scrapy.utils.log import failure_to_exc_info
 from scrapy.utils.python import to_bytes
 from scrapy.utils.request import referer_str
@@ -724,7 +725,15 @@ class FilesPipeline(MediaPipeline):
         item: Any = None,
     ) -> str:
         media_guid = hashlib.sha1(to_bytes(request.url)).hexdigest()  # noqa: S324
-        media_ext = Path(request.url).suffix
+
+        # clean it up and look at the path first
+        parsed_url = urlparse_cached(request)
+        media_ext = Path(parsed_url.path).suffix
+
+        # if path has no extension look at the raw  URL
+        if media_ext not in mimetypes.types_map:
+            media_ext = Path(request.url).suffix
+
         # Handles empty and wild extensions by trying to guess the
         # mime type then extension or default to empty string otherwise
         if media_ext not in mimetypes.types_map:

tests/test_pipeline_files.py

@@ -109,6 +109,15 @@ class TestFilesPipeline:
     def teardown_method(self):
         rmtree(self.tempdir)
 
+    def test_file_path_query_parameters(self):
+        file_path = self.pipeline.file_path
+
+        req1 = Request("http://foo.bar/baz.txt?fizz")
+        assert file_path(req1) == "full/a2b4913a62f65445aeae2bac08cd8c3b41d7195e.txt"
+
+        req2 = Request("http://foo.bar/get_img.php?file=photo.jpg")
+        assert file_path(req2) == "full/118230fd648f1080c81c234d5e2463ea496f8c05.jpg"
+
     def test_file_path(self):
         file_path = self.pipeline.file_path
         assert (