summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* http: fix v1 protocol tests with apache httpd < 2.4bw/protocol-v1Todd Zullinger2018-01-041-6/+4
| | | | | | | | | | | | | | | | | | The apache config used by tests was updated to use the SetEnvIf directive to set the Git-Protocol header in 19113a26b6 ("http: tell server that the client understands v1", 2017-10-16). Setting the Git-Protocol header is restricted to httpd >= 2.4, but mod_setenvif and the SetEnvIf directive work with lower versions, at least as far back as 2.0, according to the httpd documentation: https://httpd.apache.org/docs/2.0/mod/mod_setenvif.html Drop the restriction. Tested with httpd 2.2 and 2.4. Signed-off-by: Todd Zullinger <tmz@pobox.com> Acked-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Documentation: document Extra ParametersJonathan Tan2017-10-172-7/+44
| | | | | | | | | | Document the server support for Extra Parameters, additional information that the client can send in its first message to the server during a Git client-server interaction. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* ssh: introduce a 'simple' ssh variantBrandon Williams2017-10-175-61/+111
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When using the 'ssh' transport, the '-o' option is used to specify an environment variable which should be set on the remote end. This allows git to send additional information when contacting the server, requesting the use of a different protocol version via the 'GIT_PROTOCOL' environment variable like so: "-o SendEnv=GIT_PROTOCOL". Unfortunately not all ssh variants support the sending of environment variables to the remote end. To account for this, only use the '-o' option for ssh variants which are OpenSSH compliant. This is done by checking that the basename of the ssh command is 'ssh' or the ssh variant is overridden to be 'ssh' (via the ssh.variant config). Other options like '-p' and '-P', which are used to specify a specific port to use, or '-4' and '-6', which are used to indicate that IPV4 or IPV6 addresses should be used, may also not be supported by all ssh variants. Currently if an ssh command's basename wasn't 'plink' or 'tortoiseplink' git assumes that the command is an OpenSSH variant. Since user configured ssh commands may not be OpenSSH compliant, tighten this constraint and assume a variant of 'simple' if the basename of the command doesn't match the variants known to git. The new ssh variant 'simple' will only have the host and command to execute ([username@]host command) passed as parameters to the ssh command. Update the Documentation to better reflect the command-line options sent to ssh commands based on their variant. Reported-by: Jeffrey Yasskin <jyasskin@google.com> Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* i5700: add interop test for protocol transitionBrandon Williams2017-10-171-0/+68
| | | | | Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* http: tell server that the client understands v1Brandon Williams2017-10-174-0/+96
| | | | | | | | | | | Tell a server that protocol v1 can be used by sending the http header 'Git-Protocol' with 'version=1' indicating this. Also teach the apache http server to pass through the 'Git-Protocol' header as an environment variable 'GIT_PROTOCOL'. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* connect: tell server that the client understands v1Brandon Williams2017-10-172-5/+255
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Teach the connection logic to tell a serve that it understands protocol v1. This is done in 2 different ways for the builtin transports, both of which ultimately set 'GIT_PROTOCOL' to 'version=1' on the server. 1. git:// A normal request to git-daemon is structured as "command path/to/repo\0host=..\0" and due to a bug introduced in 49ba83fb6 (Add virtualization support to git-daemon, 2006-09-19) we aren't able to place any extra arguments (separated by NULs) besides the host otherwise the parsing of those arguments would enter an infinite loop. This bug was fixed in 73bb33a94 (daemon: Strictly parse the "extra arg" part of the command, 2009-06-04) but a check was put in place to disallow extra arguments so that new clients wouldn't trigger this bug in older servers. In order to get around this limitation git-daemon was taught to recognize additional request arguments hidden behind a second NUL byte. Requests can then be structured like: "command path/to/repo\0host=..\0\0version=1\0key=value\0". git-daemon can then parse out the extra arguments and set 'GIT_PROTOCOL' accordingly. By placing these extra arguments behind a second NUL byte we can skirt around both the infinite loop bug in 49ba83fb6 (Add virtualization support to git-daemon, 2006-09-19) as well as the explicit disallowing of extra arguments introduced in 73bb33a94 (daemon: Strictly parse the "extra arg" part of the command, 2009-06-04) because both of these versions of git-daemon check for a single NUL byte after the host argument before terminating the argument parsing. 2. ssh://, file:// Set 'GIT_PROTOCOL' environment variable with the desired protocol version. With the file:// transport, 'GIT_PROTOCOL' can be set explicitly in the locally running git-upload-pack or git-receive-pack processes. With the ssh:// transport and OpenSSH compliant ssh programs, 'GIT_PROTOCOL' can be sent across ssh by using '-o SendEnv=GIT_PROTOCOL' and having the server whitelist this environment variable. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* connect: teach client to recognize v1 server responseBrandon Williams2017-10-171-4/+26
| | | | | | | | | | Teach a client to recognize that a server understands protocol v1 by looking at the first pkt-line the server sends in response. This is done by looking for the response "version 1" send by upload-pack or receive-pack. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* upload-pack, receive-pack: introduce protocol version 1Brandon Williams2017-10-172-1/+36
| | | | | | | | | | | | | Teach upload-pack and receive-pack to understand and respond using protocol version 1, if requested. Protocol version 1 is simply the original and current protocol (what I'm calling version 0) with the addition of a single packet line, which precedes the ref advertisement, indicating the protocol version being spoken. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* daemon: recognize hidden request argumentsBrandon Williams2017-10-171-9/+62
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A normal request to git-daemon is structured as "command path/to/repo\0host=..\0" and due to a bug introduced in 49ba83fb6 (Add virtualization support to git-daemon, 2006-09-19) we aren't able to place any extra arguments (separated by NULs) besides the host otherwise the parsing of those arguments would enter an infinite loop. This bug was fixed in 73bb33a94 (daemon: Strictly parse the "extra arg" part of the command, 2009-06-04) but a check was put in place to disallow extra arguments so that new clients wouldn't trigger this bug in older servers. In order to get around this limitation teach git-daemon to recognize additional request arguments hidden behind a second NUL byte. Requests can then be structured like: "command path/to/repo\0host=..\0\0version=1\0key=value\0". git-daemon can then parse out the extra arguments and set 'GIT_PROTOCOL' accordingly. By placing these extra arguments behind a second NUL byte we can skirt around both the infinite loop bug in 49ba83fb6 (Add virtualization support to git-daemon, 2006-09-19) as well as the explicit disallowing of extra arguments introduced in 73bb33a94 (daemon: Strictly parse the "extra arg" part of the command, 2009-06-04) because both of these versions of git-daemon check for a single NUL byte after the host argument before terminating the argument parsing. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* protocol: introduce protocol extension mechanismsBrandon Williams2017-10-176-0/+144
| | | | | | | | | | | | | | Create protocol.{c,h} and provide functions which future servers and clients can use to determine which protocol to use or is being used. Also introduce the 'GIT_PROTOCOL' environment variable which will be used to communicate a colon separated list of keys with optional values to a server. Unknown keys and values must be tolerated. This mechanism is used to communicate which version of the wire protocol a client would like to use with a server. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pkt-line: add packet_write functionBrandon Williams2017-10-172-0/+7
| | | | | | | | | | Add a function which can be used to write the contents of an arbitrary buffer. This makes it easy to build up data in a buffer before writing the packet instead of formatting the entire contents of the packet using 'packet_write_fmt()'. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* connect: in ref advertisement, shallows are lastJonathan Tan2017-09-271-66/+123
| | | | | | | | | | | | | | | | | | | Currently, get_remote_heads() parses the ref advertisement in one loop, allowing refs and shallow lines to intersperse, despite this not being allowed by the specification. Refactor get_remote_heads() to use two loops instead, enforcing that refs come first, and then shallows. This also makes it easier to teach get_remote_heads() to interpret other lines in the ref advertisement, which will be done in a subsequent patch. As part of this change, this patch interprets capabilities only on the first line in the ref advertisement, printing a warning message when encountering capabilities on other lines. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Sync with 2.14.2Junio C Hamano2017-09-2611-56/+183
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | * maint: Git 2.14.2 Git 2.13.6 Git 2.12.5 Git 2.11.4 Git 2.10.5 cvsimport: shell-quote variable used in backticks archimport: use safe_pipe_capture for user input shell: drop git-cvsserver support by default cvsserver: use safe_pipe_capture for `constant commands` as well cvsserver: use safe_pipe_capture instead of backticks cvsserver: move safe_pipe_capture() to the main package
| * Git 2.14.2v2.14.2Junio C Hamano2017-09-221-0/+11
| | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * Sync with 2.13.6Junio C Hamano2017-09-2210-56/+172
| |\ | | | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | * Git 2.13.6v2.13.6maint-2.13Junio C Hamano2017-09-223-2/+19
| | | | | | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | * Sync with 2.12.5Junio C Hamano2017-09-229-56/+155
| | |\ | | | | | | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | | * Git 2.12.5v2.12.5maint-2.12Junio C Hamano2017-09-223-2/+19
| | | | | | | | | | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | | * Sync with 2.11.4Junio C Hamano2017-09-228-56/+138
| | | |\ | | | | | | | | | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | | | * Git 2.11.4v2.11.4maint-2.11Junio C Hamano2017-09-223-2/+19
| | | | | | | | | | | | | | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | | | * Sync with 2.10.5Junio C Hamano2017-09-227-56/+121
| | | | |\ | | | | | | | | | | | | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | | | | * Git 2.10.5v2.10.5maint-2.10Junio C Hamano2017-09-223-2/+19
| | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | | | | * Merge branch 'jk/safe-pipe-capture' into maint-2.10Junio C Hamano2017-09-221-2/+2
| | | | | |\
| | | | | | * archimport: use safe_pipe_capture for user inputjk/safe-pipe-captureJeff King2017-09-121-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Refnames can contain shell metacharacters which need to be passed verbatim to sub-processes. Using safe_pipe_capture skips the shell entirely. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | | | | * | Merge branch 'jk/cvsimport-quoting' into maint-2.10Junio C Hamano2017-09-221-0/+1
| | | | | |\ \
| | | | | | * | cvsimport: shell-quote variable used in backticksJeff King2017-09-121-0/+1
| | | | | | |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We run `git rev-parse` though the shell, and quote its argument only with single-quotes. This prevents most metacharacters from being a problem, but misses the obvious case when $name itself has single-quotes in it. We can fix this by applying the usual shell-quoting formula. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | | | | * | Merge branch 'jc/cvsserver' into maint-2.10Junio C Hamano2017-09-221-40/+37
| | | | | |\ \
| | | | | | * | cvsserver: use safe_pipe_capture for `constant commands` as welljc/cvsserverJunio C Hamano2017-09-111-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is not strictly necessary, but it is a good code hygiene. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | | | | | * | cvsserver: use safe_pipe_capture instead of backticksjoernchen2017-09-111-11/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This makes the script pass arguments that are derived from end-user input in safer way when invoking subcommands. Reported-by: joernchen <joernchen@phenoelit.de> Signed-off-by: joernchen <joernchen@phenoelit.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | | | | | * | cvsserver: move safe_pipe_capture() to the main packageJunio C Hamano2017-09-111-25/+22
| | | | | | |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As a preparation for replacing `command` with a call to this function from outside GITCVS::updater package, move it to the main package. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| | | | | * | Merge branch 'jk/git-shell-drop-cvsserver' into maint-2.10Junio C Hamano2017-09-223-14/+64
| | | | | |\ \
| | | | | | * | shell: drop git-cvsserver support by defaultjk/git-shell-drop-cvsserverJeff King2017-09-123-14/+64
| | | | | | |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The git-cvsserver script is old and largely unmaintained these days. But git-shell allows untrusted users to run it out of the box, significantly increasing its attack surface. Let's drop it from git-shell's list of internal handlers so that it cannot be run by default. This is not backwards compatible. But given the age and development activity on CVS-related parts of Git, this is likely to impact very few users, while helping many more (i.e., anybody who runs git-shell and had no intention of supporting CVS). There's no configuration mechanism in git-shell for us to add a boolean and flip it to "off". But there is a mechanism for adding custom commands, and adding CVS support here is fairly trivial. Let's document it to give guidance to anybody who really is still running cvsserver. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | The ninth batch for 2.15Junio C Hamano2017-09-251-0/+67
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'ks/test-readme-phrasofix'Junio C Hamano2017-09-251-3/+3
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Doc updates. * ks/test-readme-phrasofix: t/README: fix typo and grammatically improve a sentence
| * | | | | | | t/README: fix typo and grammatically improve a sentenceks/test-readme-phrasofixKaartic Sivaraam2017-09-191-3/+3
| |/ / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'ow/rev-parse-is-shallow-repo'Junio C Hamano2017-09-253-0/+23
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git rev-parse" learned "--is-shallow-repository", that is to be used in a way similar to existing "--is-bare-repository" and friends. * ow/rev-parse-is-shallow-repo: rev-parse: rev-parse: add --is-shallow-repository
| * | | | | | | rev-parse: rev-parse: add --is-shallow-repositoryow/rev-parse-is-shallow-repoØystein Walle2017-09-193-0/+23
| |/ / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Running `git fetch --unshallow` on a repo that is not in fact shallow produces a fatal error message. Add a helper to rev-parse that scripters can use to determine whether a repo is shallow or not. Signed-off-by: Øystein Walle <oystwa@gmail.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'rj/test-ulimit-on-windows'Junio C Hamano2017-09-255-40/+42
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | On Cygwin, "ulimit -s" does not report failure but it does not work at all, which causes an unexpected success of some tests that expect failures under a limited stack situation. This has been fixed. * rj/test-ulimit-on-windows: t9010-*.sh: skip all tests if the PIPE prereq is missing test-lib: use more compact expression in PIPE prerequisite test-lib: don't use ulimit in test prerequisites on cygwin
| * | | | | | | t9010-*.sh: skip all tests if the PIPE prereq is missingrj/test-ulimit-on-windowsRamsay Jones2017-09-191-27/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Every test in this file, except one, is marked with the PIPE prereq. However, that lone test ('set up svn repo'), only performs some setup work and checks whether the following test should be executed (by setting an additional SVNREPO prerequisite). Since the following test also requires the PIPE prerequisite, performing the setup test, when the PIPE preequisite is missing, is simply wasted effort. Use the skip-all test facility to skip all tests when the PIPE prerequisite is missing. Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | test-lib: use more compact expression in PIPE prerequisiteRamsay Jones2017-09-191-8/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | test-lib: don't use ulimit in test prerequisites on cygwinRamsay Jones2017-09-154-5/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | On cygwin (and MinGW), the 'ulimit' built-in bash command does not have the desired effect of limiting the resources of new processes, at least for the stack and file descriptors. However, it always returns success and leads to several test prerequisites being erroneously set to true. Add a check for cygwin and MinGW to the prerequisite expressions, using a 'test_have_prereq !MINGW,!CYGWIN' clause, to guard against using ulimit. This affects the prerequisite expressions for the ULIMIT_STACK_SIZE, CMDLINE_LIMIT and ULIMIT_FILE_DESCRIPTORS prerequisites. Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | Merge branch 'jk/info-alternates-fix'Junio C Hamano2017-09-251-20/+11
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A regression fix for 2.11 that made the code to read the list of alternate object stores overrun the end of the string. * jk/info-alternates-fix: read_info_alternates: warn on non-trivial errors read_info_alternates: read contents into strbuf
| * | | | | | | | read_info_alternates: warn on non-trivial errorsJeff King2017-09-201-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we fail to open $GIT_DIR/info/alternates, we silently assume there are no alternates. This is the right thing to do for ENOENT, but not for other errors. A hard error is probably overkill here. If we fail to read an alternates file then either we'll complete our operation anyway, or we'll fail to find some needed object. Either way, a warning is good idea. And we already have a helper function to handle this pattern; let's just call warn_on_fopen_error(). Note that technically the errno from strbuf_read_file() might be from a read() error, not open(). But since read() would never return ENOENT or ENOTDIR, and since it produces a generic "unable to access" error, it's suitable for handling errors from either. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | Merge branch 'jk/info-alternates-fix-2.11' into jk/info-alternates-fixJunio C Hamano2017-09-201-20/+10
| |\ \ \ \ \ \ \ \ | | |_|/ / / / / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | * jk/info-alternates-fix-2.11: read_info_alternates: read contents into strbuf
| | * | | | | | | read_info_alternates: read contents into strbufjk/info-alternates-fix-2.11Jeff King2017-09-201-20/+10
| | | |_|_|/ / / | | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch fixes a regression in v2.11.1 where we might read past the end of an mmap'd buffer. It was introduced in cf3c635210. The link_alt_odb_entries() function has always taken a ptr/len pair as input. Until cf3c635210 (alternates: accept double-quoted paths, 2016-12-12), we made a copy of those bytes in a string. But after that commit, we switched to parsing the input left-to-right, and we ignore "len" totally, instead reading until we hit a NUL. This has mostly gone unnoticed for a few reasons: 1. All but one caller passes a NUL-terminated string, with "len" pointing to the NUL. 2. The remaining caller, read_info_alternates(), passes in an mmap'd file. Unless the file is an exact multiple of the page size, it will generally be followed by NUL padding to the end of the page, which just works. The easiest way to demonstrate the problem is to build with: make SANITIZE=address NO_MMAP=Nope test Any test which involves $GIT_DIR/info/alternates will fail, as the mmap emulation (correctly) does not add an extra NUL, and ASAN complains about reading past the end of the buffer. One solution would be to teach link_alt_odb_entries() to respect "len". But it's actually a bit tricky, since we depend on unquote_c_style() under the hood, and it has no ptr/len variant. We could also just make a NUL-terminated copy of the input bytes and operate on that. But since all but one caller already is passing a string, instead let's just fix that caller to provide NUL-terminated input in the first place, by swapping out mmap for strbuf_read_file(). There's no advantage to using mmap on the alternates file. It's not expected to be large (and anyway, we're copying its contents into an in-memory linked list). Nor is using git_open() buying us anything here, since we don't keep the descriptor open for a long period of time. Let's also drop the "len" parameter entirely from link_alt_odb_entries(), since it's completely ignored. That will avoid any new callers re-introducing a similar bug. Reported-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | Merge branch 'mh/for-each-string-list-item-empty-fix'Junio C Hamano2017-09-251-2/+4
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Code cmp.std.c nitpick. * mh/for-each-string-list-item-empty-fix: for_each_string_list_item: avoid undefined behavior for empty list
| * | | | | | | | for_each_string_list_item: avoid undefined behavior for empty listmh/for-each-string-list-item-empty-fixMichael Haggerty2017-09-201-2/+4
| |/ / / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If you pass a newly initialized or newly cleared `string_list` to `for_each_string_list_item()`, then the latter does for ( item = (list)->items; /* NULL */ item < (list)->items + (list)->nr; /* NULL + 0 */ ++item) Even though this probably works almost everywhere, it is undefined behavior, and it could plausibly cause highly-optimizing compilers to misbehave. C99 section 6.5.6 paragraph 8 explains: If both the pointer operand and the result point to elements of the same array object, or one past the last element of the array object, the evaluation shall not produce an overflow; otherwise, the behavior is undefined. and (6.3.2.3.3) a null pointer does not point to anything. Guard the loop with a NULL check to make the intent crystal clear to even the most pedantic compiler. A suitably clever compiler could let the NULL check only run in the first iteration, but regardless, this overhead is likely to be dwarfed by the work to be done on each item. This problem was noticed by Coverity. [jn: using a NULL check instead of a placeholder empty list; fleshed out the commit message based on mailing list discussion] Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | Merge branch 'tb/test-lint-echo-e'Junio C Hamano2017-09-251-1/+1
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The test linter has been taught that we do not like "echo -e". * tb/test-lint-echo-e: test-lint: echo -e (or -E) is not portable
| * | | | | | | | test-lint: echo -e (or -E) is not portabletb/test-lint-echo-eTorsten Bögershausen2017-09-211-1/+1
| |/ / / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some implementations of `echo` support the '-e' option to enable backslash interpretation of the following string. As an addition, they support '-E' to turn it off. However, none of these are portable, POSIX doesn't even mention them, and many implementations don't support them. A check for '-n' is already done in check-non-portable-shell.pl, extend it to cover '-n', '-e' or '-E'. Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | Merge branch 'jk/revision-remove-cmdline-pathspec'Junio C Hamano2017-09-253-32/+18
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Code clean-up that also plugs memory leaks. * jk/revision-remove-cmdline-pathspec: pathspec doc: parse_pathspec does not maintain references to args revision: replace "struct cmdline_pathspec" with argv_array