Repository navigation
Conversation
git reads an argument starting with a dash as an option, and a value
from a user can be one: getRevision('--output=/path') or a checkout, a
merge, a reference name of that kind could write a file, and
isValidRepository('--upload-pack=...') ran the command in a repository
with a remote. git 1.8.4 has no --end-of-options, and "--" starts the
paths of most commands, ls-remote taking it only since git 2.8.
Both drivers refuse a revision, a reference, a hash or a URL starting
with a dash with an InvalidArgumentException (Util\LeadingDash), for the
same parameters. A path may start with one: init and clone end the
options of git with "--". run() and runProcess() pass their arguments
as they are, which the doc says.
A URL like https://user:token@host/repository.git ended up, token included, in the message of the ProcessException of a failed clone, through its command line, and in the logs of CliDriver, through the command and its output (git remote -v...). The messages of LibGit2Driver named the URL too. Util\Credentials::redact() replaces the user information of the URLs by "***" in the messages of ProcessException and of libgit2's errors, and in the logs. getOutput() and getErrorOutput() of ProcessException still return the output of git as it is.
Hooks joined the name to the hooks directory as it was:
set('../evil', ...) wrote .git/evil, and remove('../config') deleted
the configuration of the repository.
A hook name is made of letters, digits, "-", "_" and ".", and doesn't
start with a dot, as git for Windows also runs "pre-commit.exe": every
method of Hooks throws an InvalidArgumentException for any other name.
git reads the configuration of the user running PHP, which changed what CliDriver parsed: color.ui=always, log.showSignature, diff.noprefix, diff.external, diff.suppressBlankEmpty, textconv filters or column.ui made reads throw or return wrong data, a short hash prefixed with the verification of a signature for instance. diff.algorithm, diff.context, diff.orderFile... changed the diff of the working tree, unlike the one of a commit, which diff-tree reads without them, and log.follow and branch.sort the logs and the branches of a commit. CliRepositoryDriver sets them back to git's defaults: with the options of its commands (--no-color, --no-ext-diff, --no-textconv, the prefixes, the diff algorithm, the context...), and with -c on every command, run() included, where git 1.8.4 has none. Identity, credentials, URL rewrites, core.abbrev or blame.ignoreRevsFile still apply, like with git. The tests no longer depend on the configuration of the developer: the bootstrap points HOME to tests/fixtures/home, whose .gitconfig sets everything gitlib sets back, so every run checks it. test-git-versions needs no HOME of its own anymore, and leaves nothing in the temporary directory.
git runs the command an "ext::" URL names, to talk to it: an
application checking or cloning a URL from its users,
isValidRepository('ext::sh -c ...') for instance, ran the command with
git before 2.12, which made the protocol.allow setting forbid ext by
default. git 1.8.4, still supported, runs it.
Both drivers refuse such a URL with an InvalidArgumentException, like a
URL starting with a dash: Util\RemoteUrl checks both. The tests of the
URLs move to RemoteUrlTest.
Before 2.14.1 (CVE-2017-1000117), git passes the "user@host" of an ssh URL to ssh, which reads "ssh://-oProxyCommand=.../repository" as an option and runs the command. git passes the host of a git:// URL to the command of core.gitProxy the same way. Util\RemoteUrl refuses, with an InvalidArgumentException, an ssh, scp-like or git URL whose host or user starts with a dash, even encoded ("%2D", or "%5B" for a bracket), as git decodes the URL before it reads its host. "git update-ref" writes any name: ReferenceBag::delete('HEAD') deleted .git/HEAD with CliDriver, which left no repository, and creating the short name "master" wrote it next to HEAD. Util\ReferenceName makes both drivers refuse, with an InvalidArgumentException, to create, update or delete a reference outside refs/.
The credentials of a URL still leaked: - Credentials::redact() stopped at a "#" or a "?" of the password, which git and libgit2 accept unencoded there; - the exceptions of Symfony Process, the previous exception of a timeout or of a process that couldn't start, hold the command line; - the InvalidArgumentException of a URL holding a NUL byte or starting with a dash named it as it was; - with zend.exception_ignore_args off, the default of php.ini-development, the traces of the exceptions hold the arguments of the calls. redact() reads the user information up to the last "@" before the host, the exceptions of Symfony are no longer chained, NulByte and LeadingDash redact their messages, and every parameter taking a URL, or arguments of git, is #[\SensitiveParameter].
git reads the replacements of objects (git replace, refs/replace/): whoever could push such a reference to a repository changed the message of a commit, or the content of a blob, CliDriver read for a hash, where libgit2 ignores them. CliDriver passes --no-replace-objects to every command, run() included. "git update-ref" writes through a symbolic reference: deleting refs/remotes/origin/HEAD deleted the branch it points to, origin/main, and moving it moved that branch. CliDriver passes --no-deref, which gives the behavior LibGit2Driver already had.
Hooks::has() used file_exists(), false for a symbolic link to a file not deployed yet: set() then wrote through the link, out of the hooks directory. A link is a hook, whatever its target: set() and setSymlink() refuse it, as a hook that exists already, get() throws a RuntimeException, as it can't read it, and remove() removes the link.
When no revision had the name, "git checkout <name>" restored the file
of this name, discarding its changes: WorkingCopy::checkout('README.md')
did. git also read a revision named like a file of the working tree as
ambiguous, which made getShortHash(), the data of a commit and the
diffs fail. CliDriver ends the revisions with "--".
WorkingCopy::checkout() and merge() resolve the revision first, and
throw a ReferenceNotFoundException for a name that no revision has, with
both drivers. checkout('feature') created the local branch of
origin/feature with CliDriver, where LibGit2Driver threw: gitlib doesn't
guess anymore, checkoutNewBranch() creates it. A range, which
"rev-parse --verify" prints before exiting with 1, throws a
ReferenceNotFoundException too, and ":/text" is resolved before
"^{commit}", which would be part of its text.
checkoutNewBranch() passes the name of its start point to the driver,
instead of its hash: the branch tracks it when it is a remote branch,
like with "git checkout -b" and branch.autoSetupMerge=true, git's
default, whatever the configuration. CliDriver sets
branch.autoSetupMerge and branch.autoSetupRebase back, and
LibGit2Driver finds the remote and its branch through the refspecs of
the remotes. Like git, both refuse an ambiguous start point, and create
no branch when the checkout fails. The configuration of the tests sets
both to "always", so every run checks it.
With diff.submodule=log, "git diff" writes the log of a submodule instead of its patch: DiffParser stopped there, silently, and the files after the submodule were lost. CliDriver passes --submodule=short to the diffs of the working tree. DiffParser reads an unmerged path ending a diff, and throws a RuntimeException for anything left it can't read. Its message quotes the start of the line only, as the lines of a diff come from the files of the repository.
The dates of commits and tags were in UTC: the offset of the author, committer or tagger was lost, as in 1.x. Both drivers return them at this offset, like git log shows them. The instant doesn't change, nor do comparisons and getTimestamp(), but format() returns the local time of the author. An offset of more than 14 hours or 59 minutes, which no timezone has, in a corrupt object, gives UTC, like libgit2 does.
LibGit2Driver differed from git, and from CliDriver:
- paths weren't normalized: "./README.md", "dir/../file" or "dir//file"
matched nothing in a log or a blame, and a path starting with ":",
git's magic, matched a file of this name. It now throws an
UnsupportedOperationException;
- a reference other than a tag pointing to an annotated tag, like one a
fetch puts under refs/remotes/, wasn't peeled to its commit, nor
listed among the branches of the commit;
- checkout('HEAD') detached HEAD, and @{-1}, the branch checked out
before, wasn't understood;
- the message of a merge named a full reference name, "branch~1" or
@{-1} unlike git, without the message of an annotated tag, and a merge
into a branch yet to be born threw instead of fast-forwarding it;
- a file added with "git add -N" was a staged empty file, unlike git
since 2.19, and a file the working tree changed back after staging it
was listed in the diff of the working tree;
- a reftable repository or a bundle threw a DriverException, instead of
the UnsupportedOperationException of an operation the driver can't
perform;
- /etc/gitconfig was read whatever GIT_CONFIG_NOSYSTEM.
The checks for a blame hunk and a patch that libgit2 1.9 never leaves
null go: they were dead code.
A log of thousands of revisions, the branches of a large repository for instance, exceeded the limit of the command line, 2 MB by default on Linux: git couldn't start. CliDriver passes the hashes of a log and of countCommits() to git on its standard input, with --stdin.
The message of a ProcessException held the command line, the whole error output and the whole output of git, over many lines: no sentence, and the data of the repository in the logs. It is one line, with the git command, its exit code and the error of git, the first line of its error output starting with "fatal:" or "error:", after the warnings, or its first line otherwise: 'The git command "clone" failed (exit 128): fatal: ...'. getErrorOutput() and getOutput() still return the outputs of git. The constructor takes the git command, optional. When git can't start, a missing binary, a command line too long or a working directory removed, CliDriver throws a DriverException instead of a ProcessException, which says so in one line, without the command line: 'The git command "clone" could not start: ...'.
- Repository::getHead() returns a Reference\Branch or a Commit:
Reference didn't declare getName(), which the doc calls. It now does,
abstract. HEAD pointing to a tag, which git allows, throws a
ReferenceNotFoundException.
- The constructors of Commit, Tree, Blob, Revision, the references and
ReferenceBag are internal: Repository returns the same commit, tree or
blob while the application uses it, and reads the references again
after its writes. Extending Revision or Reference isn't supported.
- getSize() returned kilobytes, followed links, and threw for a link to
a missing file in the git directory, like a hook not deployed yet.
getSizeInBytes() returns bytes, follows no link, and leaves out a file
git deletes meanwhile. getSize() is deprecated.
- Exception\UnexpectedValueException, which gitlib no longer throws, is
deprecated.
- getBlob(), getTree() or getCommit() of an abbreviated hash throws
'The object "56953e3" is not a blob.' or 'The object "56953e3" does
not exist.', instead of 'The revision "56953e3^{blob}" does not
exist.'.
- BlameLineData names its line numbers like Blame\Line: number and
originalNumber.
- DriverInterface and RepositoryDriverInterface list the exceptions their methods throw: a ReferenceNotFoundException for a missing object, an UnsupportedOperationException, and the URLs and arguments they refuse. - DiffFileData::$newIndex is the hash of the content of a file of the working tree, which both drivers give, not null. - getCommitHash() is the object a tag points to, which may be a tree or a blob. - benchmark.php needs a clone of gitlib: Composer installs neither it nor symfony/console. - LibGit2Driver::isSupported() takes no path: the doc shows how to check one. - The errors list RuntimeException, and the ProcessTimedOutException of run(). - The CHANGELOG no longer announces Symfony 6.4, which gitlib doesn't support. - Typos.
- The bootstrap removes every variable of git from the environment of the tests, but GIT_COMMAND: run from a hook, like pre-commit, they got GIT_DIR or GIT_INDEX_FILE, and wrote the repository of the hook. BootstrapTest checks it. - The tests of worktrees, reftable and SHA-256 skip by version of git, instead of on any failure of git. - The tests expecting a RuntimeException expect the exception gitlib throws, and check the messages, of hooks too. - Log::getDiff(), which ignores the paths of the log, and the pairing of renames, which differs between the drivers, have tests. - shivammathur/setup-php, a third-party action, is pinned by commit, like setup-castor. - PHPStan runs without baseline: the types it reported are declared.
Security fixes, and align both drivers with git
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.
The commits of #254, which was merged into dto after #253 was merged into main.