From 9332a2ec6296860354eb872a3151dd0dea0cf0a4 Mon Sep 17 00:00:00 2001 From: Tim Jenness Date: Mon, 17 Aug 2026 09:49:39 -0700 Subject: [PATCH 1/2] Percent-encode the remote-test base_path base_path is interpolated straight into a URI string, so a raw OS path leaves characters such as "+" unencoded. join() quotes only the component being joined and so preserves the unencoded base, while walk() rebuilds the URI from the OS path and encodes all of it, making the two disagree. Root the tests below a directory containing a "+" so this is always exercised. EUPS build directories embed the package version in their name, which is where this first showed up. Co-Authored-By: Claude Opus 5 (1M context) --- tests/test_remote_test.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/test_remote_test.py b/tests/test_remote_test.py index 581d9c0e..44957346 100644 --- a/tests/test_remote_test.py +++ b/tests/test_remote_test.py @@ -41,7 +41,14 @@ def setUpClass(cls) -> None: # This scheme is backed by the local file system, so the URI path has # to name a real writable directory. The netloc is not used. cls._tmproot = makeTestTempDir(TESTDIR) - cls.base_path = cls._tmproot + # Root the tests below a "+" so that every test exercises a path that + # has to be percent-encoded. An EUPS build directory embeds the + # package version in its name, so this really happens. + root = os.path.join(cls._tmproot, "resources-g1234567890+abcdef1234") + os.makedirs(root) + # base_path must be percent-encoded, so derive it from a URI rather + # than using the OS path directly. + cls.base_path = make_remote_test_uri(root).path.rstrip("/") super().setUpClass() @classmethod From 16f84e76224e5b493166eb22bf99e29b65ce6a2b Mon Sep 17 00:00:00 2001 From: Tim Jenness Date: Mon, 17 Aug 2026 09:49:46 -0700 Subject: [PATCH 2/2] Document that _make_uri needs pre-quoted path components _make_uri can not percent-encode the components itself: it does not know which ResourcePath class will be built from the string it returns, so it can not consult quotePaths, and encoding an already-encoded path would corrupt it. Record that requirement on the method and on base_path. Co-Authored-By: Claude Opus 5 (1M context) --- python/lsst/resources/tests.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/python/lsst/resources/tests.py b/python/lsst/resources/tests.py index c489f42b..449c1c79 100644 --- a/python/lsst/resources/tests.py +++ b/python/lsst/resources/tests.py @@ -231,10 +231,43 @@ class _GenericTestCase(TestCaseMixin): scheme: str | None = None netloc: str | None = None base_path: str | None = None + """Path prepended to every URI built by `_make_uri`. Must already be + percent-encoded.""" path1 = "test_dir" path2 = "file.txt" def _make_uri(self, path: str, netloc: str | None = None) -> str: + """Build a URI string for the scheme under test. + + Parameters + ---------- + path : `str` + Path component to place below `base_path`. Must already be + percent-encoded. + netloc : `str` or `None`, optional + Network location to use instead of `netloc`. + + Returns + ------- + uri : `str` + The assembled URI, or ``path`` unchanged if no scheme is defined. + + Notes + ----- + The components are interpolated into the URI verbatim, so both ``path`` + and `base_path` must arrive percent-encoded for any scheme whose + `~lsst.resources.ResourcePath` class sets ``quotePaths``. This method + can not do the encoding itself because it does not know which class + will be created from the string it returns, and encoding a path that + was already encoded would corrupt it. + + Getting this wrong is not always visible: `ResourcePath.join` quotes + only the component being joined and leaves the rest of the URI alone, + so an unencoded ``base_path`` survives into every derived URI, while + `~lsst.resources.file.FileResourcePath.walk` rebuilds the URI from the + OS path and encodes all of it. The two then disagree for any path + holding a character such as ``+`` or a space. + """ if self.scheme is not None: if netloc is None: netloc = self.netloc