aboutsummaryrefslogtreecommitdiffstats
path: root/daemon.c
AgeCommit message (Collapse)AuthorFilesLines
2025-08-04Merge branch 'ps/config-wo-the-repository'Junio C Hamano1-1/+1
The config API had a set of convenience wrapper functions that implicitly use the_repository instance; they have been removed and inlined at the calling sites. * ps/config-wo-the-repository: (21 commits) config: fix sign comparison warnings config: move Git config parsing into "environment.c" config: remove unused `the_repository` wrappers config: drop `git_config_set_multivar()` wrapper config: drop `git_config_get_multivar_gently()` wrapper config: drop `git_config_set_multivar_in_file_gently()` wrapper config: drop `git_config_set_in_file_gently()` wrapper config: drop `git_config_set()` wrapper config: drop `git_config_set_gently()` wrapper config: drop `git_config_set_in_file()` wrapper config: drop `git_config_get_bool()` wrapper config: drop `git_config_get_ulong()` wrapper config: drop `git_config_get_int()` wrapper config: drop `git_config_get_string()` wrapper config: drop `git_config_get_string()` wrapper config: drop `git_config_get_string_multi()` wrapper config: drop `git_config_get_value()` wrapper config: drop `git_config_get_value()` wrapper config: drop `git_config_get()` wrapper config: drop `git_config_clear()` wrapper ...
2025-07-23config: drop `git_config_get_bool()` wrapperPatrick Steinhardt1-1/+1
In 036876a1067 (config: hide functions using `the_repository` by default, 2024-08-13) we have moved around a bunch of functions in the config subsystem that depend on `the_repository`. Those function have been converted into mere wrappers around their equivalent function that takes in a repository as parameter, and the intent was that we'll eventually remove those wrappers to make the dependency on the global repository variable explicit at the callsite. Follow through with that intent and remove `git_config_get_bool()`. All callsites are adjusted so that they use `repo_config_get_bool(the_repository, ...)` instead. While some callsites might already have a repository available, this mechanical conversion is the exact same as the current situation and thus cannot cause any regression. Those sites should eventually be cleaned up in a later patch series. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-07-21Merge branch 'cb/daemon-reap-children'Junio C Hamano1-5/+7
Futz with SIGCHLD handling in "git daemon". * cb/daemon-reap-children: daemon: use sigaction() to install child_handler() compat/mingw: allow sigaction(SIGCHLD)
2025-07-10daemon: use sigaction() to install child_handler()Carlo Marcelo Arenas Belón1-5/+7
Replace signal() with an equivalent invocation of sigaction(), but make sure to NOT set SA_RESTART so the original code that expects to be interrupted when children complete still works as designed. This change has the added benefit of using BSD signal semantics reliably and therefore not needing the rearming call in the signal handler. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-07-07Merge branch 'cb/daemon-retry-interrupted-accept'Junio C Hamano1-2/+10
When "git daemon" sees a signal while attempting to accept() a new client, instead of retrying, it skipped it by mistake, which has been corrected. * cb/daemon-retry-interrupted-accept: daemon: correctly handle soft accept() errors in service_loop
2025-06-30daemon: correctly handle soft accept() errors in service_loopCarlo Marcelo Arenas Belón1-2/+10
Since df076bdbcc ([PATCH] GIT: Listen on IPv6 as well, if available., 2005-07-23), the original error checking was included in an inner loop unchanged, where its effect was different. Instead of retrying, after a EINTR during accept() in the listening socket, it will advance to the next one and try with that instead, leaving the client waiting for another round. Make sure to retry with the same listener socket that failed originally. To avoid an unlikely busy loop, fallback to the old behaviour after a couple of attempts. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Acked-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-26daemon: remove unnecesary restriction for listener fdCarlo Marcelo Arenas Belón1-5/+0
Since df076bdbcc ([PATCH] GIT: Listen on IPv6 as well, if available., 2005-07-23), any file descriptor assigned to a listening socket was validated to be within the range to be used in an FDSET later. 6573faff34 (NO_IPV6 support for git daemon, 2005-09-28), moves to use poll() instead of select(), that doesn't have that restriction, so remove the original check. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Acked-by: Phillip Wood <phillip.wood123@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-23Merge branch 'ps/build-sign-compare'Junio C Hamano1-18/+13
Start working to make the codebase buildable with -Wsign-compare. * ps/build-sign-compare: t/helper: don't depend on implicit wraparound scalar: address -Wsign-compare warnings builtin/patch-id: fix type of `get_one_patchid()` builtin/blame: fix type of `length` variable when emitting object ID gpg-interface: address -Wsign-comparison warnings daemon: fix type of `max_connections` daemon: fix loops that have mismatching integer types global: trivial conversions to fix `-Wsign-compare` warnings pkt-line: fix -Wsign-compare warning on 32 bit platform csum-file: fix -Wsign-compare warning on 32-bit platform diff.h: fix index used to loop through unsigned integer config.mak.dev: drop `-Wno-sign-compare` global: mark code units that generate warnings with `-Wsign-compare` compat/win32: fix -Wsign-compare warning in "wWinMain()" compat/regex: explicitly ignore "-Wsign-compare" warnings git-compat-util: introduce macros to disable "-Wsign-compare" warnings
2024-12-10Merge branch 'bc/allow-upload-pack-from-other-people'Junio C Hamano1-2/+4
Loosen overly strict ownership check introduced in the recent past, to keep the promise "cloning a suspicious repository is a safe first step to inspect it". * bc/allow-upload-pack-from-other-people: Allow cloning from repositories owned by another user
2024-12-06daemon: fix type of `max_connections`Patrick Steinhardt1-6/+5
The `max_connections` type tracks how many children git-daemon(1) would spawn at the same time. This value can be controlled via a command line switch: if given a positive value we'll set that up as the limit. But when given either zero or a negative value we don't enforce any limit at all. But even when being passed a negative value we won't actually store it, but normalize it to 0. Still, the variable used to store the config is using a signed integer, which causes warnings when comparing the number of accepted connections (`max_connections`) with the number of current connections being handled (`live_children`). Adapt the type of `max_connections` such that the types of both variables match. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-06daemon: fix loops that have mismatching integer typesPatrick Steinhardt1-13/+8
We have several loops in "daemon.c" that use a signed integer to loop through a `size_t`. Adapt them to instead use a `size_t` as counter value. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-06global: mark code units that generate warnings with `-Wsign-compare`Patrick Steinhardt1-0/+1
Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-11-15Allow cloning from repositories owned by another userbrian m. carlson1-2/+4
Historically, Git has allowed users to clone from an untrusted repository, and we have documented that this is safe to do so: `upload-pack` tries to avoid any dangerous configuration options or hooks from the repository it's serving, making it safe to clone an untrusted directory and run commands on the resulting clone. However, this was broken by f4aa8c8bb1 ("fetch/clone: detect dubious ownership of local repositories", 2024-04-10) in an attempt to make things more secure. That change resulted in a variety of problems when cloning locally and over SSH, but it did not change the stated security boundary. Because the security boundary has not changed, it is safe to adjust part of the code that patch introduced. To do that and restore the previous functionality, adjust enter_repo to take two flags instead of one. The two bits are - ENTER_REPO_STRICT: callers that require exact paths (as opposed to allowing known suffixes like ".git", ".git/.git" to be omitted) can set this bit. Corresponds to the "strict" parameter that the flags word replaces. - ENTER_REPO_ANY_OWNER_OK: callers that are willing to run without ownership check can set this bit. The former is --strict-paths option of "git daemon". The latter is set only by upload-pack, which honors the claimed security boundary. Note that local clones across ownership boundaries require --no-local so that upload-pack is used. Document this fact in the manual page and provide an example. This patch was based on one written by Junio C Hamano. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-10-24daemon: replace atoi() with strtoul_ui() and strtol_i()Usman Akinyemi1-4/+8
Replace atoi() with strtoul_ui() for --timeout and --init-timeout (non-negative integers) and with strtol_i() for --max-connections (signed integers). This improves error handling and input validation by detecting invalid values and providing clear error messages. Update tests to ensure these arguments are properly validated. Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-08-26Merge branch 'jk/mark-unused-parameters'Junio C Hamano1-3/+3
Mark unused parameters as UNUSED to squelch -Wunused warnings. * jk/mark-unused-parameters: t-hashmap: stop calling setup() for t_intern() test scalar: mark unused parameters in dummy function daemon: mark unused parameters in non-posix fallbacks setup: mark unused parameter in config callback test-mergesort: mark unused parameters in trivial callback t-hashmap: mark unused parameters in callback function reftable: mark unused parameters in virtual functions reftable: drop obsolete test function declarations reftable: ignore unused argc/argv in test functions unit-tests: ignore unused argc/argv t/helper: mark more unused argv/argc arguments oss-fuzz: mark unused argv/argc argument refs: mark unused parameters in do_for_each_reflog_helper() refs: mark unused parameters in ref_store fsck callbacks update-ref: mark more unused parameters in parser callbacks imap-send: mark unused parameter in ssl_socket_connect() fallback
2024-08-17daemon: mark unused parameters in non-posix fallbacksJeff King1-3/+3
If NO_POSIX_GOODIES is set, we compile fallback versions of a few functions. These don't do anything, so their parameters are unused, but we must keep them to match the ones on the other side of the #ifdef. Mark them to quiet -Wunused-parameter. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-13global: prepare for hiding away repo-less config functionsPatrick Steinhardt1-0/+2
We're about to hide config functions that implicitly depend on `the_repository` behind the `USE_THE_REPOSITORY_VARIABLE` macro. This will uncover a bunch of dependents that transitively relied on the global variable, but didn't define the macro yet. Adapt them such that we define the macro to prepare for this change. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-05daemon: free listen_addr before returningJeff King1-16/+21
We build up a string list of listen addresses from the command-line arguments, but never free it. This causes t5811 to complain of a leak (though curiously it seems to do so only when compiled with gcc, not with clang). To handle this correctly, we have to do a little refactoring: - there are two exit points from the main function, depending on whether we are entering the main loop or serving a single client (since rather than a traditional fork model, we re-exec ourselves with the extra "--serve" argument to accommodate Windows). We don't need --listen at all in the --serve case, of course, but it is passed along by the parent daemon, which simply copies all of the command-line options it got. - we just "return serve()" to run the main loop, giving us no chance to do any cleanup So let's use a "ret" variable to store the return code, and give ourselves a single exit point at the end. That gives us one place to do cleanup. Note that this code also uses the "use a no-dup string-list, but allocate strings we add to it" trick, meaning string_list_clear() will not realize it should free them. We can fix this by switching to a "dup" string-list, but using the "append_nodup" function to add to it (this is preferable to tweaking the strdup_strings flag before clearing, as it puts all the subtle memory-ownership code together). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-17Merge branch 'cw/compat-util-header-cleanup'Junio C Hamano1-2/+0
Further shuffling of declarations across header files to streamline file dependencies. * cw/compat-util-header-cleanup: git-compat-util: move alloc macros to git-compat-util.h treewide: remove unnecessary includes for wrapper.h kwset: move translation table from ctype sane-ctype.h: create header for sane-ctype macros git-compat-util: move wrapper.c funcs to its header git-compat-util: move strbuf.c funcs to its header
2023-07-05git-compat-util: move alloc macros to git-compat-util.hCalvin Wan1-1/+0
alloc_nr, ALLOC_GROW, and ALLOC_GROW_BY are commonly used macros for dynamic array allocation. Moving these macros to git-compat-util.h with the other alloc macros focuses alloc.[ch] to allocation for Git objects and additionally allows us to remove inclusions to alloc.h from files that solely used the above macros. Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-05treewide: remove unnecessary includes for wrapper.hCalvin Wan1-1/+0
Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-06-18replace strbuf_expand() with strbuf_expand_step()René Scharfe1-42/+19
Avoid the overhead of passing context to a callback function of strbuf_expand() by using strbuf_expand_step() in a loop instead. It requires explicit handling of %% and unrecognized placeholders, but is simpler, more direct and avoids void pointers. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-24treewide: remove cache.h inclusion due to previous changesElijah Newren1-1/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-24hash-ll.h: split out of hash.h to remove dependency on repository.hElijah Newren1-0/+1
hash.h depends upon and includes repository.h, due to the definition and use of the_hash_algo (defined as the_repository->hash_algo). However, most headers trying to include hash.h are only interested in the layout of the structs like object_id. Move the parts of hash.h that do not depend upon repository.h into a new file hash-ll.h (the "low level" parts of hash.h), and adjust other files to use this new header where the convenience inline functions aren't needed. This allows hash.h and object.h to be fairly small, minimal headers. It also exposes a lot of hidden dependencies on both path.h (which was brought in by repository.h) and repository.h (which was previously implicitly brought in by object.h), so also adjust other files to be more explicit about what they depend upon. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-04-18protocol.h: move definition of DEFAULT_GIT_PORT from cache.hElijah Newren1-0/+1
Michael J Gruber noticed that connection via the git:// protocol no longer worked after a recent header clean-up. This was caused by funny interaction of few gotchas. First, a necessary definition #define DEFAULT_GIT_PORT 9418 was made invisible to a place where const char *port = STR(DEFAULT_GIT_PORT); was expecting to turn the integer into "9418" with a clever STR() macro, and ended up stringifying it to const char *port = "DEFAULT_GIT_PORT"; without giving any chance to compilers to notice such a mistake. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-21setup.h: move declarations for setup.c functions from cache.hElijah Newren1-0/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-21environment.h: move declarations for environment.c functions from cache.hElijah Newren1-0/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-21wrapper.h: move declarations for wrapper.c functions from cache.hElijah Newren1-0/+1
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-21abspath.h: move absolute path functions from cache.hElijah Newren1-0/+1
This is another step towards letting us remove the include of cache.h in strbuf.c. It does mean that we also need to add includes of abspath.h in a number of C files. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-03-17Merge branch 'jk/unused-post-2.39-part2'Junio C Hamano1-1/+1
More work towards -Wunused. * jk/unused-post-2.39-part2: (21 commits) help: mark unused parameter in git_unknown_cmd_config() run_processes_parallel: mark unused callback parameters userformat_want_item(): mark unused parameter for_each_commit_graft(): mark unused callback parameter rewrite_parents(): mark unused callback parameter fetch-pack: mark unused parameter in callback function notes: mark unused callback parameters prio-queue: mark unused parameters in comparison functions for_each_object: mark unused callback parameters list-objects: mark unused callback parameters mark unused parameters in signal handlers run-command: mark error routine parameters as unused mark "pointless" data pointers in callbacks ref-filter: mark unused callback parameters http-backend: mark unused parameters in virtual functions http-backend: mark argc/argv unused object-name: mark unused parameters in disambiguate callbacks serve: mark unused parameters in virtual functions serve: use repository pointer to get config ls-refs: drop config caching ...
2023-02-24mark unused parameters in signal handlersJeff King1-1/+1
Signal handlers receive their signal number as a parameter, but many don't care what it is (because they only handle one signal, or because their action is the same regardless of the signal). Mark such parameters to silence -Wunused-parameter. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-02-23alloc.h: move ALLOC_GROW() functions from cache.hElijah Newren1-0/+1
This allows us to replace includes of cache.h with includes of the much smaller alloc.h in many places. It does mean that we also need to add includes of alloc.h in a number of C files. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-19daemon: clarify directory argumentsDerrick Stolee1-4/+4
The undecorated arguments to the 'git-daemon' command provide a list of directories. When at least one directory is specified, then 'git-daemon' only serves requests that are within that directory list. The boolean '--strict-paths' option makes the list more explicit in that subdirectories are no longer included. The existing documentation and error messages around this directory list refer to it and its behavior as a "whitelist". The word "whitelist" has cultural implications that are not inclusive. Thankfully, it is not difficult to reword and avoid its use. In the process, we can define the purpose of this directory list directly. In Documentation/git-daemon.txt, rewrite the OPTIONS section around the '<directory>' option. Add additional clarity to the other options that refer to these directories. Some error messages can also be improved in daemon.c. The '--strict-paths' option requires '<directory>' arguments, so refer to that section of the documentation directly. A logerror() call points out that a requested directory is not in the specified directory list. We can use "list" here without any loss of information. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-06-10Merge branch 'ab/env-array'Junio C Hamano1-7/+7
Rename .env_array member to .env in the child_process structure. * ab/env-array: run-command API users: use "env" not "env_array" in comments & names run-command API: rename "env_array" to "env"
2022-06-02run-command API: rename "env_array" to "env"Ævar Arnfjörð Bjarmason1-7/+7
Start following-up on the rename mentioned in c7c4bdeccf3 (run-command API: remove "env" member, always use "env_array", 2021-11-25) of "env_array" to "env". The "env_array" name was picked in 19a583dc39e (run-command: add env_array, an optional argv_array for env, 2014-10-19) because "env" was taken. Let's not forever keep the oddity of "*_array" for this "struct strvec", but not for its "args" sibling. This commit is almost entirely made with a coccinelle rule[1]. The only manual change here is in run-command.h to rename the struct member itself and to change "env_array" to "env" in the CHILD_PROCESS_INIT initializer. The rest of this is all a result of applying [1]: * make contrib/coccinelle/run_command.cocci.patch * patch -p1 <contrib/coccinelle/run_command.cocci.patch * git add -u 1. cat contrib/coccinelle/run_command.pending.cocci @@ struct child_process E; @@ - E.env_array + E.env @@ struct child_process *E; @@ - E->env_array + E->env I've avoided changing any comments and derived variable names here, that will all be done in the next commit. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-05-02Merge branch 'ep/maint-equals-null-cocci' for maint-2.35Junio C Hamano1-1/+1
* ep/maint-equals-null-cocci: tree-wide: apply equals-null.cocci contrib/coccinnelle: add equals-null.cocci
2022-05-02tree-wide: apply equals-null.cocciJunio C Hamano1-1/+1
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-05Merge branch 'rs/daemon-plug-leak'Junio C Hamano1-1/+1
Plug a memory leak. * rs/daemon-plug-leak: daemon: plug memory leak on overlong path
2021-12-20daemon: plug memory leak on overlong pathRené Scharfe1-1/+1
Release the strbuf containing the interpolated path after copying it to a stack buffer and before erroring out if it's too long. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-25run-command API users: use strvec_push(), not argv constructionÆvar Arnfjörð Bjarmason1-11/+7
Change a pattern of hardcoding an "argv" array size, populating it and assigning to the "argv" member of "struct child_process" to instead use "strvec_push()" to add data to the "args" member. As noted in the preceding commit this moves us further towards being able to remove the "argv" member in a subsequent commit These callers could have used strvec_pushl(), but moving to strvec_push() makes the diff easier to read, and keeps the arguments aligned as before. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-25run-command API users: use strvec_pushv(), not argv assignmentÆvar Arnfjörð Bjarmason1-1/+1
Migrate those run-command API users that assign directly to the "argv" member to use a strvec_pushv() of "args" instead. In these cases it did not make sense to further refactor these callers, e.g. daemon.c could be made to construct the arguments closer to handle(), but that would require moving the construction from its cmd_main() and pass "argv" through two intermediate functions. It would be possible for a change like this to introduce a regression if we were doing: cp.argv = argv; argv[1] = "foo"; And changed the code, as is being done here, to: strvec_pushv(&cp.args, argv); argv[1] = "foo"; But as viewing this change with the "-W" flag reveals none of these functions modify variable that's being pushed afterwards in a way that would introduce such a logic error. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-25Merge branch 'ab/pkt-line-cleanup'Junio C Hamano1-1/+1
Code clean-up. * ab/pkt-line-cleanup: pkt-line.[ch]: remove unused packet_read_line_buf() pkt-line.[ch]: remove unused packet_buf_write_len()
2021-10-15pkt-line.[ch]: remove unused packet_read_line_buf()Ævar Arnfjörð Bjarmason1-1/+1
This function was added in 4981fe750b1 (pkt-line: share buffer/descriptor reading implementation, 2013-02-23), but in 01f9ec64c8a (Use packet_reader instead of packet_read_line, 2018-12-29) the code that was using it was removed. Since it's being removed we can in turn remove the "src" and "src_len" arguments to packet_read(), all the remaining users just passed a NULL/NULL pair to it. That function is only a thin wrapper for packet_read_with_status() which still needs those arguments, but for the thin packet_read() convenience wrapper we can do away with it for now. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macroÆvar Arnfjörð Bjarmason1-12/+7
Remove the hostinfo_init() function added in 01cec54e135 (daemon: deglobalize hostname information, 2015-03-07) and instead initialize the "struct hostinfo" with a macro. This is the more idiomatic pattern in the codebase, and doesn't leave us wondering when we see the *_init() function if this struct needs more complex initialization than a macro can provide. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-08Merge branch 'rs/daemon-sanitize-dir-sep'Junio C Hamano1-4/+4
"git daemon" has been tightened against systems that take backslash as directory separator. * rs/daemon-sanitize-dir-sep: daemon: sanitize all directory separators
2021-03-26daemon: sanitize all directory separatorsRené Scharfe1-4/+4
When sanitizing client-supplied strings on Windows, also strip off backslashes, not just slashes. Signed-off-by: René Scharfe <l.s.r@web.de> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-13use CALLOC_ARRAYRené Scharfe1-2/+2
Add and apply a semantic patch for converting code that open-codes CALLOC_ARRAY to use it instead. It shortens the code and infers the element size automatically. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-30strvec: rename struct fieldsJeff King1-4/+4
The "argc" and "argv" names made sense when the struct was argv_array, but now they're just confusing. Let's rename them to "nr" (which we use for counts elsewhere) and "v" (which is rather terse, but reads well when combined with typical variable names like "args.v"). Note that we have to update all of the callers immediately. Playing tricks with the preprocessor is hard here, because we wouldn't want to rewrite unrelated tokens. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-28strvec: fix indentation in renamed callsJeff King1-3/+3
Code which split an argv_array call across multiple lines, like: argv_array_pushl(&args, "one argument", "another argument", "and more", NULL); was recently mechanically renamed to use strvec, which results in mis-matched indentation like: strvec_pushl(&args, "one argument", "another argument", "and more", NULL); Let's fix these up to align the arguments with the opening paren. I did this manually by sifting through the results of: git jump grep 'strvec_.*,$' and liberally applying my editor's auto-format. Most of the changes are of the form shown above, though I also normalized a few that had originally used a single-tab indentation (rather than our usual style of aligning with the open paren). I also rewrapped a couple of obvious cases (e.g., where previously too-long lines became short enough to fit on one), but I wasn't aggressive about it. In cases broken to three or more lines, the grouping of arguments is sometimes meaningful, and it wasn't worth my time or reviewer time to ponder each case individually. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-28strvec: convert more callers away from argv_array nameJeff King1-26/+26
We eventually want to drop the argv_array name and just use strvec consistently. There's no particular reason we have to do it all at once, or care about interactions between converted and unconverted bits. Because of our preprocessor compat layer, the names are interchangeable to the compiler (so even a definition and declaration using different names is OK). This patch converts remaining files from the first half of the alphabet, to keep the diff to a manageable size. The conversion was done purely mechanically with: git ls-files '*.c' '*.h' | xargs perl -i -pe ' s/ARGV_ARRAY/STRVEC/g; s/argv_array/strvec/g; ' and then selectively staging files with "git add '[abcdefghjkl]*'". We'll deal with any indentation/style fallouts separately. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-10Fix spelling errors in code commentsElijah Newren1-2/+2
Reported-by: Jens Schleusener <Jens.Schleusener@fossies.org> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-25Merge branch 'lw/daemon-log-destination'Junio C Hamano1-1/+1
Recent introduction of "--log-destination" option to "git daemon" did not work well when the daemon was run under "--inetd" mode. * lw/daemon-log-destination: daemon.c: fix condition for redirecting stderr
2018-04-09daemon.c: fix condition for redirecting stderrLucas Werkmeister1-1/+1
Since the --log-destination option was added in 0c591cacb ("daemon: add --log-destination=(stderr|syslog|none)", 2018-02-04) with the explicit goal of allowing logging to stderr when running in inetd mode, we should not always redirect stderr to /dev/null in inetd mode, but rather only when stderr is not being used for logging. Signed-off-by: Lucas Werkmeister <mail@lucaswerkmeister.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-21Merge branch 'lw/daemon-log-destination'Junio C Hamano1-7/+39
The log from "git daemon" can be redirected with a new option; one relevant use case is to send the log to standard error (instead of syslog) when running it from inetd. * lw/daemon-log-destination: daemon: add --log-destination=(stderr|syslog|none)
2018-02-05daemon: add --log-destination=(stderr|syslog|none)Lucas Werkmeister1-7/+39
This new option can be used to override the implicit --syslog of --inetd, or to disable all logging. (While --detach also implies --syslog, --log-destination=stderr with --detach is useless since --detach disassociates the process from the original stderr.) --syslog is retained as an alias for --log-destination=syslog. --log-destination always overrides implicit --syslog regardless of option order. This is different than the “last one wins” logic that applies to some implicit options elsewhere in Git, but should hopefully be less confusing. (I also don’t know if *all* implicit options in Git follow “last one wins”.) The combination of --inetd with --log-destination=stderr is useful, for instance, when running `git daemon` as an instanced systemd service (with associated socket unit). In this case, log messages sent via syslog are received by the journal daemon, but run the risk of being processed at a time when the `git daemon` process has already exited (especially if the process was very short-lived, e.g. due to client error), so that the journal daemon can no longer read its cgroup and attach the message to the correct systemd unit (see systemd/systemd#2913 [1]). Logging to stderr instead can solve this problem, because systemd can connect stderr directly to the journal daemon, which then already knows which unit is associated with this stream. [1]: https://github.com/systemd/systemd/issues/2913 Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Lucas Werkmeister <mail@lucaswerkmeister.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-25daemon: fix length computation in newline strippingJeff King1-4/+2
When git-daemon gets a pktline request, we strip off any trailing newline, replacing it with a NUL. Clients prior to 5ad312bede (in git v1.4.0) would send: git-upload-pack repo.git\n and we need to strip it off to understand their request. After 5ad312bede, we send the host attribute but no newline, like: git-upload-pack repo.git\0host=example.com\0 Both of these are parsed correctly by git-daemon. But if some client were to combine the two: git-upload-pack repo.git\n\0host=example.com\0 we don't parse it correctly. The problem is that we use the "len" variable to record the position of the NUL separator, but then decrement it when we strip the newline. So we start with: git-upload-pack repo.git\n\0host=example.com\0 ^-- len and end up with: git-upload-pack repo.git\0\0host=example.com\0 ^-- len This is arguably correct, since "len" tells us the length of the initial string, but we don't actually use it for that. What we do use it for is finding the offset of the extended attributes; they used to be at len+1, but are now at len+2. We can solve that by just leaving "len" where it is. We don't have to care about the length of the shortened string, since we just treat it like a C string. No version of Git ever produced such a string, but it seems like the daemon code meant to handle this case (and it seems like a reasonable thing for somebody to do in a 3rd-party implementation). 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>
2018-01-25daemon: handle NULs in extended attribute stringJeff King1-5/+4
If we receive a request with extended attributes after the NUL, we try to write those attributes to the log. We do so with a "%s" format specifier, which will only show characters up to the first NUL. That's enough for printing a "host=" specifier. But since dfe422d04d (daemon: recognize hidden request arguments, 2017-10-16) we may have another NUL, followed by protocol parameters, and those are not logged at all. Let's cut out the attempt to show the whole string, and instead log when we parse individual attributes. We could leave the "extended attributes (%d bytes) exist" part of the log, which in theory could alert us to attributes that fail to parse. But anything we don't parse as a "host=" parameter gets blindly added to the "protocol" attribute, so we'd see it in that part of the log. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-25daemon: fix off-by-one in logging extended attributesJeff King1-2/+2
If receive a request like: git-upload-pack /foo.git\0host=localhost we mark the offset of the NUL byte as "len", and then log the bytes after the NUL with a "%.*s" placeholder, using "pktlen - len" as the length, and "line + len + 1" as the start of the string. This is off-by-one, since the start of the string skips past the separating NUL byte, but the adjusted length includes it. Fortunately this doesn't actually read past the end of the buffer, since "%.*s" will stop when it hits a NUL. And regardless of what is in the buffer, packet_read() will always add an extra NUL terminator for safety. As an aside, the git.git client sends an extra NUL after a "host" field, too, so we'd generally hit that one first, not the one added by packet_read(). You can see this in the test output which reports 15 bytes, even though the string has only 14 bytes of visible data. But the point is that even a client sending unusual data could not get us to read past the end of the buffer, so this is purely a cosmetic fix. 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>
2017-10-17daemon: recognize hidden request argumentsBrandon Williams1-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>
2017-06-15config: don't include config.h by defaultBrandon Williams1-0/+1
Stop including config.h by default in cache.h. Instead only include config.h in those files which require use of the config system. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-23Merge branch 'dt/xgethostname-nul-termination'Junio C Hamano1-4/+0
gethostname(2) may not NUL terminate the buffer if hostname does not fit; unfortunately there is no easy way to see if our buffer was too small, but at least this will make sure we will not end up using garbage past the end of the buffer. * dt/xgethostname-nul-termination: xgethostname: handle long hostnames use HOST_NAME_MAX to size buffers for gethostname(2)
2017-04-18use HOST_NAME_MAX to size buffers for gethostname(2)René Scharfe1-4/+0
POSIX limits the length of host names to HOST_NAME_MAX. Export the fallback definition from daemon.c and use this constant to make all buffers used with gethostname(2) big enough for any possible result and a terminating NUL. Inspired-by: David Turner <dturner@twosigma.com> Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: David Turner <dturner@twosigma.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-30daemon: use an argv_array to exec childrenJeff King1-21/+17
Our struct child_process already has its own argv_array. Let's use that to avoid having to format options into separate buffers. Note that we'll need to declare the child process outside of the run_service_command() helper to do this. But that opens up a further simplification, which is that the helper can append to our argument list, saving each caller from specifying "." manually. Signed-off-by: Jeff King <peff@peff.net>
2016-11-29Merge branch 'jk/daemon-path-ok-check-truncation' into maintJunio C Hamano1-4/+21
"git daemon" used fixed-length buffers to turn URL to the repository the client asked for into the server side directory path, using snprintf() to avoid overflowing these buffers, but allowed possibly truncated paths to the directory. This has been tightened to reject such a request that causes overlong path to be required to serve. * jk/daemon-path-ok-check-truncation: daemon: detect and reject too-long paths
2016-10-31Merge branch 'ls/filter-process'Junio C Hamano1-1/+1
The smudge/clean filter API expect an external process is spawned to filter the contents for each path that has a filter defined. A new type of "process" filter API has been added to allow the first request to run the filter for a path to spawn a single process, and all filtering need is served by this single process for multiple paths, reducing the process creation overhead. * ls/filter-process: contrib/long-running-filter: add long running filter example convert: add filter.<driver>.process option convert: prepare filter.<driver>.process option convert: make apply_filter() adhere to standard Git error handling pkt-line: add functions to read/write flush terminated packet streams pkt-line: add packet_write_gently() pkt-line: add packet_flush_gently() pkt-line: add packet_write_fmt_gently() pkt-line: extract set_packet_header() pkt-line: rename packet_write() to packet_write_fmt() run-command: add clean_on_exit_handler run-command: move check_pipe() from write_or_die to run_command convert: modernize tests convert: quote filter names in error messages
2016-10-27Merge branch 'jk/daemon-path-ok-check-truncation'Junio C Hamano1-4/+21
"git daemon" used fixed-length buffers to turn URL to the repository the client asked for into the server side directory path, using snprintf() to avoid overflowing these buffers, but allowed possibly truncated paths to the directory. This has been tightened to reject such a request that causes overlong path to be required to serve. * jk/daemon-path-ok-check-truncation: daemon: detect and reject too-long paths
2016-10-24daemon: detect and reject too-long pathsJeff King1-4/+21
When we are checking the path via path_ok(), we use some fixed PATH_MAX buffers. We write into them via snprintf(), so there's no possibility of overflow, but it does mean we may silently truncate the path, leading to potentially confusing errors when the partial path does not exist. We're better off to reject the path explicitly. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-17pkt-line: rename packet_write() to packet_write_fmt()Lars Schneider1-1/+1
packet_write() should be called packet_write_fmt() because it is a printf-like function that takes a format string as first parameter. packet_write_fmt() should be used for text strings only. Arbitrary binary data should use a new packet_write() function that is introduced in a subsequent patch. Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Lars Schneider <larsxschneider@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-08Merge branch 'jk/common-main' into maintJunio C Hamano1-11/+5
There are certain house-keeping tasks that need to be performed at the very beginning of any Git program, and programs that are not built-in commands had to do them exactly the same way as "git" potty does. It was easy to make mistakes in one-off standalone programs (like test helpers). A common "main()" function that calls cmd_main() of individual program has been introduced to make it harder to make mistakes. * jk/common-main: mingw: declare main()'s argv as const common-main: call git_setup_gettext() common-main: call restore_sigpipe_to_default() common-main: call sanitize_stdfds() common-main: call git_extract_argv0_path() add an extra level of indirection to main()
2016-07-28Merge branch 'ew/daemon-socket-keepalive'Junio C Hamano1-3/+5
Recent update to "git daemon" tries to enable the socket-level KEEPALIVE, but when it is spawned via inetd, the standard input file descriptor may not necessarily be connected to a socket. Suppress an ENOTSOCK error from setsockopt(). * ew/daemon-socket-keepalive: Windows: add missing definition of ENOTSOCK daemon: ignore ENOTSOCK from setsockopt
2016-07-19Merge branch 'jk/common-main'Junio C Hamano1-11/+5
There are certain house-keeping tasks that need to be performed at the very beginning of any Git program, and programs that are not built-in commands had to do them exactly the same way as "git" potty does. It was easy to make mistakes in one-off standalone programs (like test helpers). A common "main()" function that calls cmd_main() of individual program has been introduced to make it harder to make mistakes. * jk/common-main: mingw: declare main()'s argv as const common-main: call git_setup_gettext() common-main: call restore_sigpipe_to_default() common-main: call sanitize_stdfds() common-main: call git_extract_argv0_path() add an extra level of indirection to main()
2016-07-18daemon: ignore ENOTSOCK from setsockoptEric Wong1-3/+5
In inetd mode, we are not guaranteed stdin or stdout is a socket; callers could filter the data through a pipe or be testing with regular files. This prevents t5802 from polluting syslog. Signed-off-by: Eric Wong <e@80x24.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-01common-main: call git_setup_gettext()Jeff King1-2/+0
This should be part of every program, as otherwise users do not get translated error messages. However, some external commands forgot to do so (e.g., git-credential-store). This fixes them, and eliminates the repeated code in programs that did remember to use it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-01common-main: call sanitize_stdfds()Jeff King1-2/+1
This is setup that should be done in every program for safety, but we never got around to adding it everywhere (so builtins benefited from the call in git.c, but any external commands did not). Putting it in the common main() gives us this safety everywhere. Note that the case in daemon.c is a little funny. We wait until we know whether we want to daemonize, and then either: - call daemonize(), which will close stdio and reopen it to /dev/null under the hood - sanitize_stdfds(), to fix up any odd cases But that is way too late; the point of sanitizing is to give us reliable descriptors on 0/1/2, and we will already have executed code, possibly called die(), etc. The sanitizing should be the very first thing that happens. With this patch, git-daemon will sanitize first, and can remove the call in the non-daemonize case. It does mean that daemonize() may just end up closing the descriptors we opened, but that's not a big deal (it's not wrong to do so, nor is it really less optimal than the case where our parent process redirected us from /dev/null ahead of time). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-01common-main: call git_extract_argv0_path()Jeff King1-3/+0
Every program which links against libgit.a must call this function, or risk hitting an assert() in system_path() that checks whether we have configured argv0_path (though only when RUNTIME_PREFIX is defined, so essentially only on Windows). Looking at the diff, you can see that putting it into the common main() saves us having to do it individually in each of the external commands. But what you can't see are the cases where we _should_ have been doing so, but weren't (e.g., git-credential-store, and all of the t/helper test programs). This has been an accident-waiting-to-happen for a long time, but wasn't triggered until recently because it involves one of those programs actually calling system_path(). That happened with git-credential-store in v2.8.0 with ae5f677 (lazily load core.sharedrepository, 2016-03-11). The program: - takes a lock file, which... - opens a tempfile, which... - calls adjust_shared_perm to fix permissions, which... - lazy-loads the config (as of ae5f677), which... - calls system_path() to find the location of /etc/gitconfig On systems with RUNTIME_PREFIX, this means credential-store reliably hits that assert() and cannot be used. We never noticed in the test suite, because we set GIT_CONFIG_NOSYSTEM there, which skips the system_path() lookup entirely. But if we were to tweak git_config() to find /etc/gitconfig even when we aren't going to open it, then the test suite shows multiple failures (for credential-store, and for some other test helpers). I didn't include that tweak here because it's way too specific to this particular call to be worth carrying around what is essentially dead code. The implementation is fairly straightforward, with one exception: there is exactly one caller (git.c) that actually cares about the result of the function, and not the side-effect of setting up argv0_path. We can accommodate that by simply replacing the value of argv[0] in the array we hand down to cmd_main(). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-01add an extra level of indirection to main()Jeff King1-4/+4
There are certain startup tasks that we expect every git process to do. In some cases this is just to improve the quality of the program (e.g., setting up gettext()). In others it is a requirement for using certain functions in libgit.a (e.g., system_path() expects that you have called git_extract_argv0_path()). Most commands are builtins and are covered by the git.c version of main(). However, there are still a few external commands that use their own main(). Each of these has to remember to include the correct startup sequence, and we are not always consistent. Rather than just fix the inconsistencies, let's make this harder to get wrong by providing a common main() that can run this standard startup. We basically have two options to do this: - the compat/mingw.h file already does something like this by adding a #define that replaces the definition of main with a wrapper that calls mingw_startup(). The upside is that the code in each program doesn't need to be changed at all; it's rewritten on the fly by the preprocessor. The downside is that it may make debugging of the startup sequence a bit more confusing, as the preprocessor is quietly inserting new code. - the builtin functions are all of the form cmd_foo(), and git.c's main() calls them. This is much more explicit, which may make things more obvious to somebody reading the code. It's also more flexible (because of course we have to figure out _which_ cmd_foo() to call). The downside is that each of the builtins must define cmd_foo(), instead of just main(). This patch chooses the latter option, preferring the more explicit approach, even though it is more invasive. We introduce a new file common-main.c, with the "real" main. It expects to call cmd_main() from whatever other objects it is linked against. We link common-main.o against anything that links against libgit.a, since we know that such programs will need to do this setup. Note that common-main.o can't actually go inside libgit.a, as the linker would not pick up its main() function automatically (it has no callers). The rest of the patch is just adjusting all of the various external programs (mostly in t/helper) to use cmd_main(). I've provided a global declaration for cmd_main(), which means that all of the programs also need to match its signature. In particular, many functions need to switch to "const char **" instead of "char **" for argv. This effect ripples out to a few other variables and functions, as well. This makes the patch even more invasive, but the end result is much better. We should be treating argv strings as const anyway, and now all programs conform to the same signature (which also matches the way builtins are defined). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-25daemon: enable SO_KEEPALIVE for all socketsEric Wong1-0/+14
While --init-timeout and --timeout options exist and I've never run git-daemon without them, some users may forget to set them and encounter hung daemon processes when connections fail. Enable socket-level timeouts so the kernel can send keepalive probes as necessary to detect failed connections. Signed-off-by: Eric Wong <e@80x24.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-02-26Merge branch 'jk/tighten-alloc'Junio C Hamano1-7/+5
Update various codepaths to avoid manually-counted malloc(). * jk/tighten-alloc: (22 commits) ewah: convert to REALLOC_ARRAY, etc convert ewah/bitmap code to use xmalloc diff_populate_gitlink: use a strbuf transport_anonymize_url: use xstrfmt git-compat-util: drop mempcpy compat code sequencer: simplify memory allocation of get_message test-path-utils: fix normalize_path_copy output buffer size fetch-pack: simplify add_sought_entry fast-import: simplify allocation in start_packfile write_untracked_extension: use FLEX_ALLOC helper prepare_{git,shell}_cmd: use argv_array use st_add and st_mult for allocation size computation convert trivial cases to FLEX_ARRAY macros use xmallocz to avoid size arithmetic convert trivial cases to ALLOC_ARRAY convert manual allocations to argv_array argv-array: add detach function add helpers for allocating flex-array structs harden REALLOC_ARRAY and xcalloc against size_t overflow tree-diff: catch integer overflow in combine_diff_path allocation ...
2016-02-22convert manual allocations to argv_arrayJeff King1-7/+5
There are many manual argv allocations that predate the argv_array API. Switching to that API brings a few advantages: 1. We no longer have to manually compute the correct final array size (so it's one less thing we can screw up). 2. In many cases we had to make a separate pass to count, then allocate, then fill in the array. Now we can do it in one pass, making the code shorter and easier to follow. 3. argv_array handles memory ownership for us, making it more obvious when things should be free()d and and when not. Most of these cases are pretty straightforward. In some, we switch from "run_command_v" to "run_command" which lets us directly use the argv_array embedded in "struct child_process". Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-01-15strbuf: introduce strbuf_getline_{lf,nul}()Junio C Hamano1-1/+1
The strbuf_getline() interface allows a byte other than LF or NUL as the line terminator, but this is only because I wrote these codepaths anticipating that there might be a value other than NUL and LF that could be useful when I introduced line_termination long time ago. No useful caller that uses other value has emerged. By now, it is clear that the interface is overly broad without a good reason. Many codepaths have hardcoded preference to read either LF terminated or NUL terminated records from their input, and then call strbuf_getline() with LF or NUL as the third parameter. This step introduces two thin wrappers around strbuf_getline(), namely, strbuf_getline_lf() and strbuf_getline_nul(), and mechanically rewrites these call sites to call either one of them. The changes contained in this patch are: * introduction of these two functions in strbuf.[ch] * mechanical conversion of all callers to strbuf_getline() with either '\n' or '\0' as the third parameter to instead call the respective thin wrapper. After this step, output from "git grep 'strbuf_getline('" would become a lot smaller. An interim goal of this series is to make this an empty set, so that we can have strbuf_getline_crlf() take over the shorter name strbuf_getline(). Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-11-03Merge branch 'rs/daemon-plug-child-leak'Junio C Hamano1-0/+1
"git daemon" uses "run_command()" without "finish_command()", so it needs to release resources itself, which it forgot to do. * rs/daemon-plug-child-leak: daemon: plug memory leak run-command: factor out child_process_clear()
2015-11-02daemon: plug memory leakRené Scharfe1-0/+1
Call child_process_clear() when a child ends to release the memory allocated for its environment. This is necessary because unlike all other users of start_command() we don't call finish_command(), which would have taken care of that for us. This leak was introduced by f063d38b (daemon: use cld->env_array when re-spawning). Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-10-05daemon: use cld->env_array when re-spawningJeff King1-16/+10
This avoids an ugly strcat into a fixed-size buffer. It's not wrong (the buffer is plenty large enough for an IPv6 address plus some minor formatting), but it takes some effort to verify that. Unfortunately we are still stuck with some fixed-size buffers to hold the output of inet_ntop. But at least we now pass very easy-to-verify parameters, rather than doing a manual computation to account for other data in the buffer. As a side effect, this also fixes the case where we might pass an uninitialized portbuf buffer through the environment. This probably couldn't happen in practice, as it would mean that addr->sa_family was neither AF_INET nor AF_INET6 (and that is all we are listening on). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-25convert trivial sprintf / strcpy calls to xsnprintfJeff King1-2/+2
We sometimes sprintf into fixed-size buffers when we know that the buffer is large enough to fit the input (either because it's a constant, or because it's numeric input that is bounded in size). Likewise with strcpy of constant strings. However, these sites make it hard to audit sprintf and strcpy calls for buffer overflows, as a reader has to cross-reference the size of the array with the input. Let's use xsnprintf instead, which communicates to a reader that we don't expect this to overflow (and catches the mistake in case we do). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-25write_file(): drop caller-supplied LF from calls to create a one-liner fileJunio C Hamano1-1/+1
All of the callsites covered by this change call write_file() or write_file_gently() to create a one-liner file. Drop the caller supplied LF and let these callees to append it as necessary. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-24write_file(): drop "fatal" parameterJunio C Hamano1-1/+1
All callers except three passed 1 for the "fatal" parameter to ask this function to die upon error, but to a casual reader of the code, it was not all obvious what that 1 meant. Instead, split the function into two based on a common write_file_v() that takes the flag, introduce write_file_gently() as a new way to attempt creating a file without dying on error, and make three callers to call it. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-11Merge branch 'jc/daemon-no-ipv6-for-2.4.1'Junio C Hamano1-1/+1
"git daemon" fails to build from the source under NO_IPV6 configuration (regression in 2.4). * jc/daemon-no-ipv6-for-2.4.1: daemon: unbreak NO_IPV6 build regression
2015-05-11Merge branch 'nd/multiple-work-trees'Junio C Hamano1-10/+1
A replacement for contrib/workdir/git-new-workdir that does not rely on symbolic links and make sharing of objects and refs safer by making the borrowee and borrowers aware of each other. * nd/multiple-work-trees: (41 commits) prune --worktrees: fix expire vs worktree existence condition t1501: fix test with split index t2026: fix broken &&-chain t2026 needs procondition SANITY git-checkout.txt: a note about multiple checkout support for submodules checkout: add --ignore-other-wortrees checkout: pass whole struct to parse_branchname_arg instead of individual flags git-common-dir: make "modules/" per-working-directory directory checkout: do not fail if target is an empty directory t2025: add a test to make sure grafts is working from a linked checkout checkout: don't require a work tree when checking out into a new one git_path(): keep "info/sparse-checkout" per work-tree count-objects: report unused files in $GIT_DIR/worktrees/... gc: support prune --worktrees gc: factor out gc.pruneexpire parsing code gc: style change -- no SP before closing parenthesis checkout: clean up half-prepared directories in --to mode checkout: reject if the branch is already checked out elsewhere prune: strategies for linked checkouts checkout: support checking out into a new working directory ...
2015-05-05daemon: unbreak NO_IPV6 build regressionJunio C Hamano1-1/+1
When 01cec54e (daemon: deglobalize hostname information, 2015-03-07) wrapped the global variables such as hostname inside a struct, it forgot to convert one location that spelled "hostname" that needs to be updated to "hi->hostname". This was inside NO_IPV6 block, and was not caught by anybody. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-09daemon: deglobalize hostname informationRené Scharfe1-59/+74
Move the variables related to the client-supplied hostname into its own struct, let execute() own an instance of that instead of storing the information in global variables and pass the struct to any function that needs to access it as a parameter. The lifetime of the variables is easier to see this way. Allocated memory is released within execute(). The strbufs don't have to be reset anymore because they are written to only once at most: parse_host_arg() is only called once by execute() and lookup_hostname() guards against being called twice using hostname_lookup_done. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-09daemon: use strbuf for hostname infoRené Scharfe1-57/+41
Convert hostname, canon_hostname, ip_address and tcp_port to strbuf. This allows to get rid of the helpers strbuf_addstr_or_null() and STRARG because a strbuf always represents a valid (initially empty) string. sanitize_client() is not needed anymore and sanitize_client_strbuf() takes its place and name. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-03Merge branch 'jk/daemon-interpolate'Junio C Hamano1-5/+45
The "interpolated-path" option of "git daemon" inserted any string client declared on the "host=" capability request without checking. Sanitize and limit %H and %CH to a saner and a valid DNS name. * jk/daemon-interpolate: daemon: sanitize incoming virtual hostname t5570: test git-daemon's --interpolated-path option git_connect: let user override virtual-host we send to daemon
2015-03-03Merge branch 'rs/daemon-interpolate'Junio C Hamano1-15/+72
"git daemon" looked up the hostname even when "%CH" and "%IP" interpolations are not requested, which was unnecessary. * rs/daemon-interpolate: daemon: use callback to build interpolated path daemon: look up client-supplied hostname lazily
2015-02-17daemon: use callback to build interpolated pathRené Scharfe1-9/+45
Provide a callback function for strbuf_expand() instead of using the helper strbuf_expand_dict_cb(). While the resulting code is longer, it only looks up the canonical hostname and IP address if at least one of the placeholders %CH and %IP are used with --interpolated-path. Use a struct for passing the directory to the callback function instead of passing it directly to avoid having to cast away its const qualifier. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-02-17daemon: look up client-supplied hostname lazilyRené Scharfe1-8/+29
Look up canonical hostname and IP address using getaddrinfo(3) or gethostbyname(3) only if --interpolated-path or --access-hook were specified. Do that by introducing getter functions for canon_hostname and ip_address and using them for all read accesses. These wrappers call the new helper lookup_hostname(), which sets the variables only at its first call. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-02-17daemon: sanitize incoming virtual hostnameJeff King1-5/+45
We use the daemon_avoid_alias function to make sure that the pathname the user gives us is sane. However, after applying that check, we might then interpolate the path using a string given by the server admin, but which may contain more untrusted data from the client. We should be sure to sanitize this data, as well. We cannot use daemon_avoid_alias here, as it is more strict than we need in requiring a leading '/'. At the same time, we can be much more strict here. We are interpreting a hostname, which should not contain slashes or excessive runs of dots, as those things are not allowed in DNS names. Note that in addition to cleansing the hostname field, we must check the "canonical hostname" (%CH) as well as the port (%P), which we take as a raw string. For the canonical hostname, this comes from an actual DNS lookup on the accessed IP, which makes it a much less likely vector for problems. But it does not hurt to sanitize it in the same way. Unfortunately we cannot test this case easily, as it would involve a custom hostname lookup. We do not need to check %IP, as it comes straight from inet_ntop, so must have a sane form. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-12-01use new wrapper write_file() for simple file writingNguyễn Thái Ngọc Duy1-10/+1
This fixes common problems in these code about error handling, forgetting to close the file handle after fprintf() fails, or not printing out the error string.. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-29Merge branch 'rs/daemon-fixes' into maintJunio C Hamano1-18/+15
* rs/daemon-fixes: daemon: remove write-only variable maxfd daemon: fix error message after bind() daemon: handle gethostbyname() error
2014-10-14Merge branch 'rs/daemon-fixes'Junio C Hamano1-18/+15
"git daemon" (with NO_IPV6 build configuration) used to incorrectly use the hostname even when gethostbyname() reported that the given hostname is not found. * rs/daemon-fixes: daemon: remove write-only variable maxfd daemon: fix error message after bind() daemon: handle gethostbyname() error
2014-10-01daemon: remove write-only variable maxfdRené Scharfe1-4/+0
It became unused when 6573faff (NO_IPV6 support for git daemon) replaced select() with poll(). Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-01daemon: fix error message after bind()René Scharfe1-1/+1
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-01daemon: handle gethostbyname() errorRené Scharfe1-13/+14
If the user-supplied hostname can't be found then we should not use it. We already avoid doing that in the non-NO_IPV6 case by checking if the return value of getaddrinfo() is zero (success). Do the same in the NO_IPV6 case and make sure the return value of gethostbyname() isn't NULL before dereferencing this pointer. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-09-11Merge branch 'rs/child-process-init'Junio C Hamano1-5/+3
Code clean-up. * rs/child-process-init: run-command: inline prepare_run_command_v_opt() run-command: call run_command_v_opt_cd_env() instead of duplicating it run-command: introduce child_process_init() run-command: introduce CHILD_PROCESS_INIT
2014-08-20run-command: introduce CHILD_PROCESS_INITRené Scharfe1-5/+3
Most struct child_process variables are cleared using memset first after declaration. Provide a macro, CHILD_PROCESS_INIT, that can be used to initialize them statically instead. That's shorter, doesn't require a function call and is slightly more readable (especially given that we already have STRBUF_INIT, ARGV_ARRAY_INIT etc.). Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-07daemon.c: replace `git_config()` with `git_config_get_bool()` familyTanay Abhra1-22/+4
Use `git_config_get_bool()` family instead of `git_config()` to take advantage of the config-set API which provides a cleaner control flow. Signed-off-by: Tanay Abhra <tanayabh@gmail.com> Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-07-16Merge branch 'cc/replace-edit'Junio C Hamano1-1/+0
Teach "git replace" an "--edit" mode. * cc/replace-edit: replace: use argv_array in export_object avoid double close of descriptors handed to run_command replace: replace spaces with tabs in indentation
2014-06-25avoid double close of descriptors handed to run_commandJeff King1-1/+0
When a file descriptor is given to run_command via the "in", "out", or "err" parameters, run_command takes ownership. The descriptor will be closed in the parent process whether the process is spawned successfully or not, and closing it again is wrong. In practice this has not caused problems, because we usually close() right after start_command returns, meaning no other code has opened a descriptor in the meantime. So we just get EBADF and ignore it (rather than accidentally closing somebody else's descriptor!). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-20daemon: use skip_prefix to avoid magic numbersJeff King1-5/+6
Like earlier cases, we can use skip_prefix to avoid magic numbers that must match the length of starts_with prefixes. However, the numbers are a little more complicated here, as we keep parsing past the prefix. We can solve it by keeping a running pointer as we parse; its final value is the location we want. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-20use skip_prefix to avoid magic numbersJeff King1-35/+38
It's a common idiom to match a prefix and then skip past it with a magic number, like: if (starts_with(foo, "bar")) foo += 3; This is easy to get wrong, since you have to count the prefix string yourself, and there's no compiler check if the string changes. We can use skip_prefix to avoid the magic numbers here. Note that some of these conversions could be much shorter. For example: if (starts_with(arg, "--foo=")) { bar = arg + 6; continue; } could become: if (skip_prefix(arg, "--foo=", &bar)) continue; However, I have left it as: if (skip_prefix(arg, "--foo=", &v)) { bar = v; continue; } to visually match nearby cases which need to actually process the string. Like: if (skip_prefix(arg, "--foo=", &v)) { bar = atoi(v); continue; } Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-18daemon: mark some strings as constJeff King1-7/+7
None of these strings is modified; marking them as const will help later refactoring. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-16Merge branch 'jk/daemon-tolower'Junio C Hamano1-8/+0
* jk/daemon-tolower: daemon/config: factor out duplicate xstrdup_tolower
2014-05-23daemon/config: factor out duplicate xstrdup_tolowerJeff King1-8/+0
We have two implementations of the same function; let's drop that to one. We take the name from daemon.c, but the implementation (which is just slightly more efficient) from the config code. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-02-10daemon: move daemonize() to libgit.aNguyễn Thái Ngọc Duy1-26/+4
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-01-10Merge branch 'nd/daemon-informative-errors-typofix'Junio C Hamano1-2/+2
* nd/daemon-informative-errors-typofix: daemon: be strict at parsing parameters --[no-]informative-errors
2013-12-20daemon: be strict at parsing parameters --[no-]informative-errorsNguyễn Thái Ngọc Duy1-2/+2
Use strcmp() instead of starts_with()/!prefixcmp() to stop accepting --informative-errors-just-a-little Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-12-05replace {pre,suf}fixcmp() with {starts,ends}_with()Christian Couder1-20/+20
Leaving only the function definitions and declarations so that any new topic in flight can still make use of the old functions, replace existing uses of the prefixcmp() and suffixcmp() with new API functions. The change can be recreated by mechanically applying this: $ git grep -l -e prefixcmp -e suffixcmp -- \*.c | grep -v strbuf\\.c | xargs perl -pi -e ' s|!prefixcmp\(|starts_with\(|g; s|prefixcmp\(|!starts_with\(|g; s|!suffixcmp\(|ends_with\(|g; s|suffixcmp\(|!ends_with\(|g; ' on the result of preparatory changes in this series. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-07-24Merge branch 'sb/misc-fixes'Junio C Hamano1-1/+1
Assorted code cleanups and a minor fix. * sb/misc-fixes: diff.c: Do not initialize a variable, which gets reassigned anyway. commit: Fix a memory leak in determine_author_info daemon.c:handle: Remove unneeded check for null pointer.
2013-07-22Merge branch 'tr/protect-low-3-fds'Junio C Hamano1-12/+0
When "git" is spawned in such a way that any of the low 3 file descriptors is closed, our first open() may yield file descriptor 2, and writing error message to it would screw things up in a big way. * tr/protect-low-3-fds: git: ensure 0/1/2 are open in main() daemon/shell: refactor redirection of 0/1/2 from /dev/null
2013-07-17daemon/shell: refactor redirection of 0/1/2 from /dev/nullThomas Rast1-12/+0
Both daemon.c and shell.c contain logic to open FDs 0/1/2 from /dev/null if they are not already open. Move the function in daemon.c to setup.c and use it in shell.c, too. While there, remove a 'not' that inverted the meaning of the comment. The point is indeed to *avoid* messing up. Signed-off-by: Thomas Rast <trast@inf.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-07-15daemon.c:handle: Remove unneeded check for null pointer.Stefan Beller1-1/+1
addr doesn't need to be checked at that line as it it already accessed 7 lines before in the if (addr->sa_family). Signed-off-by: Stefan Beller <stefanbeller@googlemail.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-01Merge branch 'jk/pkt-line-cleanup'Junio C Hamano1-2/+2
Clean up pkt-line API, implementation and its callers to make them more robust. * jk/pkt-line-cleanup: do not use GIT_TRACE_PACKET=3 in tests remote-curl: always parse incoming refs remote-curl: move ref-parsing code up in file remote-curl: pass buffer straight to get_remote_heads teach get_remote_heads to read from a memory buffer pkt-line: share buffer/descriptor reading implementation pkt-line: provide a LARGE_PACKET_MAX static buffer pkt-line: move LARGE_PACKET_MAX definition from sideband pkt-line: teach packet_read_line to chomp newlines pkt-line: provide a generic reading function with options pkt-line: drop safe_write function pkt-line: move a misplaced comment write_or_die: raise SIGPIPE when we get EPIPE upload-archive: use argv_array to store client arguments upload-archive: do not copy repo name send-pack: prefer prefixcmp over memcmp in receive_status fetch-pack: fix out-of-bounds buffer offset in get_ack upload-pack: remove packet debugging harness upload-pack: do not add duplicate objects to shallow list upload-pack: use get_sha1_hex to parse "shallow" lines
2013-03-25Merge branch 'dm/ni-maxhost-may-be-missing' into maint-1.8.1Junio C Hamano1-4/+0
Some sources failed to compile on systems that lack NI_MAXHOST in their system header. * dm/ni-maxhost-may-be-missing: git-compat-util.h: Provide missing netdb.h definitions
2013-03-19Merge branch 'dm/ni-maxhost-may-be-missing'Junio C Hamano1-4/+0
On systems without NI_MAXHOST in their system header files, connect.c (hence most of the transport) did not compile. * dm/ni-maxhost-may-be-missing: git-compat-util.h: Provide missing netdb.h definitions
2013-02-25git-compat-util.h: Provide missing netdb.h definitionsDavid Michael1-4/+0
Some platforms may lack the NI_MAXHOST and NI_MAXSERV values in their system headers, so ensure they are available. Signed-off-by: David Michael <fedora.dm0@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-02-24pkt-line: share buffer/descriptor reading implementationJeff King1-1/+1
The packet_read function reads from a descriptor. The packet_get_line function is similar, but reads from an in-memory buffer, and uses a completely separate implementation. This patch teaches the generic packet_read function to accept either source, and we can do away with packet_get_line's implementation. There are two other differences to account for between the old and new functions. The first is that we used to read into a strbuf, but now read into a fixed size buffer. The only two callers are fine with that, and in fact it simplifies their code, since they can use the same static-buffer interface as the rest of the packet_read_line callers (and we provide a similar convenience wrapper for reading from a buffer rather than a descriptor). This is technically an externally-visible behavior change in that we used to accept arbitrary sized packets up to 65532 bytes, and now cap out at LARGE_PACKET_MAX, 65520. In practice this doesn't matter, as we use it only for parsing smart-http headers (of which there is exactly one defined, and it is small and fixed-size). And any extension headers would be breaking the protocol to go over LARGE_PACKET_MAX anyway. The other difference is that packet_get_line would return on error rather than dying. However, both callers of packet_get_line are actually improved by dying. The first caller does its own error checking, but we can drop that; as a result, we'll actually get more specific reporting about protocol breakage when packet_read dies internally. The only downside is that packet_read will not print the smart-http URL that failed, but that's not a big deal; anybody not debugging can already see the remote's URL already, and anybody debugging would want to run with GIT_CURL_VERBOSE anyway to see way more information. The second caller, which is just trying to skip past any extra smart-http headers (of which there are none defined, but which we allow to keep room for future expansion), did not error check at all. As a result, it would treat an error just like a flush packet. The resulting mess would generally cause an error later in get_remote_heads, but now we get error reporting much closer to the source of the problem. Brown-paper-bag-fixes-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-02-20pkt-line: provide a LARGE_PACKET_MAX static bufferJeff King1-2/+2
Most of the callers of packet_read_line just read into a static 1000-byte buffer (callers which handle arbitrary binary data already use LARGE_PACKET_MAX). This works fine in practice, because: 1. The only variable-sized data in these lines is a ref name, and refs tend to be a lot shorter than 1000 characters. 2. When sending ref lines, git-core always limits itself to 1000 byte packets. However, the only limit given in the protocol specification in Documentation/technical/protocol-common.txt is LARGE_PACKET_MAX; the 1000 byte limit is mentioned only in pack-protocol.txt, and then only describing what we write, not as a specific limit for readers. This patch lets us bump the 1000-byte limit to LARGE_PACKET_MAX. Even though git-core will never write a packet where this makes a difference, there are two good reasons to do this: 1. Other git implementations may have followed protocol-common.txt and used a larger maximum size. We don't bump into it in practice because it would involve very long ref names. 2. We may want to increase the 1000-byte limit one day. Since packets are transferred before any capabilities, it's difficult to do this in a backwards-compatible way. But if we bump the size of buffer the readers can handle, eventually older versions of git will be obsolete enough that we can justify bumping the writers, as well. We don't have plans to do this anytime soon, but there is no reason not to start the clock ticking now. Just bumping all of the reading bufs to LARGE_PACKET_MAX would waste memory. Instead, since most readers just read into a temporary buffer anyway, let's provide a single static buffer that all callers can use. We can further wrap this detail away by having the packet_read_line wrapper just use the buffer transparently and return a pointer to the static storage. That covers most of the cases, and the remaining ones already read into their own LARGE_PACKET_MAX buffers. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-02-20pkt-line: teach packet_read_line to chomp newlinesJeff King1-1/+1
The packets sent during ref negotiation are all terminated by newline; even though the code to chomp these newlines is short, we end up doing it in a lot of places. This patch teaches packet_read_line to auto-chomp the trailing newline; this lets us get rid of a lot of inline chomping code. As a result, some call-sites which are not reading line-oriented data (e.g., when reading chunks of packfiles alongside sideband) transition away from packet_read_line to the generic packet_read interface. This patch converts all of the existing callsites. Since the function signature of packet_read_line does not change (but its behavior does), there is a possibility of new callsites being introduced in later commits, silently introducing an incompatibility. However, since a later patch in this series will change the signature, such a commit would have to be merged directly into this commit, not to the tip of the series; we can therefore ignore the issue. This is an internal cleanup and should produce no change of behavior in the normal case. However, there is one corner case to note. Callers of packet_read_line have never been able to tell the difference between a flush packet ("0000") and an empty packet ("0004"), as both cause packet_read_line to return a length of 0. Readers treat them identically, even though Documentation/technical/protocol-common.txt says we must not; it also says that implementations should not send an empty pkt-line. By stripping out the newline before the result gets to the caller, we will now treat the newline-only packet ("0005\n") the same as an empty packet, which in turn gets treated like a flush packet. In practice this doesn't matter, as neither empty nor newline-only packets are part of git's protocols (at least not for the line-oriented bits, and readers who are not expecting line-oriented packets will be calling packet_read directly, anyway). But even if we do decide to care about the distinction later, it is orthogonal to this patch. The right place to tighten would be to stop treating empty packets as flush packets, and this change does not make doing so any harder. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-08-15daemon: --access-hook optionJunio C Hamano1-0/+77
The --access-hook option to "git daemon" specifies an external command to be run every time a client connects, with - service name (e.g. "upload-pack", etc.), - path to the repository, - hostname (%H), - canonical hostname (%CH), - ip address (%IP), - tcp port (%P) as its command line arguments. The external command can decide to decline the service by exiting with a non-zero status (or to allow it by exiting with a zero status). It can also look at the $REMOTE_ADDR and $REMOTE_PORT environment variables to learn about the requestor when making this decision. The external command can optionally write a single line to its standard output to be sent to the requestor as an error message when it declines the service. Acked-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-01-08git-daemon: produce output when readyClemens Buchacher1-3/+3
If a client tries to connect after git-daemon starts, but before it opens a listening socket, the connection will fail. Output "[PID] Ready to rumble]" after opening the socket successfully in order to inform the user that the daemon is now ready to receive connections. Signed-off-by: Clemens Buchacher <drizzd@aon.at> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-12-05i18n: add infrastructure for translating Git with gettextÆvar Arnfjörð Bjarmason1-0/+2
Change the skeleton implementation of i18n in Git to one that can show localized strings to users for our C, Shell and Perl programs using either GNU libintl or the Solaris gettext implementation. This new internationalization support is enabled by default. If gettext isn't available, or if Git is compiled with NO_GETTEXT=YesPlease, Git falls back on its current behavior of showing interface messages in English. When using the autoconf script we'll auto-detect if the gettext libraries are installed and act appropriately. This change is somewhat large because as well as adding a C, Shell and Perl i18n interface we're adding a lot of tests for them, and for those tests to work we need a skeleton PO file to actually test translations. A minimal Icelandic translation is included for this purpose. Icelandic includes multi-byte characters which makes it easy to test various edge cases, and it's a language I happen to understand. The rest of the commit message goes into detail about various sub-parts of this commit. = Installation Gettext .mo files will be installed and looked for in the standard $(prefix)/share/locale path. GIT_TEXTDOMAINDIR can also be set to override that, but that's only intended to be used to test Git itself. = Perl Perl code that's to be localized should use the new Git::I18n module. It imports a __ function into the caller's package by default. Instead of using the high level Locale::TextDomain interface I've opted to use the low-level (equivalent to the C interface) Locale::Messages module, which Locale::TextDomain itself uses. Locale::TextDomain does a lot of redundant work we don't need, and some of it would potentially introduce bugs. It tries to set the $TEXTDOMAIN based on package of the caller, and has its own hardcoded paths where it'll search for messages. I found it easier just to completely avoid it rather than try to circumvent its behavior. In any case, this is an issue wholly internal Git::I18N. Its guts can be changed later if that's deemed necessary. See <AANLkTilYD_NyIZMyj9dHtVk-ylVBfvyxpCC7982LWnVd@mail.gmail.com> for a further elaboration on this topic. = Shell Shell code that's to be localized should use the git-sh-i18n library. It's basically just a wrapper for the system's gettext.sh. If gettext.sh isn't available we'll fall back on gettext(1) if it's available. The latter is available without the former on Solaris, which has its own non-GNU gettext implementation. We also need to emulate eval_gettext() there. If neither are present we'll use a dumb printf(1) fall-through wrapper. = About libcharset.h and langinfo.h We use libcharset to query the character set of the current locale if it's available. I.e. we'll use it instead of nl_langinfo if HAVE_LIBCHARSET_H is set. The GNU gettext manual recommends using langinfo.h's nl_langinfo(CODESET) to acquire the current character set, but on systems that have libcharset.h's locale_charset() using the latter is either saner, or the only option on those systems. GNU and Solaris have a nl_langinfo(CODESET), FreeBSD can use either, but MinGW and some others need to use libcharset.h's locale_charset() instead. =Credits This patch is based on work by Jeff Epler <jepler@unpythonic.net> who did the initial Makefile / C work, and a lot of comments from the Git mailing list, including Jonathan Nieder, Jakub Narebski, Johannes Sixt, Erik Faye-Lund, Peter Krefting, Junio C Hamano, Thomas Rast and others. [jc: squashed a small Makefile fix from Ramsay] Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-10-21Merge branch 'jk/daemon-msgs'Junio C Hamano1-8/+21
* jk/daemon-msgs: daemon: give friendlier error messages to clients Conflicts: daemon.c
2011-10-21Merge branch 'ph/transport-with-gitfile'Junio C Hamano1-2/+2
* ph/transport-with-gitfile: Fix is_gitfile() for files too small or larger than PATH_MAX to be a gitfile Add test showing git-fetch groks gitfiles Teach transport about the gitfile mechanism Learn to handle gitfiles in enter_repo enter_repo: do not modify input
2011-10-15daemon: give friendlier error messages to clientsJeff King1-4/+21
When the git-daemon is asked about an inaccessible repository, it simply hangs up the connection without saying anything further. This makes it hard to distinguish between a repository we cannot access (e.g., due to typo), and a service or network outage. Instead, let's print an "ERR" line, which git clients understand since v1.6.1 (2008-12-24). Because there is a risk of leaking information about non-exported repositories, by default all errors simply say "access denied or repository not exported". Sites which don't have hidden repositories, or don't care, can pass a flag to turn on more specific messages. Signed-off-by: Jeff King <peff@peff.net> Helped-by: Sitaram Chamarty <sitaramc@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-10-13Merge branch 'nd/git-daemon-error-msgs'Junio C Hamano1-4/+8
* nd/git-daemon-error-msgs: daemon: return "access denied" if a service is not allowed
2011-10-04enter_repo: do not modify inputErik Faye-Lund1-2/+2
entr_repo(..., 0) currently modifies the input to strip away trailing slashes. This means that we some times need to copy the input to keep the original. Change it to unconditionally copy it into the used_path buffer so we can safely use the input without having to copy it. Also store a working copy in validated_path up-front before we start resolving anything. Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Phil Hord <hordp@cisco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-10-03daemon: log errors if we could not use some socketsNguyễn Thái Ngọc Duy1-0/+37
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-10-03daemon: return "access denied" if a service is not allowedNguyễn Thái Ngọc Duy1-4/+8
The message is chosen to avoid leaking information, yet let users know that they are deliberately not allowed to use the service, not a fault in service configuration or the service itself. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-03-22Fix sparse warningsStephen Boyd1-1/+1
Fix warnings from 'make check'. - These files don't include 'builtin.h' causing sparse to complain that cmd_* isn't declared: builtin/clone.c:364, builtin/fetch-pack.c:797, builtin/fmt-merge-msg.c:34, builtin/hash-object.c:78, builtin/merge-index.c:69, builtin/merge-recursive.c:22 builtin/merge-tree.c:341, builtin/mktag.c:156, builtin/notes.c:426 builtin/notes.c:822, builtin/pack-redundant.c:596, builtin/pack-refs.c:10, builtin/patch-id.c:60, builtin/patch-id.c:149, builtin/remote.c:1512, builtin/remote-ext.c:240, builtin/remote-fd.c:53, builtin/reset.c:236, builtin/send-pack.c:384, builtin/unpack-file.c:25, builtin/var.c:75 - These files have symbols which should be marked static since they're only file scope: submodule.c:12, diff.c:631, replace_object.c:92, submodule.c:13, submodule.c:14, trace.c:78, transport.c:195, transport-helper.c:79, unpack-trees.c:19, url.c:3, url.c:18, url.c:104, url.c:117, url.c:123, url.c:129, url.c:136, thread-utils.c:21, thread-utils.c:48 - These files redeclare symbols to be different types: builtin/index-pack.c:210, parse-options.c:564, parse-options.c:571, usage.c:49, usage.c:58, usage.c:63, usage.c:72 - These files use a literal integer 0 when they really should use a NULL pointer: daemon.c:663, fast-import.c:2942, imap-send.c:1072, notes-merge.c:362 While we're in the area, clean up some unused #includes in builtin files (mostly exec_cmd.h). Signed-off-by: Stephen Boyd <bebarino@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-01-04daemon: support <directory> arguments againJonathan Nieder1-3/+4
Ever since v1.7.4-rc0~125^2~8 (daemon: use run-command api for async serving, 2010-11-04), git daemon spawns child processes instead of forking to serve requests. The child processes learn that they are being run for this purpose from the presence of the --serve command line flag. When running with <ok_path> arguments, the --serve flag is treated as one of the path arguments and the special child behavior does not kick in. So the child becomes an ordinary git daemon process, notices that all the addresses it needs are in use, and exits with the message "fatal: unable to allocate any listen sockets on port 9418". Fix it by putting --serve at the beginning of the command line, where the flag cannot be mistaken for a path argument. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Acked-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-11-29Merge branch 'md/interix'Junio C Hamano1-0/+4
* md/interix: Interix: add configure checks add support for the SUA layer (interix; windows) Conflicts: git-compat-util.h
2010-11-04daemon: opt-out on features that require posixErik Faye-Lund1-24/+64
Windows does not supply the POSIX-functions fork(), setuuid(), setgid(), setsid() and initgroups(). Error out if --user or --detach is specified when if so. MinGW doesn't have prototypes and headers for inet_ntop and inet_pton, so include our implementation instead. MSVC does, so avoid doing so there. Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-11-04daemon: make --inetd and --detach incompatibleErik Faye-Lund1-4/+4
Since --inetd makes main return with the result of execute() before daemonize is gets called, these two options are already incompatible. Document it, and add an error if attempted. Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-11-04daemon: use socklen_tErik Faye-Lund1-3/+3
Windows's accept()-function takes the last argument as an int, but glibc takes an unsigned int. Use socklen_t to get rid of a warning. This is basically a revert of 7fa0908, but we have already been depending on socklen_t existing since June 2006 (commit 5b276ee4). I guess this means that socklen_t IS defined on OSX after all - at least in recent headers. Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-11-04daemon: get remote host address from root-processErik Faye-Lund1-40/+39
Get remote host in the process that accept() and pass it through the REMOTE_ADDR environment variable to the handler-process. Introduce the REMOTE_PORT environmen variable for the port. Use these variables for reporting instead of doing getpeername(0, ...), which doesn't work on Windows. Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-11-04daemon: use full buffered mode for stderrErik Faye-Lund1-2/+4
Windows doesn't support line buffered mode for file streams, so let's just use full buffered mode with a big buffer ("4096 should be enough for everyone") and add explicit flushing. Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-11-04daemon: use run-command api for async servingErik Faye-Lund1-46/+47
fork() is only available on POSIX, so to support git-daemon on Windows we have to use something else. Instead we invent the flag --serve, which is a stripped down version of --inetd-mode. We use start_command() to call git-daemon with this flag appended to serve clients. Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-11-04mingw: implement syslogMike Pape1-2/+0
Syslog does not usually exist on Windows, so implement our own using Window's ReportEvent mechanism. Strings containing "%1" gets expanded into them selves by ReportEvent, resulting in an unreadable string. "%2" and above is not a problem. Unfortunately, on Windows an IPv6 address can contain "%1", so expand "%1" to "% 1" before reporting. "%%1" is also a problem for ReportEvent, but that string cannot occur in an IPv6 address. Signed-off-by: Mike Pape <dotzenlabs@gmail.com> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-10-28add support for the SUA layer (interix; windows)Markus Duft1-0/+4
* add required build options to Makefile. * introduce new NO_INTTYPES_H for systems lacking inttypes; code includes stdint.h instead, if this is set. * introduce new NO_SYS_POLL_H for systems lacking sys/poll.h; code includes poll.h instead, if this is set. * introduce NO_INITGROUPS. initgroups() call is simply omitted. Signed-off-by: Markus Duft <mduft@gentoo.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-10-26Merge branch 'as/daemon-multi-listen'Junio C Hamano1-27/+53
* as/daemon-multi-listen: daemon: allow more than one host address given via --listen daemon: add helper function named_sock_setup
2010-10-08Use angles for placeholders consistentlyŠtěpán Němec1-9/+9
Signed-off-by: Štěpán Němec <stepnem@gmail.com> Acked-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-09-28daemon: allow more than one host address given via --listenAlexander Sulfrian1-9/+22
When the host has more than one interfaces, daemon can listen to all of them by not giving any --listen option, or listen to only one. Teach it to accept more than one --listen options. Remove the hostname information form the die, if no socket could be created. It would only trigger when no interface out of either all interface or the ones specified on the command line with --listen options, can be listened to and so the user does know which "host" was asked. Signed-off-by: Alexander Sulfrian <alexander@sulfrian.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-09-28daemon: add helper function named_sock_setupAlexander Sulfrian1-20/+33
Add named_sock_setup as helper function for socksetup to make it easier to create more than one listen sockets. named_sock_setup could be called more than one time and add the new sockets to the supplied socklist_p. Signed-off-by: Alexander Sulfrian <alexander@sulfrian.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-05-31Rewrite dynamic structure initializations to runtime assignmentGary V. Vaughan1-10/+11
Unfortunately, there are still plenty of production systems with vendor compilers that choke unless all compound declarations can be determined statically at compile time, for example hpux10.20 (I can provide a comprehensive list of our supported platforms that exhibit this problem if necessary). This patch simply breaks apart any compound declarations with dynamic initialisation expressions, and moves the initialisation until after the last declaration in the same block, in all the places necessary to have the offending compilers accept the code. Signed-off-by: Gary V. Vaughan <gary@thewrittenword.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-04-03Merge branch 'bc/maint-daemon-sans-ss-family'Junio C Hamano1-4/+7
* bc/maint-daemon-sans-ss-family: daemon.c: avoid accessing ss_family member of struct sockaddr_storage
2010-03-20daemon: parse_host_and_port SIGSEGV if port is specifiedImre Deak1-1/+1
This typo will lead to git-daemon dying any time the connect string includes a port after the host= attribute. This can lead for example to one of the following error messages on the client side when someone tries git clone git://...:<port>. When the daemon is running on localhost: fatal: The remote end hung up unexpectedly or when the daemon is connected through an ssh tunnel: fatal: protocol error: bad line length character: erro In the latter case 'erro' comes from the daemon's reply: error: git-daemon died of signal 11 Signed-off-by: Imre Deak <imre.deak@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-03-16daemon.c: avoid accessing ss_family member of struct sockaddr_storageBrandon Casey1-4/+7
When NO_SOCKADDR_STORAGE is set for a platform, either sockaddr_in or sockaddr_in6 is used intead. Neither of which has an ss_family member. They have an sin_family and sin6_family member respectively. Since the addrcmp() function accesses the ss_family member of a sockaddr_storage struct, compilation fails on platforms which define NO_SOCKADDR_STORAGE. Since any sockaddr_* structure can be cast to a struct sockaddr and have its sa_family member read, do so here to workaround this issue. Thanks to Martin Storsjö for pointing out the fix, and Gary Vaughan for drawing attention to the issue. Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-02-03Typofixes outside documentation areaJunio C Hamano1-1/+1
begining -> beginning canonicalizations -> canonicalization comand -> command dewrapping -> unwrapping dirtyness -> dirtiness DISCLAMER -> DISCLAIMER explicitely -> explicitly feeded -> fed impiled -> implied madatory -> mandatory mimick -> mimic preceeding -> preceding reqeuest -> request substition -> substitution Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-01-27Merge branch 'il/maint-colon-address'Junio C Hamano1-4/+30
* il/maint-colon-address: Allow use of []-wrapped addresses in git:// Support addresses with ':' in git-daemon
2010-01-26Support addresses with ':' in git-daemonIlari Liusvaara1-4/+30
If host address could have ':' in it (e.g. numeric IPv6 address), then host and port could not be uniquely parsed. Fix this by parsing the "["<host>"]":<port> and "["<host>"]" notations. Currently the built-in git:// client would send <host>:<port> or <host> for such thing, but it doesn't matter as due to bugs, resolving address fails if <host> contains ':'. Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-01-20Merge branch 'jk/warn-author-committer-after-commit'Junio C Hamano1-1/+0
* jk/warn-author-committer-after-commit: user_ident_sufficiently_given(): refactor the logic to be usable from elsewhere commit.c::print_summary: do not release the format string too early commit: allow suppression of implicit identity advice commit: show interesting ident information in summary strbuf: add strbuf_addbuf_percentquote strbuf_expand: convert "%%" to "%" Conflicts: builtin-commit.c ident.c
2010-01-14strbuf_expand: convert "%%" to "%"Jeff King1-1/+0
The only way to safely quote arbitrary text in a pretty-print user format is to replace instances of "%" with "%x25". This is slightly unreadable, and many users would expect "%%" to produce a single "%", as that is what printf format specifiers do. This patch converts "%%" to "%" for all users of strbuf_expand(): (1) git-daemon interpolated paths (2) pretty-print user formats (3) merge driver command lines Case (1) was already doing the conversion itself outside of strbuf_expand(). Case (2) is the intended beneficiary of this patch. Case (3) users probably won't notice, but as this is user-facing behavior, consistently providing the quoting mechanism makes sense. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-01-09daemon: consider only address in kill_some_child()Erik Faye-Lund1-8/+20
kill_some_child() compares the entire sockaddr_storage structure (with the pad-bits zeroed out) when trying to find out if connections originate from the same host. However, sockaddr_storage contains the port-number for the connection (which varies between connections), so the comparison always fails. Change the code so we only consider the host-address, by introducing the addrcmp()-function that inspects the address family and compare as appropriate. Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-11-22Merge branch 'tc/format-attribute'Junio C Hamano1-0/+2
* tc/format-attribute: Check the format of more printf-type functions
2009-11-15Check the format of more printf-type functionsTarmigan Casebolt1-0/+2
We already have these checks in many printf-type functions that have prototypes which are in header files. Add these same checks to some more prototypes in header functions and to static functions in .c files. cc: Miklos Vajna <vmiklos@frugalware.org> Signed-off-by: Tarmigan Casebolt <tarmigan+git@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-11-09http-backend: Protect GIT_PROJECT_ROOT from /../ requestsShawn O. Pearce1-48/+1
Eons ago HPA taught git-daemon how to protect itself from /../ attacks, which Junio brought back into service in d79374c7b58d ("daemon.c and path.enter_repo(): revamp path validation"). I did not carry this into git-http-backend as originally we relied only upon PATH_TRANSLATED, and assumed the HTTP server had done its access control checks to validate the resolved path was within a directory permitting access from the remote client. This would usually be sufficient to protect a server from requests for its /etc/passwd file by http://host/smart/../etc/passwd sorts of URLs. However in 917adc036086 Mark Lodato added GIT_PROJECT_ROOT as an additional method of configuring the CGI. When this environment variable is used the web server does not generate the final access path and therefore may blindly pass through "/../etc/passwd" in PATH_INFO under the assumption that "/../" might have special meaning to the invoked CGI. Instead of permitting these sorts of malformed path requests, we now reject them back at the client, with an error message for the server log. This matches git-daemon behavior. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-07-06Merge branch 'tr/die_errno'Junio C Hamano1-8/+7
* tr/die_errno: Use die_errno() instead of die() when checking syscalls Convert existing die(..., strerror(errno)) to die_errno() die_errno(): double % in strerror() output just in case Introduce die_errno() that appends strerror(errno) to die()
2009-07-01Merge branch 'js/daemon-log'Junio C Hamano1-8/+48
* js/daemon-log: receive-pack: do not send error details to the client upload-pack: squelch progress indicator if client cannot see it daemon: send stderr of service programs to the syslog
2009-06-27Convert existing die(..., strerror(errno)) to die_errno()Thomas Rast1-8/+7
Change calls to die(..., strerror(errno)) to use the new die_errno(). In the process, also make slight style adjustments: at least state _something_ about the function that failed (instead of just printing the pathname), and put paths in single quotes. Signed-off-by: Thomas Rast <trast@student.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-06-20Fix various sparse warnings in the git source codeLinus Torvalds1-1/+1
There are a few remaining ones, but this fixes the trivial ones. It boils down to two main issues that sparse complains about: - warning: Using plain integer as NULL pointer Sparse doesn't like you using '0' instead of 'NULL'. For various good reasons, not the least of which is just the visual confusion. A NULL pointer is not an integer, and that whole "0 works as NULL" is a historical accident and not very pretty. A few of these remain: zlib is a total mess, and Z_NULL is just a 0. I didn't touch those. - warning: symbol 'xyz' was not declared. Should it be static? Sparse wants to see declarations for any functions you export. A lack of a declaration tends to mean that you should either add one, or you should mark the function 'static' to show that it's in file scope. A few of these remain: I only did the ones that should obviously just be made static. That 'wt_status_submodule_summary' one is debatable. It has a few related flags (like 'wt_status_use_color') which _are_ declared, and are used by builtin-commit.c. So maybe we'd like to export it at some point, but it's not declared now, and not used outside of that file, so 'static' it is in this patch. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-06-18daemon: send stderr of service programs to the syslogJohannes Sixt1-8/+48
If git-daemon is run with --detach or --inetd, then stderr is explicitly redirected to /dev/null. But notice that the service programs were spawned via execl_git_cmd(), in particular, the stderr channel is inherited from the daemon. This means that errors that the programs wrote to stderr (for example, via die()), went to /dev/null. This patch arranges that the daemon does not merely exec the service program, but forks it and monitors stderr of the child; it writes the errors that it produces to the daemons log via logerror(). A consequence is that the daemon process remains in memory for the full duration of the service program, but this cannot be avoided. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-06-06daemon: Strictly parse the "extra arg" part of the commandShawn O. Pearce1-4/+6
Since 1.4.4.5 (49ba83fb67 "Add virtualization support to git-daemon") git daemon enters an infinite loop and never terminates if a client hides any extra arguments in the initial request line which is not exactly "\0host=blah\0". Since that change, a client must never insert additional extra arguments, or attempt to use any argument other than "host=", as any daemon will get stuck parsing the request line and will never complete the request. Since the client can't tell if the daemon is patched or not, it is not possible to know if additional extra args might actually be able to be safely requested. If we ever need to extend the git daemon protocol to support a new feature, we may have to do something like this to the exchange: # If both support git:// v2 # C: 000cgit://v2 S: 0010ok host user C: 0018host git.kernel.org C: 0027git-upload-pack /pub/linux-2.6.git S: ...git-upload-pack header... # If client supports git:// v2, server does not: # C: 000cgit://v2 S: <EOF> C: 003bgit-upload-pack /pub/linux-2.6.git\0host=git.kernel.org\0 S: ...git-upload-pack header... This requires the client to create two TCP connections to talk to an older git daemon, however all daemons since the introduction of daemon.c will safely reject the unknown "git://v2" command request, so the client can quite easily determine the server supports an older protocol. Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-04-29daemon.c: fix segfault on OS XBenjamin Kramer1-14/+14
On OS X (and maybe other unices), getaddrinfo(3) returns NULL in the ai_canonname field if it's called with an IP address for the hostname. We'll now use the IP address for the hostname if ai_canonname was NULL, this also matches the behaviour on Linux. steps to reproduce: $ git daemon --export-all $ git clone git://127.0.0.1/frotz => git daemon's fork (silently) segfaults. Remove the pointless loop while at it. There is only one iteration because of the break; on the last line and there are no continues. Signed-off-by: Benjamin Kramer <benny.kra@googlemail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-03-04improve missing repository error messageJeff King1-1/+1
Certain remote commands, when asked to do something in a particular directory that was not actually a git repository, would say "unable to chdir or not a git archive". The "chdir" bit is an unnecessary detail, and the term "git archive" is much less common these days than "git repository". So let's switch them all to: fatal: '%s' does not appear to be a git repository Signed-off-by: Jeff King <peff@peff.net> Acked-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-01-26Add calls to git_extract_argv0_path() in programs that call git_config_*Steffen Prohaska1-0/+2
Programs that use git_config need to find the global configuration. When runtime prefix computation is enabled, this requires that git_extract_argv0_path() is called early in the program's main(). This commit adds the necessary calls. Signed-off-by: Steffen Prohaska <prohaska@zib.de> Acked-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-01-05remove trailing LF in die() messagesAlexander Potashev1-1/+1
LF at the end of format strings given to die() is redundant because die already adds one on its own. Signed-off-by: Alexander Potashev <aspotashev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-12-26daemon: cleanup: factor out xstrdup_tolower()René Scharfe1-21/+13
Add xstrdup_tolower(), a helper to get a lower case copy of a string, and use it in two cases. Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-12-26daemon: cleanup: replace loop with ifRené Scharfe1-13/+5
Replace a loop around an enter_repo() call, which was used to retry a single time with a different parameter in case the first call fails, with two calls and an if. This is shorter and cleaner. Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-12-26daemon: handle freopen() failureRené Scharfe1-1/+3
Die if stderr couldn't be sent to /dev/null when operating in inetd mode and report the error message from the OS. This fixes a compiler warning about the return value of freopen() being ignored on Ubuntu 8.10. Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-11-23daemon: deglobalize variable 'directory'René Scharfe1-10/+6
Remove the global variable 'directory' and pass it as a parameter of the two functions that use it instead, (almost) restoring their interface to how it was before 49ba83fb67d9e447b86953965ce5f949c6a93b81. Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-11-23daemon: inline fill_in_extra_table_entries()René Scharfe1-8/+2
Having fill_in_extra_table_entries() as a separate function has no advantage -- a function with no parameters and return values might as well be an anonymous block of code. Its name still refers to the table of interpolate() which has been removed earlier, so it's better to inline it at its only call site. Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-11-23daemon: use strbuf_expand() instead of interpolate()René Scharfe1-55/+54
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-10-24git-daemon: set REMOTE_ADDR to client addressJoey Hess1-0/+4
This allows hooks like pre-receive to look at the client's IP address. Of course the IP address can't be used to get strong security; git-daemon isn't the right thing to use if you need that. However, basic IP address checking can be good enough in some situations. REMOTE_ADDR is the same environment variable used to communicate the client's address to CGI scripts. Signed-off-by: Joey Hess <joey@kitenet.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-09-25Merge branch 'jc/alternate-push'Shawn O. Pearce1-7/+3
* jc/alternate-push: push: receiver end advertises refs from alternate repositories push: prepare sender to receive extended ref information from the receiver receive-pack: make it a builtin is_directory(): a generic helper function
2008-09-09is_directory(): a generic helper functionJunio C Hamano1-7/+3
A simple "grep -e stat --and -e S_ISDIR" revealed there are many open-coded implementations of this function. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-09-06daemon.c: avoid setlinebuf()Junio C Hamano1-1/+2
This function is outside POSIX (Linux and recent BSD have it). Replace it with setvbuf() which is POSIX. I am not sure about the value this patch passes as size argument to setvbuf(), though. I know the call this patch makes is equivalent to calling setlinebuf() with GNU libc, but POSIX itself leaves what happens to the size argument quite vague, saying only "otherwise [i.e. when buf is a null pointer], size _may_ determine the size of a buffer allocated by the setvbuf() function." If passing size=0 causes stdio to allocate very small buffer, and while stdio tries to line buffer the output, it might make it to fail to buffer an entire line, causing early flushing of the stream. Even if that turns out to be a problem on minorority platforms, we won't know it until the issue actually hurts them, so let's push this change out and see what happens. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-08-31cast pid_t's to uintmax_t to improve portabilityDavid Soria Parra1-3/+3
Some systems (like e.g. OpenSolaris) define pid_t as long, therefore all our sprintf that use %i/%d cause a compiler warning beacuse of the implicit long->int cast. To make sure that we fit the limits, we display pids as PRIuMAX and cast them explicitly to uintmax_t. Signed-off-by: David Soria Parra <dsp@php.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-08-25daemon.c: minor style fixupJunio C Hamano1-42/+34
* "else" on the same line as "}" that closes corresponding "if (...) {"; * multi-line comments begin with "/*\n"; * sizeof, even it is not a function, is written as "sizeof(...)"; * no need to check x?alloc() return value -- it would have died; * "if (...) { ... }" that covers the whole function body can be dedented by returning from the function early with "if (!...) return;"; * SP on each side of an operator, i.e. "a > 0", not "a>0"; Also removes stale comment describing how remove_child() used to do its thing. Signed-off-by: Junio C Hamano <gitster@pobox.com>: Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-08-17git-daemon: rewrite kindergarden, new option --max-connectionsStephen R. van den Berg1-138/+75
Get rid of the fixed array of children and make max-connections dynamic and configurable. Fix the killing code to actually kill the newest connections from duplicate IP-addresses. Avoid forking if too busy already. Signed-off-by: Stephen R. van den Berg <srb@cuci.nl> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-08-17git-daemon: Simplify dead-children reaping logicStephen R. van den Berg1-32/+23
Move almost all code out of the child_handler() into check_dead_children(). The fact that systemcalls get interrupted by signals allows us to make the SIGCHLD signal handler almost a no-op by simply running check_dead_children() right before waiting on poll(). In case some systems do not interrupt systemcalls upon signal receipt, all zombies will eventually be collected before the next poll() cycle. Signed-off-by: Stephen R. van den Berg <srb@cuci.nl> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-08-17git-daemon: use LOG_PID, simplify logging codeStephen R. van den Berg1-31/+16
Make git-daemon use LOG_PID like most daemons, instead of prepending the pid to the message ourselves, when using syslog(3). Simplify the logging code by setting stderr to line buffered, instead of building a single string and writing it out with a single write(2). Give an extra log message at the daemon start-up. Signed-off-by: Stephen R. van den Berg <srb@cuci.nl> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-08-17git-daemon: call logerror() instead of error()Stephen R. van den Berg1-2/+2
Use logerror(), not error(), so that the messages won't be lost, especially when running the daemon with its log sent to the syslog facility. Signed-off-by: Stephen R. van den Berg <srb@cuci.nl> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-08-12git-daemon: SysV needs the signal handler reinstated.Stephen R. van den Berg1-0/+1
Fixes the bug on (amongst others) Solaris that only the first child ever is reaped. Signed-off-by: Stephen R. van den Berg <srb@cuci.nl> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-07-23git daemon: avoid waking up too oftenJohannes Schindelin1-14/+11
To avoid waking up unnecessarily, a pipe is set up that is only ever written to by child_handler(), when a child disconnects, as suggested per Junio. This avoids waking up the main process every second to see if a child was disconnected. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-07-13Make usage strings dash-lessStephan Beyer1-1/+1
When you misuse a git command, you are shown the usage string. But this is currently shown in the dashed form. So if you just copy what you see, it will not work, when the dashed form is no longer supported. This patch makes git commands show the dash-less version. For shell scripts that do not specify OPTIONS_SPEC, git-sh-setup.sh generates a dash-less usage string now. Signed-off-by: Stephan Beyer <s-beyer@gmx.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-07-06git daemon: avoid calling syslog() from a signal handlerJohannes Schindelin1-20/+50
Signal handlers should never call syslog(), as that can raise signals of its own. Instead, call the syslog() from the master process. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-05-14Provide git_config with a callback-data parameterJohannes Schindelin1-2/+2
git_config() only had a function parameter, but no callback data parameter. This assumes that all callback functions only modify global variables. With this patch, every callback gets a void * parameter, and it is hoped that this will help the libification effort. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-02-27daemon: ensure that base-path is an existing directoryJohannes Sixt1-0/+8
Any request to the daemon would fail if base-path (if specified) is not a directory. We now check for this condition early. Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-02-27daemon: send more error messages to the syslogJohannes Sixt1-5/+5
There were a number of die() calls before the syslog was opened; hence, these error messages would have been sent to /dev/null in detached mode. Now we install the daemon-specific die routine before any error message is generated so that these messages go to the syslog. Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2007-11-08Style: place opening brace of a function definition at column 1Junio C Hamano1-2/+4
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2007-11-05git-daemon: fix remote port number in log entryGerrit Pape1-2/+2
The port number in struct sockaddr_in needs to be converted from network byte order to host byte order (on some architectures). Signed-off-by: Gerrit Pape <pape@smarden.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>