Skip to content

Commit

Permalink
fix: make push_path open in binary mode so it works on non-text files (
Browse files Browse the repository at this point in the history
…canonical#1458)

There was a bug in `push_path` where it called `open` with the default
(text) mode on the local files, causing it to not work on binary files
(more specifically, if they weren't valid UTF-8). This is the fix for
that -- `push_path` should treat the files as raw bytes, so read in
binary mode.

Also change the test to ensure binary / non-UTF-8 files work as
expected.

Fixes canonical#1455.
  • Loading branch information
benhoyt authored Nov 20, 2024
1 parent 3fb8548 commit 6a17bf9
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
2 changes: 1 addition & 1 deletion ops/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -2662,7 +2662,7 @@ def local_list(source_path: Path) -> List[pebble.FileInfo]:
if info.type is pebble.FileType.DIRECTORY:
self.make_dir(dstpath, make_parents=True)
continue
with open(info.path) as src:
with open(info.path, 'rb') as src:
self.push(
dstpath,
src,
Expand Down
6 changes: 4 additions & 2 deletions test/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -1369,8 +1369,8 @@ def test_recursive_push_and_pull(case: PushPullCase):
for file in case.files:
fpath = os.path.join(push_src.name, file[1:])
os.makedirs(os.path.dirname(fpath), exist_ok=True)
with open(fpath, 'w') as f:
f.write('hello')
with open(fpath, 'wb') as f:
f.write(b'push \xc3\x28') # invalid UTF-8 to ensure binary works
if case.dirs:
for directory in case.dirs:
fpath = os.path.join(push_src.name, directory[1:])
Expand Down Expand Up @@ -1402,6 +1402,8 @@ def test_recursive_push_and_pull(case: PushPullCase):
), f'push_path gave wrong expected errors: want {case.errors}, got {errors}'
for fpath in case.want:
assert c.exists(fpath), f'push_path failed: file {fpath} missing at destination'
content = c.pull(fpath, encoding=None).read()
assert content == b'push \xc3\x28'
for fdir in case.want_dirs:
assert c.isdir(fdir), f'push_path failed: dir {fdir} missing at destination'

Expand Down

0 comments on commit 6a17bf9

Please sign in to comment.