Skip to content

Report unterminated double-quoted values instead of dropping them - #618

Open
YoussefMansour9 wants to merge 1 commit into
vlucas:masterfrom
YoussefMansour9:fix/unterminated-double-quote
Open

YoussefMansour9 wants to merge 1 commit into
vlucas:masterfrom
YoussefMansour9:fix/unterminated-double-quote

Conversation

@YoussefMansour9

Copy link
Copy Markdown

Fixes #610.

What

When the input ends while still inside a double-quoted value, the entry and every
line after it are dropped with no error:

Dotenv::parse('FOO="bar');            // [] — no exception
Dotenv::parse("A=\"oops\nB=keep");    // [] — B is swallowed too
Dotenv::parse("FOO='bar");            // correctly throws "a missing closing quote"

The single-quoted path already reports the mistake, so a stray double quote silently
erases configuration rather than failing loudly.

Why it happens

Lines::process() accumulates a multiline value in $multilineBuffer and only appends
it to $output once looksLikeMultilineStop() matches. If the input ends while
$multiline is still true, the loop simply exits and the buffer is discarded, taking the
entry and every subsequent line with it.

The fix

Lines::process() now emits the still-open buffer once the input ends. The buffer reaches
the parser, which reports the same error the single-quoted path already produces:

Failed to parse dotenv file. Encountered a missing closing quote at ["bar].

Properly closed multiline values are unaffected, as are comments and whitespace.

On the version target

The linked issue suggests this belongs in the next major, since input that currently
returns an empty or partial array would start throwing. I have opened it against master
because #615 made a comparable parsing change in this same method on this branch, but I am
happy to retarget if you would rather hold it.

Worth weighing in that decision: the current behaviour is silent data loss. Anything
relying on it is relying on a .env typo quietly wiping out the rest of the file.

Tests

Three cases added to DotenvTest: the unterminated value on its own, the case where it
swallows a following line, and the single-quoted form as a control showing both now produce
the same error. The first two fail on master and pass with this change; the control
passes either way.

Verified locally: full suite (283 tests), phpstan analyze clean, and src/Parser/Lines.php
at 34/34 statements covered.

When the input ended while still inside a double-quoted value, the buffered
content was discarded, so the entry and every line after it disappeared with
no error:

    Dotenv::parse('FOO="bar');          // []
    Dotenv::parse("A=\"oops\nB=keep");  // [] — B is swallowed too

The single-quoted path already reports a missing closing quote for the same
mistake, so a stray double quote silently erased configuration instead of
failing loudly.

`Lines::process()` now emits the still-open buffer once the input ends. The
parser then reports the same "missing closing quote" error the single-quoted
path produces, and stops swallowing the following lines. Properly closed
multiline values are unaffected.

Fixes vlucas#610
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.

Unterminated double-quoted value at end of input is silently dropped

1 participant