[Dotenv] Fix can not load BOM-signed env files#58216
Closed
hosni wants to merge 1 commit intosymfony:6.4from
hosni:6.4
Closed
[Dotenv] Fix can not load BOM-signed env files#58216hosni wants to merge 1 commit intosymfony:6.4from hosni:6.4
hosni wants to merge 1 commit intosymfony:6.4from
hosni:6.4
Conversation
Contributor
Author
|
Unit tests failed, but that's not my fault! |
derrabus
requested changes
Sep 10, 2024
|
|
||
| * 6.4.11 (2024-08-30) | ||
|
|
||
| * bug #58214 [Dotenv] Fix can not load BOM-signed env files (hosni) |
Member
There was a problem hiding this comment.
Please revert. This file is generated.
nicolas-grekas
requested changes
Sep 10, 2024
Member
There was a problem hiding this comment.
dotenv files are modeled after sourced shell scripts.
I just tested and neither sh nor bash support parsing dotenv files that start with a BOM.
This means we should not do it.
Instead, we might consider throwing an exception when one is found.
I'd suggest doing it in the doLoad method.
Also, let's use str_starts_with to check for it.
Member
|
Please submit a new PR if you want to provide the exception I suggested. |
Member
|
see #58274 for the exception |
nicolas-grekas
added a commit
that referenced
this pull request
Sep 16, 2024
…files with BOM (xabbuh) This PR was merged into the 5.4 branch. Discussion ---------- [Dotenv] throw a meaningful exception when parsing dotenv files with BOM | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #58214 | License | MIT This replaces #58216. Other than in that PR I did not account for UTF-16 and UTF-32 byte-order-mark variants as that would IMO require us to first reliably detect the file encoding. Commits ------- b4f3eaa throw a meaningful exception when parsing dotenv files with BOM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In this PR, I fixed the load env file BOM characters.
You can see the full explanation of the issue: #58214
More about BOM: Byte Order Mark