Commit 9ee6bcd398 (t5541-http-push: add test for URLs with trailing
slash, 2010-04-08) added a test that clones a URL with a trailing slash,
and confirms that we don't send a doubled slash (like "$url//info/refs")
to the server.
But this test makes no sense in t5541, which is about pushing. It should
have been added in t5551. Let's move it there.
But putting it at the end is tricky, since it checks the entire contents
of the Apache access log. We could get around this by clearing the log
before our test. But there's an even simpler solution: just make sure no
doubled slashes appear in the log (fortunately, "http://" does not
appear in the log itself).
As a bonus, this also lets us drop the check for the v0 protocol (which
is otherwise necessary since v2 makes multiple requests, and
check_access_log insists on exactly matching the number of requests,
even though we don't care about that here).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have a test which checks to see if a request to git-receive-pack was
made. Originally, it was checking the entire set of requests made in the
script so far, including clones, and thus it would break when run with
the v2 protocol (since that implies an extra request for fetches).
Since the previous commit, though, we are only checking the requests
made by a single push. And since there is no v2 push protocol, the test
now passes no matter what's in GIT_TEST_PROTOCOL_VERSION. We can just
run it all the time.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There's a test in t5541 that confirms that "git push" makes two requests
(a GET to /info/refs, and a POST to /git-receive-pack). However, it's a
noop unless GIT_TEST_PROTOCOL_VERSION is set to "0", due to 8a1b0978ab
(test: request GIT_TEST_PROTOCOL_VERSION=0 when appropriate,
2019-12-23).
This means that almost nobody runs it. And indeed, it has been broken
since b0c4adcdd7 (remote-curl: send Accept-Language header to server,
2022-07-11). But the fault is not in that commit, but in how brittle the
test is. It runs after several operations have been performed, which
means that it expects to see the complete set of requests made so far in
the script. Commit b0c4adcdd7 added a new test, which means that the
"used receive-pack service" test must be updated, too.
Let's fix this by making the test less brittle. We'll move it higher in
the script, right after the first push has completed. And we'll clear
the access log right before doing the push, so we'll see only the
requests from that command.
This is technically testing less, in that we won't check that all of
those other requests also correctly used smart http. But there's no
particular reason think that if the first one did, the others wouldn't.
After this patch, running:
GIT_TEST_PROTOCOL_VERSION=0 ./t5541-http-push-smart.sh
passes, whereas it did not before.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some passwords have an expiry date known at generation. This may be
years away for a personal access token or hours for an OAuth access
token.
When multiple credential helpers are configured, `credential fill` tries
each helper in turn until it has a username and password, returning
early. If Git authentication succeeds, `credential approve`
stores the successful credential in all helpers. If authentication
fails, `credential reject` erases matching credentials in all helpers.
Helpers implement corresponding operations: get, store, erase.
The credential protocol has no expiry attribute, so helpers cannot
store expiry information. Even if a helper returned an improvised
expiry attribute, git credential discards unrecognised attributes
between operations and between helpers.
This is a particular issue when a storage helper and a
credential-generating helper are configured together:
[credential]
helper = storage # eg. cache or osxkeychain
helper = generate # eg. oauth
`credential approve` stores the generated credential in both helpers
without expiry information. Later `credential fill` may return an
expired credential from storage. There is no workaround, no matter how
clever the second helper. The user sees authentication fail (a retry
will succeed).
Introduce a password expiry attribute. In `credential fill`, ignore
expired passwords and continue to query subsequent helpers.
In the example above, `credential fill` ignores the expired password
and a fresh credential is generated. If authentication succeeds,
`credential approve` replaces the expired password in storage.
If authentication fails, the expired credential is erased by
`credential reject`. It is unnecessary but harmless for storage
helpers to self prune expired credentials.
Add support for the new attribute to credential-cache.
Eventually, I hope to see support in other popular storage helpers.
Example usage in a credential-generating helper
https://github.com/hickford/git-credential-oauth/pull/16
Signed-off-by: M Hickford <mirth.hickford@gmail.com>
Reviewed-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Extend the run-hooks API to allow feeding data from the standard
input when running the hook script(s).
* ab/hook-api-with-stdin:
hook: support a --to-stdin=<path> option
sequencer: use the new hook API for the simpler "post-rewrite" call
hook API: support passing stdin to hooks, convert am's 'post-rewrite'
run-command: allow stdin for run_processes_parallel
run-command.c: remove dead assignment in while-loop
Leak fixes.
* ab/various-leak-fixes:
push: free_refs() the "local_refs" in set_refspecs()
push: refactor refspec_append_mapped() for subsequent leak-fix
receive-pack: release the linked "struct command *" list
grep API: plug memory leaks by freeing "header_list"
grep.c: refactor free_grep_patterns()
builtin/merge.c: free "&buf" on "Your local changes..." error
builtin/merge.c: use fixed strings, not "strbuf", fix leak
show-branch: free() allocated "head" before return
commit-graph: fix a parse_options_concat() leak
http-backend.c: fix cmd_main() memory leak, refactor reg{exec,free}()
http-backend.c: fix "dir" and "cmd_arg" leaks in cmd_main()
worktree: fix a trivial leak in prune_worktrees()
repack: fix leaks on error with "goto cleanup"
name-rev: don't xstrdup() an already dup'd string
various: add missing clear_pathspec(), fix leaks
clone: use free() instead of UNLEAK()
commit-graph: use free_commit_graph() instead of UNLEAK()
bundle.c: don't leak the "args" in the "struct child_process"
tests: mark tests as passing with SANITIZE=leak
As mentioned in 'git-range-diff.txt': "`git range-diff` also accepts the
regular diff options (see linkgit:git-diff[1])...", but '--abbrev' is not
in the "regular" scope.
In Git, the "abbrev" of an object may not be a fixed value in different
repositories, depending on the needs of the them(Linus mentioned in
e6c587c7 in 2016: "the Linux kernel project needs 11 to 12 hexdigits"
at that time ), that's why a user may want to display abbrev according
to a specified length.
Although a similar effect can be achieved through configuration (like:
git -c core.abbrev=<abbrev>), but based on ease of use (many users may not
know that the -c option can be specified) and the description in existing
document, supporting users to directly use '--abbrev', could be a good way.
Signed-off-by: Teng Long <dyroneteng@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
prior to 51243f9 (run-command API: don't fall back on online_cpus(),
2022-10-12) `git fetch --multiple --jobs=0` would choose some default amount
of jobs, similar to `git -c fetch.parallel=0 fetch --multiple`. While our
documentation only ever promised that `fetch.parallel` would fall back to a
"sensible default", it makes sense to do the same for `--jobs`. So fall back
to online_cpus() and not BUG() out.
This fixes https://github.com/git-for-windows/git/issues/4302
Reported-by: Drew Noakes <drnoakes@microsoft.com>
Signed-off-by: Matthias Aßhauer <mha1993@live.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
trace_repo_setup() of trace.c is called with the argument 'prefix' from
only one location, run_builtin of git.c, which sets 'prefix' to the return
value of setup_git_directory() or setup_git_directory_gently() (a wrapper
of the former).
Now that "prefix" is in startup_info there is no need for the parameter
of trace_repo_setup() because setup_git_directory() sets "startup_info->prefix"
to the same value it returns. It would be less confusing to use "prefix"
from startup_info instead of passing it as an argument.
Signed-off-by: Idriss Fekir <mcsm224@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
the `cd` followed the old style which wasn't consistent with the rest of
the test suite, so this commit makes it consistent with the current
style of the test suite for `cd` in subshell.
Signed-off-by: Ashutosh Pandey <ashutosh.pandeyhlr007@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It can be useful to specify diff algorithms per file type. For example,
one may want to use the minimal diff algorithm for .json files, another
for .c files, etc.
The diff machinery already checks attributes for a diff driver. Teach
the diff driver parser a new type "algorithm" to look for in the
config, which will be used if a driver has been specified through the
attributes.
Enforce precedence of the diff algorithm by favoring the command line
option, then looking at the driver attributes & config combination, then
finally the diff.algorithm config.
To enforce precedence order, use a new `ignore_driver_algorithm` member
during options parsing to indicate the diff algorithm was set via command
line args.
Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A subsequent commit will need the ability to tell if the diff algorithm
was set through the command line through setting a new member of
diff_options. While this logic can be added to the
diff_opt_diff_algorithm() callback, the `--minimal` and `--histogram`
options are handled via OPT_BIT without a callback.
Remedy this by consolidating the options parsing logic for --minimal and
--histogram into one callback. This way we can modify `diff_options` in
that function.
As an additional refactor, the logic that sets the diff algorithm in
diff_opt_diff_algorithm() can be refactored into a helper that will
allow multiple callsites to set the diff algorithm.
Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Check that the argument to the "label" and "update-ref" commands is a
valid refname when the todo list is parsed rather than waiting until the
command is executed. This means that the user can deal with any errors
at the beginning of the rebase rather than having it stop halfway
through due to a typo in a label name. The "update-ref" command is
changed to reject single level refs as it is all to easy to type
"update-ref branch" which is incorrect rather than "update-ref
refs/heads/branch"
Note that it is not straight forward to check the arguments to "reset"
and "merge" commands as they may be any revision, not just a refname and
we do not have an equivalent of check_refname_format() for revisions.
Helped-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Acked-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 647982bb71 (delta-islands: free island_marks and bitmaps, 2023-02-03)
we have introduced logic to free `island_marks` in order to reduce heap
memory usage in git-pack-objects(1). This commit is causing segfaults in
the case where this Git command does not load delta islands at all, e.g.
when reading object IDs from standard input. One such crash can be hit
when using repacking multi-pack-indices with delta islands enabled.
The root cause of this bug is that we unconditionally dereference the
`island_marks` variable even in the case where it is a `NULL` pointer,
which is fixed by making it conditional. Note that we still leave the
logic in place to set the pointer to `-1` to detect use-after-free bugs
even when there are no loaded island marks at all.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Acked-by: Eric Wong <e@80x24.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Allow users to specify the modification time of archive entries. The
new option --mtime uses approxidate() to parse a time specification and
overrides the default of using the current time for trees and the commit
time for tags and commits. It can be used to create a reproducible
archive for a tree, or to use a specific mtime without creating a commit
with GIT_COMMITTER_DATE set.
This implementation doesn't support the negated form of the new option,
i.e. --no-mtime is not accepted. It is not possible to have no mtime at
all. We could use the Unix epoch or revert to the default behavior, but
since negation is not necessary for the intended use it's left undecided
for now.
Requested-by: Raul E Rangel <rrangel@chromium.org>
Suggested-by: demerphq <demerphq@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a lower precedence configuration file (e.g. /etc/gitconfig)
defines format.attach in any way, there was no way to disable it in
a more specific configuration file (e.g. $HOME/.gitconfig).
Change the behaviour of setting it to an empty string. It used to
mean that the result is still a multipart message with only dashes
used as a multi-part separator, but now it resets the setting to
the default (which would be to give an inline patch, unless other
command line options are in effect).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The symlink setup in t0066 makes several directories with links, dir4
through dir6. But ever since dir5 was introduced in fa1da7d2ee
(dir-iterator: add flags parameter to dir_iterator_begin, 2019-07-10),
it has never actually been used. It was left over from an earlier
iteration of the patch which tried to handle recursive symlinks
specially, as seen in:
https://lore.kernel.org/git/20190502144829.4394-7-matheus.bernardino@usp.br/
It's not hurting any of the existing tests to be there, but the extra
setup is confusing to anybody trying to read and understand the tests.
Let's drop the extra directory, and we'll rename "dir6" to "dir5" so
nobody wonders whether the gap in naming is important.
Helped-by: Matheus Tavares Bernardino <matheus.tavb@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We do not test our http proxy functionality at all in the test suite, so
this is a pretty big blind spot. Let's at least add a basic check that
we can go through an authenticating proxy to perform a clone.
A few notes on the implementation:
- I'm using a single apache instance to proxy to itself. This seems to
work fine in practice, and we can check with a test that this rather
unusual setup is doing what we expect.
- I've put the proxy tests into their own script, and it's the only
one which loads the apache proxy config. If any platform can't
handle this (e.g., doesn't have the right modules), the start_httpd
step should fail and gracefully skip the rest of the script (but all
the other http tests in existing scripts will continue to run).
- I used a separate passwd file to make sure we don't ever get
confused between proxy and regular auth credentials. It's using the
antiquated crypt() format. This is a terrible choice security-wise
in the modern age, but it's what our existing passwd file uses, and
should be portable. It would probably be reasonable to switch both
of these to bcrypt, but we can do that in a separate patch.
- On the client side, we test two situations with credentials: when
they are present in the url, and when the username is present but we
prompt for the password. I think we should be able to handle the
case that _neither_ is present, but an HTTP 407 causes us to prompt
for them. However, this doesn't seem to work. That's either a bug,
or at the very least an opportunity for a feature, but I punted on
it for now. The point of this patch is just getting basic coverage,
and we can explore possible deficiencies later.
- this doesn't work with LIB_HTTPD_SSL. This probably would be
valuable to have, as https over an http proxy is totally different
(it uses CONNECT to tunnel the session). But adding in
mod_proxy_connect and some basic config didn't seem to work for me,
so I punted for now. Much of the rest of the test suite does not
currently work with LIB_HTTPD_SSL either, so we shouldn't be making
anything much worse here.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `FOLLOW_SYMLINKS` flag was added to the dir-iterator API in
fa1da7d2ee (dir-iterator: add flags parameter to dir_iterator_begin,
2019-07-10) in order to follow symbolic links while traversing through a
directory.
`FOLLOW_SYMLINKS` gained its first caller in ff7ccc8c9a (clone: use
dir-iterator to avoid explicit dir traversal, 2019-07-10), but it was
subsequently removed in 6f054f9fb3 (builtin/clone.c: disallow `--local`
clones with symlinks, 2022-07-28).
Since then, we've held on to the code for `DIR_ITERATOR_FOLLOW_SYMLINKS`
in the name of making minimally invasive changes during a security
embargo.
In fact, we even changed the dir-iterator API in bffc762f87
(dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS,
2023-01-24) without having any non-test callers of that flag.
Now that we're past those security embargo(s), let's finalize our
cleanup of the `DIR_ITERATOR_FOLLOW_SYMLINKS` code and remove its
implementation since there are no remaining callers.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This test helper feeds 256kB of data at once to a single invocation
of the write(2) system call, which may be too much for some
platforms.
Call our xwrite() wrapper that knows to honor MAX_IO_SIZE limit and
cope with short writes due to EINTR instead, and die a bit more
loudly by calling die_errno() when xwrite() indicates an error.
Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The documentation mistakenly said that the default format was
similar to RFC 2822 format and tried to specify it by enumerating
differences, which had two problems:
* There are some more differences from the 2822 format that are not
mentioned; worse yet
* The default format is not modeled after RFC 2822 format at all.
As can be seen in f80cd783 (date.c: add "show_date()" function.,
2005-05-06), it is a derivative of ctime(3) format.
Stop saying that it is similar to RFC 2822, and rewrite the
description to explain the format without requiring the reader to
know any other format.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Finally retire the scripted "git add -p/-i" implementation and have
everybody use the one reimplemented in C.
* ab/retire-scripted-add-p:
docs & comments: replace mentions of "git-add--interactive.perl"
add API: remove run_add_interactive() wrapper function
add: remove "add.interactive.useBuiltin" & Perl "git add--interactive"
Userdiff regexp update for Java language.
* ar/userdiff-java-update:
userdiff: support Java sealed classes
userdiff: support Java record types
userdiff: support Java type parameters
In-tree .gitattributes update to match the way we recommend our
users to mark a file as text.
* po/attributes-text:
.gitattributes: include `text` attribute for eol attributes
Plug leaks in sequencer subsystem and its users.
* ab/sequencer-unleak:
commit.c: free() revs.commit in get_fork_point()
builtin/rebase.c: free() "options.strategy_opts"
sequencer.c: always free() the "msgbuf" in do_pick_commit()
builtin/rebase.c: fix "options.onto_name" leak
builtin/revert.c: move free-ing of "revs" to replay_opts_release()
sequencer API users: fix get_replay_opts() leaks
sequencer.c: split up sequencer_remove_state()
rebase: use "cleanup" pattern in do_interactive_rebase()
The bundle-URI subsystem adds support for creation-token heuristics
to help incremental fetches.
* ds/bundle-uri-5:
bundle-uri: test missing bundles with heuristic
bundle-uri: store fetch.bundleCreationToken
fetch: fetch from an external bundle URI
bundle-uri: drop bundle.flag from design doc
clone: set fetch.bundleURI if appropriate
bundle-uri: download in creationToken order
bundle-uri: parse bundle.<id>.creationToken values
bundle-uri: parse bundle.heuristic=creationToken
t5558: add tests for creationToken heuristic
bundle: verify using check_connected()
bundle: test unbundling with incomplete history
In an environment where dynamically generated code is prohibited to
run (e.g. SELinux), failure to JIT pcre patterns is expected. Fall
back to interpreted execution in such a case.
* cb/grep-fallback-failing-jit:
grep: fall back to interpreter if JIT memory allocation fails
There are few things more frustrating when signing a commit fails than
reading a terse "error: gpg failed to sign the data" message followed by
the unsurprising "fatal: failed to write commit object" message.
In many cases where signing a commit or tag fails, `gpg` actually said
something helpful, on its stderr, and Git even consumed that, but then
keeps mum about it.
Teach Git to stop withholding that rather important information.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This test case not only increases test coverage in setups without
working gpg, but also prepares for verifying that the error message of
`gpg.program` is shown upon failure.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
To shorten a fully qualified ref (e.g., taking "refs/heads/foo" to just
"foo"), we munge the usual lookup rules ("refs/heads/%.*s", etc) to drop
the ".*" modifier (so "refs/heads/%s"), and then use sscanf() to match
that against the refname, pulling the "%s" content into a separate
buffer.
This has a few downsides:
- sscanf("%s") reportedly misbehaves on macOS with some input and
locale combinations, returning a partial or garbled string. See
this thread:
https://lore.kernel.org/git/CAGF3oAcCi+fG12j-1U0hcrWwkF5K_9WhOi6ZPHBzUUzfkrZDxA@mail.gmail.com/
- scanf's matching of "%s" is greedy. So the "refs/remotes/%s/HEAD"
rule would never pull "origin" out of "refs/remotes/origin/HEAD".
Instead it always produced "origin/HEAD", which is redundant with
the "refs/remotes/%s" rule.
- scanf in general is an error-prone interface. For example, scanning
for "%s" will copy bytes into a destination string, which must have
been correctly sized ahead of time to avoid a buffer overflow. In
this case, the code is OK (the buffer is pessimistically sized to
match the original string, which should give us a maximum). But in
general, we do not want to encourage people to use scanf at all.
So instead, let's note that our lookup rules are not arbitrary format
strings, but all contain exactly one "%.*s" placeholder. We already rely
on this, both for lookup (we feed the lookup format along with exactly
one int/ptr combo to snprintf, etc) and for shortening (we munge "%.*s"
to "%s", and then insist that sscanf() finds exactly one result).
We can parse this manually by just matching the bytes that occur before
and after the "%.*s" placeholder. While we have a few extra lines of
parsing code, the result is arguably simpler, as can skip the
preprocessing step and its tricky memory management entirely.
The in-code comments should explain the parsing strategy, but there's
one subtle change here. The original code allocated a single buffer, and
then overwrote it in each loop iteration, since that's the only option
sscanf() gives us. But our parser can actually return a ptr/len combo
for the matched string, which is all we need (since we just feed it back
to the lookup rules with "%.*s"), and then copy it only when returning
to the caller.
There are a few new tests here, all using symbolic-ref (the code can be
triggered in many ways, but symrefs are convenient in that we don't need
to create a real ref, which avoids any complications from the filesystem
munging the name):
- the first covers the real-world case which misbehaved on macOS.
Setting LC_ALL is required to trigger the problem there (since
otherwise our tests use LC_ALL=C), and hopefully is at worst simply
ignored on other systems (and doesn't cause libc to complain, etc,
on systems without that locale).
- the second covers the "origin/HEAD" case as discussed above, which
is now fixed
- the remainder are for "weird" cases that work both before and after
this patch, but would be easy to get wrong with off-by-one problems
in the parsing (and came out of discussions and earlier iterations
of the patch that did get them wrong).
- absent here are tests of boring, expected-to-work cases like
"refs/heads/foo", etc. Those are covered all over the test suite
both explicitly (for-each-ref's refname:short) and implicitly (in
the output of git-status, etc).
Reported-by: 孟子易 <mengziyi540841@gmail.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The ref_rev_parse_rules[] array is terminated with a NULL entry, and we
count it and store the result in the local nr_rules variable. But we
don't need to do so; since the array is a constant, we can compute its
size directly. The original code probably didn't do that because it was
written as part of for-each-ref, and saw the array only as a pointer. It
was migrated in 7c2b3029df (make get_short_ref a public function,
2009-04-07) and could have been updated then, but that subtlety was not
noticed.
We even have a constant that represents this value already, courtesy of
60650a48c0 (remote: make refspec follow the same disambiguation rule as
local refs, 2018-08-01), though again, nobody noticed at the time that
it could be used here, too.
The current count-up isn't a big deal, as we need to preprocess that
array anyway. But it will become more cumbersome as we refactor the
shortening code. So let's get rid of it and just use the constant
everywhere.
Note that there are two things here that aren't just simple text
replacements:
1. We also use nr_rules to see if a previous call has initialized the
static pre-processing variables. We can just use the scanf_fmts
pointer to do the same thing, as it is non-NULL only after we've
done that initialization.
2. If nr_rules is zero after we've counted it up, we bail from the
function. This code is unreachable, though, as the set of rules is
hard-coded and non-empty. And that becomes even more apparent now
that we are using the constant. So we can drop this conditional
completely (and ironically, the code would have the same output if
it _did_ trigger, as we'd simply skip the loop entirely and return
the whole refname).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We parse the shortened name "foo" out of the full refname
"refs/heads/foo", and then assign the result of strlen(short_name) to an
int, which may truncate or wrap to negative.
In practice, this should never happen, as it requires a 2GB refname. And
even somebody trying to do something malicious should at worst end up
with a confused answer (we use the size only to feed back as a
placeholder length to strbuf_addf() to see if there are any collisions
in the lookup rules).
And it may even be impossible to trigger this, as we parse the string
with sscanf(), and stdio formatting functions are not known for handling
large strings well. I didn't test, but I wouldn't be surprised if
sscanf() on many platforms simply reports no match here.
But even if it is not a problem in practice so far, it is worth fixing
for two reasons:
1. We'll shortly be replacing the sscanf() call with a real parser
which will handle arbitrary-sized strings.
2. Assigning strlen() to an int is an anti-pattern that requires
people to look twice when auditing for real overflow problems.
So we'll make this a size_t. Unfortunately we still have to cast to int
eventually for the strbuf_addf() call, but at least we can localize the
cast there, and check that it will be valid. I used our new cast helper
here, which will just bail completely. That should be OK, as anybody
with a 2GB refname is up to no good, but if we really wanted to, we
could detect it manually and just refuse to shorten the refname.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
