From 8937b6de485d750e6d85fe33054c48939bddcf70 Mon Sep 17 00:00:00 2001 From: Jonathan Payne Date: Wed, 7 Oct 2026 10:29:20 -0400 Subject: [PATCH 1/5] OpenConceptLab/ocl_issues#2864 | export download saves under the API's name by default -o is now optional. Without it, or when it names a directory, the file is saved under the name the API gives the export: Content-Disposition first, then the signed URL's response-content-disposition, then the URL's last path segment. Only the base name is used. Co-Authored-By: Claude Opus 5.5 --- README.md | 9 ++- src/ocl_cli/commands/export.py | 43 ++++++++++++- tests/test_export_commands.py | 110 +++++++++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 6 deletions(-) create mode 100644 tests/test_export_commands.py diff --git a/README.md b/README.md index 52c2f5c..28cb850 100644 --- a/README.md +++ b/README.md @@ -191,8 +191,8 @@ ocl repo export status OWNER REPO VERSION --type source|collection # Trigger export creation (if not already cached) ocl repo export create OWNER REPO VERSION --type source|collection -# Download to a local file -ocl repo export download OWNER REPO VERSION --type source|collection -o FILENAME +# Download, saved under the API's name (-o FILE renames it, -o DIR saves it there) +ocl repo export download OWNER REPO VERSION --type source|collection [-o FILE|DIR] # Delete a cached export ocl repo export delete OWNER REPO VERSION --type source|collection @@ -202,9 +202,12 @@ Example — download the latest CIEL release export: ```bash ocl repo export status CIEL CIEL v2026-03-23 --type source -ocl repo export download CIEL CIEL v2026-03-23 --type source -o CIEL_v2026-03-23.zip +ocl repo export download CIEL CIEL v2026-03-23 --type source +# Saved to orgs_CIEL_sources_CIEL_v2026-03-23_.zip ``` +The API names every export `[owner-type]_[owner]_[repo-type]_[repo]_[version]_[expansion]_[lastUpdated].zip`; collection exports include the expansion. + ### Concepts ```bash diff --git a/src/ocl_cli/commands/export.py b/src/ocl_cli/commands/export.py index 925b274..d3aef0d 100644 --- a/src/ocl_cli/commands/export.py +++ b/src/ocl_cli/commands/export.py @@ -1,6 +1,9 @@ """Export commands: status, create, delete, download.""" +import os import sys +from email.message import Message +from urllib.parse import parse_qs, unquote, urlsplit import click @@ -80,17 +83,45 @@ def delete(ctx, owner, repo, version, repo_type, owner_type): handle_api_error(e) +def _filename_from_content_disposition(value): + """The filename in a Content-Disposition value, or None.""" + if not value: + return None + message = Message() + message["content-disposition"] = value + return message.get_filename() + + +def export_filename(response): + """The name the API gives the export. + + Read from the Content-Disposition header, then from the signed URL's + response-content-disposition, then from the URL's last path segment. + Only the base name is kept, so the file can't land outside the target directory. + """ + url = urlsplit(str(response.url)) + candidates = [ + _filename_from_content_disposition(response.headers.get("content-disposition")), + _filename_from_content_disposition(parse_qs(url.query).get("response-content-disposition", [None])[0]), + unquote(url.path.rsplit("/", 1)[-1]), + ] + for name in candidates: + name = os.path.basename((name or "").replace("\\", "/")) + if name not in ("", ".", ".."): + return name + return None + + @export.command() @export_args @click.option( "-o", "--output", "output_path", type=click.Path(), - help="Output file path (required).", - required=True, + help="Output file or directory (default: the API's name for the export, in the current directory).", ) @click.pass_context def download(ctx, owner, repo, version, repo_type, owner_type, output_path): - """Download an export file to a local path.""" + """Download an export file, named as the API names it unless -o gives a file.""" client = ctx.obj["client"] try: click.echo(f"Downloading export...", err=True) @@ -98,6 +129,12 @@ def download(ctx, owner, repo, version, repo_type, owner_type, output_path): owner, repo, version, owner_type=owner_type, repo_type=repo_type, ) + if not output_path or os.path.isdir(output_path): + filename = export_filename(response) + if not filename: + raise click.ClickException("The API didn't name the export. Pass -o FILE to choose a name.") + output_path = os.path.join(output_path or "", filename) + with open(output_path, "wb") as f: f.write(response.content) diff --git a/tests/test_export_commands.py b/tests/test_export_commands.py new file mode 100644 index 0000000..e81e164 --- /dev/null +++ b/tests/test_export_commands.py @@ -0,0 +1,110 @@ +import os +import unittest +from types import SimpleNamespace +from unittest.mock import patch + +import httpx +from click.testing import CliRunner + +from ocl_cli.main import cli + +API_NAME = "orgs_PIH_sources_PIH_1.8.24_2026-09-30_123456.zip" +SIGNED_URL = "https://exports.example.test/orgs/PIH/PIH_PIH_v1.8.24.2026-09-30_123456.zip?X-Amz-Signature=abc" + + +def export_response(url=SIGNED_URL, headers=None): + return httpx.Response(200, headers=headers, content=b"zip-bytes", request=httpx.Request("GET", url)) + + +def make_runner(): + # Click 8.1 mixes stderr into stdout unless told not to; 8.2 dropped mix_stderr. + try: + return CliRunner(mix_stderr=False) + except TypeError: + return CliRunner() + + +class FakeConfig: + def get_server(self, server_id): + return SimpleNamespace(base_url="https://api.example.test") + + def resolve_token(self, server, token_override=None): + return token_override + + +class FakeExportClient: + def __init__(self, response): + self.response = response + + def export_download(self, *args, **kwargs): + return self.response + + def close(self): + pass + + +class ExportDownloadCommandTest(unittest.TestCase): + def download(self, response, *args, exit_code=0): + runner = make_runner() + with ( + runner.isolated_filesystem(), + patch("ocl_cli.main.CLIConfig.load", return_value=FakeConfig()), + patch("ocl_cli.main.OCLAPIClient", return_value=FakeExportClient(response)), + ): + os.mkdir("downloads") + result = runner.invoke( + cli, ["repo", "export", "download", "PIH", "PIH", "1.8.24", "--type", "source", *args] + ) + self.assertEqual(result.exit_code, exit_code, result.output) + files = sorted( + os.path.join(root, name) for root, _, names in os.walk(".") for name in names + ) + return files, result + + def test_saves_under_the_content_disposition_name_by_default(self): + response = export_response(headers={"content-disposition": f'attachment; filename="{API_NAME}"'}) + + files, _ = self.download(response) + + self.assertEqual(files, [f"./{API_NAME}"]) + + def test_reads_the_name_from_the_signed_url_without_a_header(self): + url = f"{SIGNED_URL}&response-content-disposition=attachment%3B%20filename%3D%22{API_NAME}%22" + + files, _ = self.download(export_response(url=url)) + + self.assertEqual(files, [f"./{API_NAME}"]) + + def test_falls_back_to_the_storage_name(self): + files, _ = self.download(export_response()) + + self.assertEqual(files, ["./PIH_PIH_v1.8.24.2026-09-30_123456.zip"]) + + def test_output_directory_keeps_the_api_name(self): + response = export_response(headers={"content-disposition": f'attachment; filename="{API_NAME}"'}) + + files, _ = self.download(response, "-o", "downloads") + + self.assertEqual(files, [f"./downloads/{API_NAME}"]) + + def test_output_file_overrides_the_api_name(self): + response = export_response(headers={"content-disposition": f'attachment; filename="{API_NAME}"'}) + + files, _ = self.download(response, "-o", "mine.zip") + + self.assertEqual(files, ["./mine.zip"]) + + def test_name_cannot_leave_the_target_directory(self): + response = export_response(headers={"content-disposition": 'attachment; filename="../../evil.zip"'}) + + files, _ = self.download(response, "-o", "downloads") + + self.assertEqual(files, ["./downloads/evil.zip"]) + + def test_unnamed_export_asks_for_output(self): + response = export_response(url="https://api.example.test/orgs/PIH/sources/PIH/1.8.24/export/") + + files, result = self.download(response, exit_code=1) + + self.assertEqual(files, []) + self.assertIn("Pass -o FILE", result.stderr) From f69e0e6da053f849a80082279debd20a4176f07b Mon Sep 17 00:00:00 2001 From: Jonathan Payne Date: Wed, 7 Oct 2026 10:29:34 -0400 Subject: [PATCH 2/5] OpenConceptLab/ocl_issues#2864 | Drop export.py's unused import and placeholder-free f-string (ruff) Co-Authored-By: Claude Opus 5.5 --- src/ocl_cli/commands/export.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ocl_cli/commands/export.py b/src/ocl_cli/commands/export.py index d3aef0d..6e0be81 100644 --- a/src/ocl_cli/commands/export.py +++ b/src/ocl_cli/commands/export.py @@ -1,7 +1,6 @@ """Export commands: status, create, delete, download.""" import os -import sys from email.message import Message from urllib.parse import parse_qs, unquote, urlsplit @@ -124,7 +123,7 @@ def download(ctx, owner, repo, version, repo_type, owner_type, output_path): """Download an export file, named as the API names it unless -o gives a file.""" client = ctx.obj["client"] try: - click.echo(f"Downloading export...", err=True) + click.echo("Downloading export...", err=True) response = client.export_download( owner, repo, version, owner_type=owner_type, repo_type=repo_type, ) From 65f4ef571c881469b355012fd93694589a7ac6cf Mon Sep 17 00:00:00 2001 From: Jonathan Payne Date: Wed, 7 Oct 2026 10:38:52 -0400 Subject: [PATCH 3/5] OpenConceptLab/ocl_issues#2864 | export download: filename* first, strict names, no clobbering - filename* wins over filename (RFC 6266); each name source is tried in turn, and a malformed one falls through to the next. - A name must be made of the characters the API uses (letters, digits, . _ - @) and not be a Windows device name, which rules out paths, NUL bytes and alternate streams. - A name the API chose is created exclusively, so it never replaces an existing file or follows a symlink; -o FILE still overwrites. - -o DIR/ that doesn't exist is an error; README says DIR must exist. - Tests for each, plus the API's 302 handoff through OCLAPIClient. Addresses Codex pass 1 on ocl-cli#13. Co-Authored-By: Claude Opus 5.5 --- README.md | 3 +- src/ocl_cli/commands/export.py | 73 ++++++++++++++++++++++++---------- tests/test_export_commands.py | 63 ++++++++++++++++++++++++++++- 3 files changed, 115 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 28cb850..0156633 100644 --- a/README.md +++ b/README.md @@ -191,7 +191,8 @@ ocl repo export status OWNER REPO VERSION --type source|collection # Trigger export creation (if not already cached) ocl repo export create OWNER REPO VERSION --type source|collection -# Download, saved under the API's name (-o FILE renames it, -o DIR saves it there) +# Download, saved under the API's name; never replaces an existing file +# (-o FILE saves it as FILE, overwriting; -o DIR saves it in that existing directory) ocl repo export download OWNER REPO VERSION --type source|collection [-o FILE|DIR] # Delete a cached export diff --git a/src/ocl_cli/commands/export.py b/src/ocl_cli/commands/export.py index 6e0be81..c60630c 100644 --- a/src/ocl_cli/commands/export.py +++ b/src/ocl_cli/commands/export.py @@ -1,7 +1,7 @@ """Export commands: status, create, delete, download.""" import os -from email.message import Message +import re from urllib.parse import parse_qs, unquote, urlsplit import click @@ -82,31 +82,54 @@ def delete(ctx, owner, repo, version, repo_type, owner_type): handle_api_error(e) +# The API names exports with only these characters, so anything else isn't an export name. +SAFE_FILENAME = re.compile(r"[A-Za-z0-9][A-Za-z0-9._@-]*") +WINDOWS_DEVICE_NAMES = {"CON", "PRN", "AUX", "NUL", *(f"COM{i}" for i in range(1, 10)), + *(f"LPT{i}" for i in range(1, 10))} + + def _filename_from_content_disposition(value): - """The filename in a Content-Disposition value, or None.""" + """The filename in a Content-Disposition value, preferring filename* (RFC 6266), or None.""" if not value: return None - message = Message() - message["content-disposition"] = value - return message.get_filename() + extended = re.search(r"(?:^|;)\s*filename\*\s*=\s*([^';\s]+)'[^']*'([^;\s]+)", value, re.I) + if extended: + try: + return unquote(extended.group(2), encoding=extended.group(1), errors="strict") + except (LookupError, UnicodeDecodeError): + pass + plain = re.search(r'(?:^|;)\s*filename\s*=\s*(?:"((?:[^"\\]|\\.)*)"|([^;\s]+))', value, re.I) + if plain: + return re.sub(r"\\(.)", r"\1", plain.group(1)) if plain.group(1) is not None else plain.group(2) + return None + + +def _usable_filename(name): + if not name or not SAFE_FILENAME.fullmatch(name): + return False + return name.split(".")[0].upper() not in WINDOWS_DEVICE_NAMES def export_filename(response): - """The name the API gives the export. + """The name the API gives the export, or None. - Read from the Content-Disposition header, then from the signed URL's - response-content-disposition, then from the URL's last path segment. - Only the base name is kept, so the file can't land outside the target directory. + Tried in order: the Content-Disposition header, the signed URL's + response-content-disposition, then the URL's last path segment. A name + must be a plain file name made of the characters the API uses. """ url = urlsplit(str(response.url)) - candidates = [ - _filename_from_content_disposition(response.headers.get("content-disposition")), - _filename_from_content_disposition(parse_qs(url.query).get("response-content-disposition", [None])[0]), - unquote(url.path.rsplit("/", 1)[-1]), - ] - for name in candidates: - name = os.path.basename((name or "").replace("\\", "/")) - if name not in ("", ".", ".."): + candidates = ( + lambda: _filename_from_content_disposition(response.headers.get("content-disposition")), + lambda: _filename_from_content_disposition( + parse_qs(url.query).get("response-content-disposition", [None])[0]), + lambda: unquote(url.path.rsplit("/", 1)[-1]), + ) + for candidate in candidates: + try: + name = candidate() + except ValueError: + continue + if _usable_filename(name): return name return None @@ -116,7 +139,7 @@ def export_filename(response): @click.option( "-o", "--output", "output_path", type=click.Path(), - help="Output file or directory (default: the API's name for the export, in the current directory).", + help="Output file, or an existing directory (default: the API's name for the export, in the current directory).", ) @click.pass_context def download(ctx, owner, repo, version, repo_type, owner_type, output_path): @@ -128,14 +151,22 @@ def download(ctx, owner, repo, version, repo_type, owner_type, output_path): owner, repo, version, owner_type=owner_type, repo_type=repo_type, ) + # An explicit -o FILE is overwritten, as before. A name the API chose never replaces an existing file. + mode = "wb" if not output_path or os.path.isdir(output_path): filename = export_filename(response) if not filename: raise click.ClickException("The API didn't name the export. Pass -o FILE to choose a name.") output_path = os.path.join(output_path or "", filename) - - with open(output_path, "wb") as f: - f.write(response.content) + mode = "xb" + elif output_path.endswith(("/", os.sep)): + raise click.ClickException(f"Directory {output_path} doesn't exist.") + + try: + with open(output_path, mode) as f: + f.write(response.content) + except FileExistsError: + raise click.ClickException(f"{output_path} already exists. Remove it, or pass -o FILE to overwrite.") from None size = len(response.content) click.echo(f"Saved to {output_path} ({size:,} bytes)", err=True) diff --git a/tests/test_export_commands.py b/tests/test_export_commands.py index e81e164..45eaccc 100644 --- a/tests/test_export_commands.py +++ b/tests/test_export_commands.py @@ -94,12 +94,52 @@ def test_output_file_overrides_the_api_name(self): self.assertEqual(files, ["./mine.zip"]) - def test_name_cannot_leave_the_target_directory(self): + def test_a_path_in_the_name_is_rejected(self): response = export_response(headers={"content-disposition": 'attachment; filename="../../evil.zip"'}) files, _ = self.download(response, "-o", "downloads") - self.assertEqual(files, ["./downloads/evil.zip"]) + self.assertEqual(files, ["./downloads/PIH_PIH_v1.8.24.2026-09-30_123456.zip"]) + + def test_filename_star_wins_over_filename(self): + disposition = f"attachment; filename=\"fallback.zip\"; filename*=UTF-8''{API_NAME}" + + files, _ = self.download(export_response(headers={"content-disposition": disposition})) + + self.assertEqual(files, [f"./{API_NAME}"]) + + def test_unusable_header_name_falls_back_to_the_signed_url(self): + url = f"{SIGNED_URL}&response-content-disposition=attachment%3B%20filename%3D%22{API_NAME}%22" + for disposition in ("attachment; filename*=UTF-8''evil%00.zip", 'attachment; filename="NUL.zip"', + 'attachment; filename="victim.txt:stream.zip"', "attachment; filename*=UTF-8''%FF.zip"): + with self.subTest(disposition=disposition): + files, _ = self.download(export_response(url=url, headers={"content-disposition": disposition})) + + self.assertEqual(files, [f"./{API_NAME}"]) + + def test_an_existing_file_is_not_overwritten(self): + response = export_response(headers={"content-disposition": f'attachment; filename="{API_NAME}"'}) + runner = make_runner() + with ( + runner.isolated_filesystem(), + patch("ocl_cli.main.CLIConfig.load", return_value=FakeConfig()), + patch("ocl_cli.main.OCLAPIClient", return_value=FakeExportClient(response)), + ): + with open("important.txt", "w") as f: + f.write("keep") + os.symlink("important.txt", API_NAME) + result = runner.invoke(cli, ["repo", "export", "download", "PIH", "PIH", "1.8.24", "--type", "source"]) + + self.assertEqual(result.exit_code, 1, result.output) + self.assertIn("already exists", result.stderr) + with open("important.txt") as f: + self.assertEqual(f.read(), "keep") + + def test_a_missing_output_directory_is_an_error(self): + files, result = self.download(export_response(), "-o", "missing/", exit_code=1) + + self.assertEqual(files, []) + self.assertIn("doesn't exist", result.stderr) def test_unnamed_export_asks_for_output(self): response = export_response(url="https://api.example.test/orgs/PIH/sources/PIH/1.8.24/export/") @@ -108,3 +148,22 @@ def test_unnamed_export_asks_for_output(self): self.assertEqual(files, []) self.assertIn("Pass -o FILE", result.stderr) + + +def test_export_download_follows_the_redirect_to_the_named_file(httpx_mock): + from ocl_cli.api_client import OCLAPIClient + from ocl_cli.commands.export import export_filename + + httpx_mock.add_response( + method="GET", url="https://api.example.test/orgs/PIH/sources/PIH/1.8.24/export/", + status_code=302, headers={"location": SIGNED_URL}, + ) + httpx_mock.add_response( + method="GET", url=SIGNED_URL, content=b"zip-bytes", + headers={"content-disposition": f'attachment; filename="{API_NAME}"'}, + ) + + response = OCLAPIClient(base_url="https://api.example.test", token="t").export_download("PIH", "PIH", "1.8.24") + + assert response.content == b"zip-bytes" + assert export_filename(response) == API_NAME From ee04c51c2150f25726d3486319f7b2a48b0ff4d9 Mon Sep 17 00:00:00 2001 From: Jonathan Payne Date: Wed, 7 Oct 2026 10:47:32 -0400 Subject: [PATCH 4/5] OpenConceptLab/ocl_issues#2864 | export download: quote-aware header parsing, names may start with _ - @ - Content-Disposition parameters are parsed in order, so a quoted value containing ';' or 'filename=' can't be mistaken for the filename. - Storage names may start with _, - or @, as OCL owner names can. - README: an export the API can't name keeps its storage name. Addresses Codex pass 2 on ocl-cli#13. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- src/ocl_cli/commands/export.py | 36 +++++++++++++++++++++++----------- tests/test_export_commands.py | 14 +++++++++++++ 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 0156633..f39c4ab 100644 --- a/README.md +++ b/README.md @@ -207,7 +207,7 @@ ocl repo export download CIEL CIEL v2026-03-23 --type source # Saved to orgs_CIEL_sources_CIEL_v2026-03-23_.zip ``` -The API names every export `[owner-type]_[owner]_[repo-type]_[repo]_[version]_[expansion]_[lastUpdated].zip`; collection exports include the expansion. +The API names exports `[owner-type]_[owner]_[repo-type]_[repo]_[version]_[expansion]_[lastUpdated].zip`, with the expansion when the export has one. An older export the API can't name keeps its storage name; if no usable name comes back at all, the command asks for `-o FILE`. ### Concepts diff --git a/src/ocl_cli/commands/export.py b/src/ocl_cli/commands/export.py index c60630c..858947b 100644 --- a/src/ocl_cli/commands/export.py +++ b/src/ocl_cli/commands/export.py @@ -82,26 +82,40 @@ def delete(ctx, owner, repo, version, repo_type, owner_type): handle_api_error(e) -# The API names exports with only these characters, so anything else isn't an export name. -SAFE_FILENAME = re.compile(r"[A-Za-z0-9][A-Za-z0-9._@-]*") +# OCL names (and so export and storage names) use only these characters; a leading '.' is never a name. +SAFE_FILENAME = re.compile(r"[A-Za-z0-9_@-][A-Za-z0-9._@-]*") WINDOWS_DEVICE_NAMES = {"CON", "PRN", "AUX", "NUL", *(f"COM{i}" for i in range(1, 10)), *(f"LPT{i}" for i in range(1, 10))} +DISPOSITION_PARAM = re.compile(r'\s*;\s*([^\s=;]+)\s*=\s*("(?:[^"\\]|\\.)*"|[^;]*)') + + +def _disposition_params(value): + """Parameters of a Content-Disposition value, lower-cased names, quoted values unescaped.""" + params = {} + pos = value.find(";") + while 0 <= pos < len(value): + match = DISPOSITION_PARAM.match(value, pos) + if not match: + break + raw = match.group(2).strip() + if raw.startswith('"') and raw.endswith('"') and len(raw) > 1: + raw = re.sub(r"\\(.)", r"\1", raw[1:-1]) + params.setdefault(match.group(1).lower(), raw) + pos = match.end() + return params def _filename_from_content_disposition(value): """The filename in a Content-Disposition value, preferring filename* (RFC 6266), or None.""" - if not value: - return None - extended = re.search(r"(?:^|;)\s*filename\*\s*=\s*([^';\s]+)'[^']*'([^;\s]+)", value, re.I) - if extended: + params = _disposition_params(value or "") + extended = params.get("filename*", "") + if extended.count("'") >= 2: + charset, _, encoded = extended.split("'", 2) try: - return unquote(extended.group(2), encoding=extended.group(1), errors="strict") + return unquote(encoded, encoding=charset or "utf-8", errors="strict") except (LookupError, UnicodeDecodeError): pass - plain = re.search(r'(?:^|;)\s*filename\s*=\s*(?:"((?:[^"\\]|\\.)*)"|([^;\s]+))', value, re.I) - if plain: - return re.sub(r"\\(.)", r"\1", plain.group(1)) if plain.group(1) is not None else plain.group(2) - return None + return params.get("filename") def _usable_filename(name): diff --git a/tests/test_export_commands.py b/tests/test_export_commands.py index 45eaccc..4bad06a 100644 --- a/tests/test_export_commands.py +++ b/tests/test_export_commands.py @@ -117,6 +117,20 @@ def test_unusable_header_name_falls_back_to_the_signed_url(self): self.assertEqual(files, [f"./{API_NAME}"]) + def test_quoted_parameters_can_contain_semicolons(self): + disposition = 'attachment; note="x; filename=wrong.zip; y"; filename="right.zip"' + + files, _ = self.download(export_response(headers={"content-disposition": disposition})) + + self.assertEqual(files, ["./right.zip"]) + + def test_storage_name_may_start_with_an_underscore(self): + url = "https://exports.example.test/orgs/_PIH/_PIH_PIH_v1.8.24.zip?X-Amz-Signature=abc" + + files, _ = self.download(export_response(url=url)) + + self.assertEqual(files, ["./_PIH_PIH_v1.8.24.zip"]) + def test_an_existing_file_is_not_overwritten(self): response = export_response(headers={"content-disposition": f'attachment; filename="{API_NAME}"'}) runner = make_runner() From 22bd9ccfe7b2142ef563bbc350790c2e495db7f2 Mon Sep 17 00:00:00 2001 From: Jonathan Payne Date: Wed, 7 Oct 2026 10:54:34 -0400 Subject: [PATCH 5/5] OpenConceptLab/ocl_issues#2864 | export download: a name too long for the filesystem asks for -o FILE Addresses Codex pass 3 on ocl-cli#13. Co-Authored-By: Claude Opus 5.5 --- src/ocl_cli/commands/export.py | 7 +++++++ tests/test_export_commands.py | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/src/ocl_cli/commands/export.py b/src/ocl_cli/commands/export.py index 858947b..c7ddcf7 100644 --- a/src/ocl_cli/commands/export.py +++ b/src/ocl_cli/commands/export.py @@ -1,5 +1,6 @@ """Export commands: status, create, delete, download.""" +import errno import os import re from urllib.parse import parse_qs, unquote, urlsplit @@ -181,6 +182,12 @@ def download(ctx, owner, repo, version, repo_type, owner_type, output_path): f.write(response.content) except FileExistsError: raise click.ClickException(f"{output_path} already exists. Remove it, or pass -o FILE to overwrite.") from None + except OSError as e: + if mode != "xb" or e.errno != errno.ENAMETOOLONG: + raise + raise click.ClickException( + "The export's name is too long for this filesystem. Pass -o FILE to choose a name." + ) from None size = len(response.content) click.echo(f"Saved to {output_path} ({size:,} bytes)", err=True) diff --git a/tests/test_export_commands.py b/tests/test_export_commands.py index 4bad06a..38e972d 100644 --- a/tests/test_export_commands.py +++ b/tests/test_export_commands.py @@ -149,6 +149,15 @@ def test_an_existing_file_is_not_overwritten(self): with open("important.txt") as f: self.assertEqual(f.read(), "keep") + def test_a_name_too_long_for_the_filesystem_asks_for_output(self): + long_name = "orgs_PIH_sources_" + "x" * 300 + ".zip" + response = export_response(headers={"content-disposition": f'attachment; filename="{long_name}"'}) + + files, result = self.download(response, exit_code=1) + + self.assertEqual(files, []) + self.assertIn("too long", result.stderr) + def test_a_missing_output_directory_is_an_error(self): files, result = self.download(export_response(), "-o", "missing/", exit_code=1)