From b3670369b8620ae645a08bb27f553ec6bf830573 Mon Sep 17 00:00:00 2001 From: Adrian Date: Mon, 13 Jul 2026 19:46:41 +0200 Subject: [PATCH] Solve the feed Path issue (#7674) * Solve the feed Path issue * Address additional, related issues --- docs/topics/feed-exports.rst | 5 ++ scrapy/extensions/feedexport.py | 39 +++++++++-- tests/test_feedexport.py | 117 ++++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+), 6 deletions(-) diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index db6dd127c..ef1abe747 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -143,6 +143,11 @@ Here are some examples to illustrate: .. note:: :ref:`Spider arguments ` 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: diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index fa708a2e1..0d9b016a3 100644 --- a/scrapy/extensions/feedexport.py +++ b/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) - if not isinstance(settings_uri, Path) - else settings_uri.absolute().as_uri() + str(settings_uri.absolute()) + if isinstance(settings_uri, Path) + 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, diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 7d751f188..5ab1a54c1 100644 --- a/tests/test_feedexport.py +++ b/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