fix(env): never write an env format this binary does not understand - #1401
Merged
Conversation
A definition published to the store reaches every install within a day, whatever binary it runs, and there is no engine version gate in the index or the schema. A definition naming a format added after a given release therefore lands on machines that cannot honour it, and each of the three write switches fell through to the dotenv writer, which appended key=value lines into whatever file the definition named. Driven against a binary from before the php-vars format, that turned a Drupal settings file into one PHP refuses to parse: the site is down, on a machine whose owner did nothing. Writing goes through one function now, and it refuses a format it does not know, saying so and leaving the project exactly as it was. Reading an unknown format returns nothing rather than inventing keys out of whatever the file happens to contain. That is the worst a binary too old for its definition should ever do. A definition also needs a way to describe a file older binaries cannot handle without breaking them, so env gains app_file and app_format, naming the file the application itself reads. Where they are declared lerd reads and writes that file, and the existing file and fallback_file describe what an older binary should do with the same definition. They are a separate pair for exactly that reason: an unknown field is ignored on parse, so a definition can carry both answers at once and every install behaves as well as its version allows.
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.
A definition published to the store reaches every install within a day, whatever binary it runs, and there is no engine version gate in the index or the schema. So a definition naming an env format added after a given release lands on machines that cannot honour it, and each of the three write switches fell through to the dotenv writer, which appends key=value lines into whatever file the definition names.
Driven against a binary built from before the php-vars format, with a definition using it:
A dead site, on a machine whose owner did nothing but leave lerd running.
Writing goes through one function now, and it refuses a format it does not know, says so, and leaves the project exactly as it was. Reading an unknown format returns nothing rather than inventing keys out of whatever the file happens to contain. That is the worst a binary too old for its definition should ever do, and it is what makes any future format safe to publish.
A definition also needs a way to describe a file older binaries cannot handle without breaking them, so env gains app_file and app_format, naming the file the application itself reads. Where they are declared lerd reads and writes that file; the existing file and fallback_file describe what an older binary should do with the same definition. They are a separate pair for exactly that reason: an unknown field is ignored on parse, so one definition carries both answers and every install behaves as well as its version allows.
Verified both ways against the same definition. The old binary appends define() calls that Drupal ignores, which is what it does today, and the file still parses. The current binary writes the $databases array Drupal actually reads.