Skip to content

Open, read and write files in binary mode. #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 22, 2025

Conversation

prsephton
Copy link
Contributor

Paster breaks while copying [some] binary files (copydir.py).

In Python 3, opening files with the "r" flag produces strings on read. To do so, the file content is decoded using utf-8 by default. Some files, like .jpg images may not be valid utf8, and so cause breakage.

This patch suggests that files are always opened and read in binary (rb) mode, and only decoded for processing if we know they are intended to be used templates. Everything else is simply written back as a bytestream.

Copy link
Collaborator

@cdent cdent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable.

Is there a simple way to add a test?

@prsephton
Copy link
Contributor Author

This seems reasonable.

Is there a simple way to add a test?

I think it may be possible to write a unit test using StringIO/BytesIO. One would need to mock everything to do with the directory scan - eg. os.listdir. etc, and file open/close. I am not sure such a test would be all that valid in the end, as it would likely end up being quite complex, possibly buggy and not proving anything worthwhile.

Still, I am open to suggestions if you want a test, and willing to put the effort in if you would like to provide guidance.

@cdent
Copy link
Collaborator

cdent commented Jan 14, 2025

Still, I am open to suggestions if you want a test, and willing to put the effort in if you would like to provide guidance.

Not required, was more curiosity than anything else.

I need to fix the GitHub workflows and then can see about merging this. Thanks for the contribution.

@cdent cdent merged commit 3d6ae15 into pasteorg:master Jan 22, 2025
0 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants