fix: don't silently drop an unterminated multiline value - #616
Closed
raphyabak wants to merge 1 commit into
Closed
Conversation
When a double-quoted value was left unterminated at the end of the input, Lines::process() discarded the buffered content instead of emitting it, so the whole entry - and every line after it, since they were still being buffered as part of the same unclosed multiline value - vanished with no error. This was inconsistent with the single-quoted case, which already reports a missing closing quote: single-quoted values aren't treated as multiline at all, so an unterminated one reaches EntryParser directly and fails loudly. Emit the still-open buffer as a final output entry instead of discarding it. It then reaches EntryParser exactly like a normal entry, and the lexer ends in one of the REJECT_STATES at EOF, so it raises the same 'a missing closing quote' error the single-quoted path already produces. A properly closed multiline value is unaffected, since the buffer is already flushed to $output by the time the loop ends. This is a behaviour change - input that previously parsed to an empty or truncated array now throws - but the prior behaviour was silent data loss.
Collaborator
|
This is a major breaking change, which is why I opened an issue to track this, instead of implementing it. Please do not just point AI an issue without maintainer approval - if I wanted to do that, I could do that myself. |
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
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.
Summary
Fixes #610
When a double-quoted value was left unterminated at the end of the input,
Lines::process()discarded the buffered content instead of emitting it, so the whole entry — and every line after it, since they were still being buffered as part of the same unclosed multiline value — vanished with no error:This was inconsistent with the single-quoted case, which already reports a missing closing quote — single-quoted values aren't treated as multiline at all, so an unterminated one reaches
EntryParserdirectly and fails loudly:Fix
Emit the still-open buffer as a final output entry instead of discarding it when the input ends mid-multiline-value. It then reaches
EntryParserexactly like a normal entry, and the lexer ends in one of theREJECT_STATESat EOF, so it raises the same"a missing closing quote"error the single-quoted path already produces. A properly closed multiline value is unaffected, since the buffer is already flushed to$outputby the time the loop ends.As noted in the issue, this is a behaviour change — input that previously parsed to an empty or truncated array now throws — so it targets
masterrather than a4.xpatch release. The prior behaviour was silent data loss, which is the stronger correctness concern.Test plan
tests/Dotenv/Parser/LinesTest.php: an unterminated double-quoted value is emitted (not dropped), and a following line is no longer silently swallowed.Lines::process()returning[]), and pass with the fix.vendor/bin/phpunit): 282 tests, all passing (280 pre-existing + 2 new).vendor/bin/phpstan analyze(configured scope:src, levelmax): no errors.