From 798f455639ba740e709b635339c1107a3aa5ee7e Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Mon, 9 Nov 2026 16:22:09 +1300 Subject: [PATCH 1/4] fix: pull files with non-ASCII characters in the path The multipart headers were parsed as bytes, and the email bytes parser replaces anything outside ASCII in a header value, so the filename no longer matched and pull() failed with "path not expected". Pebble writes the raw UTF-8 path, so the header bytes are now decoded as UTF-8 first. Co-Authored-By: Claude Opus 5.5 --- ops/pebble.py | 6 ++++-- test/test_pebble.py | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/ops/pebble.py b/ops/pebble.py index 641321db7..49f5e8846 100644 --- a/ops/pebble.py +++ b/ops/pebble.py @@ -3632,8 +3632,10 @@ def __init__(self, boundary: bytes | str): self._parser.feed(b'\r\n') def _process_header(self, data: bytearray): - parser = email.parser.BytesFeedParser() - parser.feed(data) + # The bytes parser replaces anything outside ASCII in a header value, + # which loses a non-ASCII path. Pebble sends the headers as UTF-8. + parser = email.parser.FeedParser() + parser.feed(data.decode('utf-8', 'surrogateescape')) self._headers = parser.close() content_disposition = self._headers.get_content_disposition() diff --git a/test/test_pebble.py b/test/test_pebble.py index 2b5ea79e9..5a3390b1d 100644 --- a/test/test_pebble.py +++ b/test/test_pebble.py @@ -2542,6 +2542,29 @@ def test_pull_path_error(self, client: MockClient): ), ] + def test_pull_non_ascii_path(self, client: MockClient): + client.responses.append(( + {'Content-Type': 'multipart/form-data; boundary=01234567890123456789012345678901'}, + b"""\ +--01234567890123456789012345678901\r +Content-Disposition: form-data; name="files"; filename="/tmp/cr\xc3\xa8me.txt"\r +\r +contents\r +--01234567890123456789012345678901\r +Content-Disposition: form-data; name="response"\r +\r +{ + "result": [{"path": "/tmp/cr\xc3\xa8me.txt"}], + "status": "OK", + "status-code": 200, + "type": "sync" +}\r +--01234567890123456789012345678901--\r +""", + )) + with client.pull('/tmp/crème.txt') as infile: # ruff: ignore[hardcoded-temp-file] + assert infile.read() == 'contents' + def test_pull_protocol_errors(self, client: MockClient): client.responses.append(({'Content-Type': 'c/t'}, b'')) with pytest.raises(pebble.ProtocolError) as excinfo: From f5c4623f5784c2a297afc2394ef467066e58d3f7 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Tue, 10 Nov 2026 14:50:30 +1300 Subject: [PATCH 2/4] test: check that pull keeps file bytes that are not valid UTF-8 Co-Authored-By: Claude Opus 5.5 --- test/test_pebble.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_pebble.py b/test/test_pebble.py index 5a3390b1d..95a788b90 100644 --- a/test/test_pebble.py +++ b/test/test_pebble.py @@ -2479,7 +2479,7 @@ def test_pull_binary(self, client: MockClient): --01234567890123456789012345678901\r Content-Disposition: form-data; name="files"; filename="/etc/hosts"\r \r -127.0.0.1 localhost # \xf0\x9f\x98\x80\nfoo\r\nbar\r +127.0.0.1 localhost # \xf0\x9f\x98\x80\nfoo\r\nbar\xff\xfe\x00\x80\r --01234567890123456789012345678901\r Content-Disposition: form-data; name="response"\r \r @@ -2495,7 +2495,7 @@ def test_pull_binary(self, client: MockClient): with client.pull('/etc/hosts', encoding=None) as infile: content = infile.read() - assert content == b'127.0.0.1 localhost # \xf0\x9f\x98\x80\nfoo\r\nbar' + assert content == b'127.0.0.1 localhost # \xf0\x9f\x98\x80\nfoo\r\nbar\xff\xfe\x00\x80' assert client.requests == [ ( From 92951d2543dda1572b6b7c08b31d4ccae329e844 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Wed, 11 Nov 2026 15:39:09 +1300 Subject: [PATCH 3/4] docs: say that surrogateescape matches os.fsdecode for pulled paths Co-Authored-By: Claude Opus 5.5 --- ops/pebble.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ops/pebble.py b/ops/pebble.py index 49f5e8846..a0873c802 100644 --- a/ops/pebble.py +++ b/ops/pebble.py @@ -3634,6 +3634,9 @@ def __init__(self, boundary: bytes | str): def _process_header(self, data: bytearray): # The bytes parser replaces anything outside ASCII in a header value, # which loses a non-ASCII path. Pebble sends the headers as UTF-8. + # surrogateescape decodes the same way os.fsdecode() does on Linux, so + # a path that isn't valid UTF-8 still compares equal to the str a + # charm got from os.listdir(). parser = email.parser.FeedParser() parser.feed(data.decode('utf-8', 'surrogateescape')) self._headers = parser.close() From bfa39acacc5858a418695ab2b4e0d0dcba79e70b Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 13 Nov 2026 15:09:59 +0000 Subject: [PATCH 4/4] fix: keep leading and trailing whitespace in pulled file paths Message.get_filename() strips whitespace from both ends of the filename, so pulling a path that ends in a space or tab got a name that didn't match the requested path, and raised ProtocolError('path not expected'). Read the filename parameter directly instead. Co-Authored-By: Claude Opus 5.5 --- ops/pebble.py | 7 ++++++- test/test_pebble.py | 28 ++++++++++++++++++++++++++++ test/test_real_pebble.py | 10 ++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/ops/pebble.py b/ops/pebble.py index a0873c802..4d1f9e168 100644 --- a/ops/pebble.py +++ b/ops/pebble.py @@ -39,6 +39,7 @@ import datetime import email.message import email.parser +import email.utils import enum import http.client import io @@ -3647,7 +3648,11 @@ def _process_header(self, data: bytearray): name = self._headers.get_param('name', header='content-disposition') if name == 'files': - filename = self._headers.get_filename() + # Not get_filename(), because that strips leading and trailing + # whitespace, which can be part of the path. + filename = self._headers.get_param('filename', header='content-disposition') + if isinstance(filename, tuple): + filename = email.utils.collapse_rfc2231_value(filename) if filename is None: raise ProtocolError('multipart "files" part missing filename') self._prepare_tempfile(filename) diff --git a/test/test_pebble.py b/test/test_pebble.py index 95a788b90..8b9dbdc76 100644 --- a/test/test_pebble.py +++ b/test/test_pebble.py @@ -2565,6 +2565,34 @@ def test_pull_non_ascii_path(self, client: MockClient): with client.pull('/tmp/crème.txt') as infile: # ruff: ignore[hardcoded-temp-file] assert infile.read() == 'contents' + @pytest.mark.parametrize('path', ['/srv/file ', '/srv/file\t', ' /srv/file', '/srv/ file ']) + def test_pull_path_with_surrounding_whitespace(self, client: MockClient, path: str): + client.responses.append(( + {'Content-Type': 'multipart/form-data; boundary=01234567890123456789012345678901'}, + b"""\ +--01234567890123456789012345678901\r +Content-Disposition: form-data; name="files"; filename="%s"\r +\r +contents\r +--01234567890123456789012345678901\r +Content-Disposition: form-data; name="response"\r +\r +%s\r +--01234567890123456789012345678901--\r +""" + % ( + path.encode(), + json.dumps({ + 'result': [{'path': path}], + 'status': 'OK', + 'status-code': 200, + 'type': 'sync', + }).encode(), + ), + )) + with client.pull(path) as infile: + assert infile.read() == 'contents' + def test_pull_protocol_errors(self, client: MockClient): client.responses.append(({'Content-Type': 'c/t'}, b'')) with pytest.raises(pebble.ProtocolError) as excinfo: diff --git a/test/test_real_pebble.py b/test/test_real_pebble.py index b43e0db94..730c9d805 100644 --- a/test/test_real_pebble.py +++ b/test/test_real_pebble.py @@ -208,6 +208,16 @@ def test_exec_send_stdin(self, client: pebble.Client): assert out == b'FOO\nBAR\n' assert err == b'' + @pytest.mark.parametrize('name', ['trailing ', 'trailing\t', 'crème']) + def test_push_pull_name_with_whitespace_or_non_ascii( + self, client: pebble.Client, tmp_path: pathlib.Path, name: str + ): + path = tmp_path / name + content = 'foo\nbar\nbaz-42' + client.push(path, content) + with client.pull(path) as f: + assert f.read() == content + def test_push_pull(self, client: pebble.Client): fname = os.path.join(tempfile.gettempdir(), f'pebbletest-{uuid.uuid4()}') content = 'foo\nbar\nbaz-42'