Skip to content

fopen in binary mode#141

Merged
alganet merged 2 commits intoRespect:masterfrom
divinity76:patch-1
Mar 18, 2026
Merged

fopen in binary mode#141
alganet merged 2 commits intoRespect:masterfrom
divinity76:patch-1

Conversation

@divinity76
Copy link
Contributor

some shitty OS's (for example MS Windows) may corrupt binary data like the sample .jpg files with fopen text mode, and the default fopen mode if none is specified, may be text mode (the only instances i can think of off-hand is MS Windows and classic MacOS, and PHP does not even support classic MacOS, but Windows is supported), quoting the docs:

The default translation mode depends on the SAPI and version of PHP that you are using, so you are encouraged to always specify the appropriate flag for portability reasons. You should use the 't' mode if you are working with plain-text files and you use \n to delimit your line endings in your script, but expect your files to be readable with applications such as notepad. You should use the 'b' in all other cases.

so.. for Windows-portability reasons, one should pretty much always fopen in binary mode.

some shitty OS's (for example MS Windows) may corrupt binary data like the sample .jpg files with fopen text mode, and the default fopen mode if none is specified, may be text mode (the only instances i can think of off-hand is MS Windows and classic MacOS, and PHP does not even support classic MacOS, but Windows is supported), quoting the docs:

> The default translation mode depends on the SAPI and version of PHP that you are using, so you are encouraged to always specify the appropriate flag for portability reasons. You should use the 't' mode if you are working with plain-text files and you use \n to delimit your line endings in your script, but expect your files to be readable with applications such as notepad. You should use the 'b' in all other cases.

so.. for Windows-portability reasons, one should pretty much always fopen in binary mode.
@alganet
Copy link
Member

alganet commented Mar 18, 2026

We don't actually do the fopens, so I think it's fine to leave the docs as they are. It reveals an open wound in the PHP for windows (I'm not even sure it still works that way, but I'm assuming it does).

So, for now, I'll be closing this. Thanks for the PR anyway! 🐼

@alganet alganet closed this Mar 18, 2026
@divinity76
Copy link
Contributor Author

@alganet you really don't see the problem with sample code opening binary files in text mode.
Wow.

@alganet alganet reopened this Mar 18, 2026
@alganet
Copy link
Member

alganet commented Mar 18, 2026

@divinity76 my mistake, you are right. We shouldn't feature examples opening binary files in text mode.

I was honestly not expecting you to care about this! Are you willing to update the PR with a git-conflict-free change? If you are, I can accept the PR.

If not, I can fix it separately and mention it here too.

@divinity76
Copy link
Contributor Author

divinity76 commented Mar 18, 2026

@alganet i fixed the merge conflict

BUT i'm actually less enthusiastic about this PR now.

This is turning into a bit of a history lesson,
BUT when I made this PR, 2019, it was true that

The default translation mode depends on the SAPI and version of PHP

But today, it has been fixed: the default fopen mode is always binary mode, regardless of SAPI, as long as you're using a supported version of PHP (at least PHP>=8.2). The only way to open files in text mode today is to specify "rt" (t for text mode)

Still, for people using multiple programming languages (I'm among them!), it's a good habit:

If you write in Python

fp = open("img.jpg", "r")

or you write in C or C++

FILE *fp = fopen("img.jpg", "r");

You're writing code that is broken on Windows (opening jpg files in text mode), but works everywhere else.

fopen("img.jpg","r") is today harmless in PHP though.. still a bad habit for multi-language users.

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.67%. Comparing base (94d7d69) to head (9589340).

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #141   +/-   ##
=========================================
  Coverage     96.67%   96.67%           
  Complexity      430      430           
=========================================
  Files            31       31           
  Lines          1082     1082           
=========================================
  Hits           1046     1046           
  Misses           36       36           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alganet
Copy link
Member

alganet commented Mar 18, 2026

@divinity76 yep, I agree. Bad practice overall for multiple languages and our docs should be more pedantic in that aspect.

Thanks for caring about this, for adjusting the PR and for the history lesson too! 🐼

@alganet alganet merged commit 1d8db28 into Respect:master Mar 18, 2026
3 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.

3 participants