Solve the feed Path issue (#7674)

* Solve the feed Path issue

* Address additional, related issues

docs/topics/feed-exports.rst

@@ -143,6 +143,11 @@ Here are some examples to illustrate:
 .. note:: :ref:`Spider arguments <spiderargs>` become spider attributes, hence
           they can also be used as storage URI parameters.
 
+.. note:: Only ``%(...)s`` parameters are replaced. Any other percent
+          character is kept as-is, so percent-encoded URIs (e.g. ``%20`` for a
+          space or percent-encoded FTP credentials) and :class:`pathlib.Path`
+          keys containing ``%(...)s`` parameters both work as expected.
+
 
 .. _topics-feed-storage-backends:
 

scrapy/extensions/feedexport.py

@@ -47,6 +47,33 @@ if TYPE_CHECKING:
 
 logger = logging.getLogger(__name__)
 
+
+# Printf-style placeholders (e.g. %(time)s) used to build feed URIs. Any other
+# percent character in a URI (e.g. percent-encoding such as %20 or %23) must be
+# treated as a literal rather than as the start of a placeholder.
+_FEED_URI_PLACEHOLDER_RE = re.compile(
+    r"%\([^)]+\)[-+ #0]*(?:\d+|\*)?(?:\.(?:\d+|\*))?[diouxXeEfFgGcrsa]"
+)
+
+
+def apply_uri_params(uri_template: str, uri_params: dict[str, Any]) -> str:
+    """Return *uri_template* with its ``%(...)s`` placeholders replaced using
+    *uri_params*, leaving any other percent character untouched.
+
+    This allows feed URIs to contain percent-encoded characters (e.g. ``%20``
+    in a path with spaces or ``%23`` in FTP credentials) without them being
+    misinterpreted as printf-style formatting directives.
+    """
+    parts: list[str] = []
+    last = 0
+    for match in _FEED_URI_PLACEHOLDER_RE.finditer(uri_template):
+        parts.append(uri_template[last : match.start()].replace("%", "%%"))
+        parts.append(match.group(0))
+        last = match.end()
+    parts.append(uri_template[last:].replace("%", "%%"))
+    return "".join(parts) % uri_params
+
+
 UriParamsCallableT: TypeAlias = Callable[
     [dict[str, Any], Spider], dict[str, Any] | None
 ]
@@ -473,7 +500,7 @@ class FeedExporter:
             )
             uri = self.settings["FEED_URI"]
             # handle pathlib.Path objects
-            uri = str(uri) if not isinstance(uri, Path) else uri.absolute().as_uri()
+            uri = str(uri.absolute()) if isinstance(uri, Path) else str(uri)
             feed_options = {"format": self.settings["FEED_FORMAT"]}
             self.feeds[uri] = feed_complete_default_values_from_settings(
                 feed_options, self.settings
@@ -485,9 +512,9 @@ class FeedExporter:
         for settings_uri, feed_options in self.settings.getdict("FEEDS").items():
             # handle pathlib.Path objects
             uri = (
-                str(settings_uri)
+                str(settings_uri.absolute())
-                if not isinstance(settings_uri, Path)
+                if isinstance(settings_uri, Path)
-                else settings_uri.absolute().as_uri()
+                else str(settings_uri)
             )
             self.feeds[uri] = feed_complete_default_values_from_settings(
                 feed_options, self.settings
@@ -514,7 +541,7 @@ class FeedExporter:
             self.slots.append(
                 self._start_new_batch(
                     batch_id=1,
-                    uri=uri % uri_params,
+                    uri=apply_uri_params(uri, uri_params),
                     feed_options=feed_options,
                     spider=spider,
                     uri_template=uri,
@@ -639,7 +666,7 @@ class FeedExporter:
                 slots.append(
                     self._start_new_batch(
                         batch_id=slot.batch_id + 1,
-                        uri=slot.uri_template % uri_params,
+                        uri=apply_uri_params(slot.uri_template, uri_params),
                         feed_options=self.feeds[slot.uri_template],
                         spider=spider,
                         uri_template=slot.uri_template,

tests/test_feedexport.py

@@ -32,6 +32,7 @@ from scrapy.extensions.feedexport import (
     FeedSlot,
     FileFeedStorage,
     IFeedStorage,
+    apply_uri_params,
 )
 from scrapy.utils.python import to_unicode
 from scrapy.utils.test import get_crawler
@@ -526,6 +527,91 @@ class TestFeedExport(TestFeedExportBase):
         header = self.MyItem.fields.keys()
         await self.assertExported(items, header, rows)
 
+    @coroutine_test
+    async def test_pathlib_uri_with_placeholders(self):
+        feed_dir = Path(self.temp_dir, "pathlib_placeholders")
+        feed_dir.mkdir()
+        items = [self.MyItem({"foo": "bar1", "egg": "spam1"})]
+
+        class TestSpider(scrapy.Spider):
+            name = "testspider"
+
+            def parse(self, response):
+                yield from items
+
+        TestSpider.start_urls = [self.mockserver.url("/")]
+        settings = {
+            "FEEDS": {
+                feed_dir / "%(time)s.json": {"format": "json"},
+            },
+        }
+        crawler = get_crawler(TestSpider, settings)
+        await crawler.crawl_async()
+
+        files = list(feed_dir.iterdir())
+        assert len(files) == 1
+        assert "%(time)s" not in files[0].name
+        assert files[0].suffix == ".json"
+
+    @coroutine_test
+    async def test_pathlib_uri_with_spaces_and_unicode(self):
+        # A pathlib.Path key with spaces and non-ASCII characters must be kept
+        # verbatim (not percent-encoded), while %()s placeholders are still
+        # substituted. %(name)s resolves to the spider name deterministically,
+        # so the resulting file name can be asserted exactly.
+        feed_dir = Path(self.temp_dir, "pathlib_spaces_unicode")
+        feed_dir.mkdir()
+        items = [self.MyItem({"foo": "bar1", "egg": "spam1"})]
+
+        class TestSpider(scrapy.Spider):
+            name = "testspider"
+
+            def parse(self, response):
+                yield from items
+
+        TestSpider.start_urls = [self.mockserver.url("/")]
+        settings = {
+            "FEEDS": {
+                feed_dir / "out %(name)s ünïcode.json": {"format": "json"},
+            },
+        }
+        crawler = get_crawler(TestSpider, settings)
+        await crawler.crawl_async()
+
+        files = list(feed_dir.iterdir())
+        assert len(files) == 1
+        assert files[0].name == "out testspider ünïcode.json"
+
+    @coroutine_test
+    async def test_str_uri_with_percent_encoding_and_placeholder(self):
+        # A percent-encoded string URI (e.g. %20 for a space) must reach
+        # storage verbatim rather than being misinterpreted as a printf
+        # directive, while %()s placeholders are still substituted. See #6425
+        # and #5794.
+        feed_dir = Path(self.temp_dir, "dir with spaces")
+        feed_dir.mkdir()
+        items = [self.MyItem({"foo": "bar1", "egg": "spam1"})]
+
+        class TestSpider(scrapy.Spider):
+            name = "testspider"
+
+            def parse(self, response):
+                yield from items
+
+        TestSpider.start_urls = [self.mockserver.url("/")]
+        settings = {
+            "FEEDS": {
+                f"{feed_dir.as_uri()}/%(time)s.json": {"format": "json"},
+            },
+        }
+        crawler = get_crawler(TestSpider, settings)
+        await crawler.crawl_async()
+
+        files = list(feed_dir.iterdir())
+        assert len(files) == 1
+        assert "%(time)s" not in files[0].name
+        assert files[0].suffix == ".json"
+
     @coroutine_test
     async def test_export_no_items_not_store_empty(self):
         for fmt in ("json", "jsonlines", "xml", "csv"):
@@ -1408,3 +1494,34 @@ class TestFeedExportInit:
         crawler = get_crawler(settings_dict=settings)
         exporter = FeedExporter.from_crawler(crawler)
         assert isinstance(exporter, FeedExporter)
+
+
+class TestApplyUriParams:
+    params = {
+        "name": "myspider",
+        "time": "2020-01-01T00-00-00",
+        "batch_id": 2,
+        "batch_time": "2020-01-01T00-00-00",
+    }
+
+    @pytest.mark.parametrize(
+        ("uri_template", "expected"),
+        [
+            # Placeholders are substituted, including width/flags.
+            ("/data/%(name)s/%(time)s.json", "/data/myspider/2020-01-01T00-00-00.json"),
+            ("/data/%(batch_id)05d.json", "/data/00002.json"),
+            # Percent-encoding is kept verbatim (#6425, #5794).
+            (
+                "file:///path%20with%20spaces/%(name)s.json",
+                "file:///path%20with%20spaces/myspider.json",
+            ),
+            (
+                "ftp://user:2%23um25%21M%23JZ@ftp.example.com/%(name)s.csv",
+                "ftp://user:2%23um25%21M%23JZ@ftp.example.com/myspider.csv",
+            ),
+            # A lone percent character next to a placeholder stays literal.
+            ("/100%/%(name)s.json", "/100%/myspider.json"),
+        ],
+    )
+    def test_apply_uri_params(self, uri_template, expected):
+        assert apply_uri_params(uri_template, self.params) == expected