nfvasprintf() has a 8KB limit, but it's not relevant, as its result is
combined with other strings and added to a 1KB buffer by its caller.
That 1KB limit is not mentioned in RFC 9051, which specifies IMAP.
While 1KB is plenty for user names, passwords and mailbox names,
there's no point in limiting our commands like that. Call xstrvfmt()
instead of open-coding it and use strbuf to format the command to
send, as we need its length. Fail hard if it exceeds INT_MAX, because
socket_write() can't take more than that.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
nfsnprintf() wraps vsnprintf(3) and reports attempts to use too small a
buffer using BUG(), just like xsnprintf().
It has an extra check that makes sure the buffer size (converted to int)
is positive. vsnprintf(3) is supposed to handle a buffer size of zero
or bigger than INT_MAX just fine, so this extra comparison doesn't make
us any safer. If a platform has a broken implementation, we'd need to
work around it in our compat code.
Call xsnprintf() instead to reduce code duplication and make the caller
slightly more readable by using this more common helper.
Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Building with NO_CURL is currently broken since imap-send.c uses things
defined in "strbuf.h" wihtout including it.
The inclusion of that header was removed in eea0e59ffb (treewide: remove
unnecessary includes in source files, 2023-12-23), which failed to
notice that "strbuf.h" was transitively included in imap-send.c via
"http.h", but only if USE_CURL_FOR_IMAP_SEND is defined. Add back the
missing include. Note that it was explicitely added in 3307f7dde2
(imap-send: include strbuf.h, 2023-05-17) after a similar breakage in
ba3d1c73da (treewide: remove unnecessary cache.h includes, 2023-02-24) -
see the thread starting at [1].
It can be verified by inspection that this is the only case where a
header we include is dependent on a Makefile knob in the files modified
in eea0e59ffb.
[1] https://lore.kernel.org/git/20230517070632.71884-1-list@eworm.de/
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Remove unused header "#include".
* en/header-cleanup:
treewide: remove unnecessary includes in source files
treewide: add direct includes currently only pulled in transitively
trace2/tr2_tls.h: remove unnecessary include
submodule-config.h: remove unnecessary include
pkt-line.h: remove unnecessary include
line-log.h: remove unnecessary include
http.h: remove unnecessary include
fsmonitor--daemon.h: remove unnecessary includes
blame.h: remove unnecessary includes
archive.h: remove unnecessary include
treewide: remove unnecessary includes in source files
treewide: remove unnecessary includes from header files
Each of these were checked with
gcc -E -I. ${SOURCE_FILE} | grep ${HEADER_FILE}
to ensure that removing the direct inclusion of the header actually
resulted in that header no longer being included at all (i.e. that
no other header pulled it in transitively).
...except for a few cases where we verified that although the header
was brought in transitively, nothing from it was directly used in
that source file. These cases were:
* builtin/credential-cache.c
* builtin/pull.c
* builtin/send-pack.c
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Code clean-up around use of configuration variables.
* jk/config-cleanup:
sequencer: simplify away extra git_config_string() call
gpg-interface: drop pointless config_error_nonbool() checks
push: drop confusing configset/callback redundancy
config: use git_config_string() for core.checkRoundTripEncoding
diff: give more detailed messages for bogus diff.* config
config: use config_error_nonbool() instead of custom messages
imap-send: don't use git_die_config() inside callback
git_xmerge_config(): prefer error() to die()
config: reject bogus values for core.checkstat
A few config callbacks use their own custom messages to report an
unexpected implicit bool like:
[merge "foo"]
driver
These should just use config_error_nonbool(), so the user sees
consistent messages.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The point of git_die_config() is to let configset users mention the
file/line info for invalid config, like:
if (!git_config_get_int("foo.bar", &value)) {
if (!is_ok(value))
git_die_config("foo.bar");
}
Using it from within a config callback is unnecessary, because we can
simply return an error, at which point the config machinery will mention
the file/line of the offending variable. Worse, using git_die_config()
can actually produce the wrong location when the key is found in
multiple spots. For instance, with config like:
[imap]
host
host = foo
we'll report the line number of the "host = foo" line, but the problem
is on the implicit-bool "host" line.
We can fix it by just returning an error code.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These sites offer https versions of their content.
Using the https versions provides some protection for users.
Signed-off-by: Josh Soref <jsoref@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It's somewhat traditional to respect sites' self-identification.
Signed-off-by: Josh Soref <jsoref@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Earlier patches annotating unused parameters in imap-send missed a few
cases in code that is compiled only with NO_OPENSSL. These need to
retain the extra parameters to match the interfaces used when we compile
with openssl support.
Note in the case of socket_perror() that the function declaration and
parts of its code are shared between the two cases, and only the openssl
code looks at "sock". So we can't simply mark the parameter as always
unused. Instead, we can add a noop statement that references it. This is
ugly, but should be portable.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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
"imap-send" codepaths got cleaned up to get rid of unused
parameters.
* jk/imap-send-unused-variable-cleanup:
imap-send: drop unused fields from imap_cmd_cb
imap-send: drop unused parameter from imap_cmd_cb callback
imap-send: use server conf argument in setup_curl()
The imap_cmd_cb struct has several fields which are totally unused.
Presumably they did useful things in the upstream isync code from which
this is derived, but they don't in our more limited program. This is
particularly confusing for the "done" callback, which (as of the
previous patch) no longer matches the signature of the adjacent "cont"
callback.
Since we're unlikely to share code with isync going forward, we should
feel free to simplify the code here. Note that "done" is examined but
never set, so we can also drop a little bit of code outside of the
struct definition.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There's a generic callback mechanism for handling plus-continuation of
IMAP commands. It takes the imap_cmd struct itself as an argument. That
seems reasonable, and in a larger imap-using program it might be used.
But in imap-send, we have only one such callback (auth_cram_md5) and it
doesn't use this value, triggering -Wunused-parameter warnings.
We could just mark the parameter as UNUSED. But since this is the only
such function, and because we are not likely to share code with the
upstream isync anymore, we can just simplify the interface to remove
this parameter.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our caller passes in an imap_server_conf struct, but we ignore it
totally, and instead read the config directly from the global "server"
variable. This works OK, since our sole caller will pass in that same
global variable. But the intent seems to have been to use the passed-in
variable, as otherwise it has no purpose (and many other functions use
the same pattern).
Let's use the passed-in value, which also silences a -Wunused-parameter
warning.
It would be nice if "server" was not a global here, as we could avoid
making similar mistakes. But changing that would be a larger refactor,
as it must be accessed as a global in a few spots (e.g., filling it in
with the config callback).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Plumb "struct key_value_info" through all code paths that end in
die_bad_number(), which lets us remove the helper functions that read
analogous values from "struct config_reader". As a result, nothing reads
config_reader.config_kvi any more, so remove that too.
In config.c, this requires changing the signature of
git_configset_get_value() to 'return' "kvi" in an out parameter so that
git_configset_get_<type>() can pass it to git_config_<type>(). Only
numeric types will use "kvi", so for non-numeric types (e.g.
git_configset_get_string()), pass NULL to indicate that the out
parameter isn't needed.
Outside of config.c, config callbacks now need to pass "ctx->kvi" to any
of the git_config_<type>() functions that parse a config string into a
number type. Included is a .cocci patch to make that refactor.
The only exceptional case is builtin/config.c, where git_config_<type>()
is called outside of a config callback (namely, on user-provided input),
so config source information has never been available. In this case,
die_bad_number() defaults to a generic, but perfectly descriptive
message. Let's provide a safe, non-NULL for "kvi" anyway, but make sure
not to change the message.
Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add a new "const struct config_context *ctx" arg to config_fn_t to hold
additional information about the config iteration operation.
config_context has a "struct key_value_info kvi" member that holds
metadata about the config source being read (e.g. what kind of config
source it is, the filename, etc). In this series, we're only interested
in .kvi, so we could have just used "struct key_value_info" as an arg,
but config_context makes it possible to add/adjust members in the future
without changing the config_fn_t signature. We could also consider other
ways of organizing the args (e.g. moving the config name and value into
config_context or key_value_info), but in my experiments, the
incremental benefit doesn't justify the added complexity (e.g. a
config_fn_t will sometimes invoke another config_fn_t but with a
different config value).
In subsequent commits, the .kvi member will replace the global "struct
config_reader" in config.c, making config iteration a global-free
operation. It requires much more work for the machinery to provide
meaningful values of .kvi, so for now, merely change the signature and
call sites, pass NULL as a placeholder value, and don't rely on the arg
in any meaningful way.
Most of the changes are performed by
contrib/coccinelle/config_fn_ctx.pending.cocci, which, for every
config_fn_t:
- Modifies the signature to accept "const struct config_context *ctx"
- Passes "ctx" to any inner config_fn_t, if needed
- Adds UNUSED attributes to "ctx", if needed
Most config_fn_t instances are easily identified by seeing if they are
called by the various config functions. Most of the remaining ones are
manually named in the .cocci patch. Manual cleanups are still needed,
but the majority of it is trivial; it's either adjusting config_fn_t
that the .cocci patch didn't catch, or adding forward declarations of
"struct config_context ctx" to make the signatures make sense.
The non-trivial changes are in cases where we are invoking a config_fn_t
outside of config machinery, and we now need to decide what value of
"ctx" to pass. These cases are:
- trace2/tr2_cfg.c:tr2_cfg_set_fl()
This is indirectly called by git_config_set() so that the trace2
machinery can notice the new config values and update its settings
using the tr2 config parsing function, i.e. tr2_cfg_cb().
- builtin/checkout.c:checkout_main()
This calls git_xmerge_config() as a shorthand for parsing a CLI arg.
This might be worth refactoring away in the future, since
git_xmerge_config() can call git_default_config(), which can do much
more than just parsing.
Handle them by creating a KVI_INIT macro that initializes "struct
key_value_info" to a reasonable default, and use that to construct the
"ctx" arg.
Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We make liberal use of the strbuf API functions and types, but the
inclusion of <strbuf.h> comes indirectly by including <http.h>,
which does not happen if you build with NO_CURL.
Signed-off-by: Christian Hesse <mail@eworm.de>
Helped-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Dozens of files made use of gettext functions, without explicitly
including gettext.h. This made it more difficult to find which files
could remove a dependence on cache.h. Make C files explicitly include
gettext.h if they are using it.
However, while compat/fsmonitor/fsm-ipc-darwin.c should also gain an
include of gettext.h, it was left out to avoid conflicting with an
in-flight topic.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We had several C files include cache.h unnecessarily. Replace those
with an include of "git-compat-util.h" instead. Much like the previous
commit, these have all been verified via both ensuring that
gcc -E $SOURCE_FILE | grep '"cache.h"'
found no hits and that
make DEVELOPER=1 ${OBJECT_FILE_FOR_SOURCE_FILE}
successfully compiles without warnings.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
05cd988dce (wrapper: add a helper to generate numbers from a CSPRNG,
2022-01-17), configure openssl as the source for entropy in NON-STOP
but doesn't add the needed header or link options.
Since the only system that is configured to use openssl as a source
of entropy is NON-STOP, add the header unconditionally, and -lcrypto
to the list of external libraries.
An additional change is required to make sure a NO_OPENSSL=1 build
will be able to work as well (tested on Linux with a modified value
of CSPRNG_METHOD = openssl), and the more complex logic that allows
for compatibility with APPLE_COMMON_CRYPTO or allowing for simpler
ways to link (without libssl) has been punted for now.
Reported-by: Randall Becker <rsbecker@nexbridge.com>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Cut down a lot on the verbosity of the "server" assignment in
imap-send.c using designated initializers, only the "ssl_verify"
member was being set to a non-NULL non-0 value.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As discussed in 644de29e22 (http: drop support for curl < 7.19.4,
2021-07-30) checking against LIBCURL_VERSION_NUM isn't as reliable as
checking specific symbols present in curl, as some distros have been
known to backport features.
However, while some of the curl_easy_setopt() arguments we rely on are
macros, others are enum, and we can't assume that those that are
macros won't change into enums in the future.
So we're still going to have to check LIBCURL_VERSION_NUM, but by
doing that in one central place and using a macro definition of our
own, anyone who's backporting features can define it themselves, and
thus have access to more modern curl features that they backported,
even if they didn't bump the LIBCURL_VERSION_NUM.
More importantly, as shown in a preceding commit doing these version
checks makes for hard to read and possibly buggy code, as shown by the
bug fixed there where we were conflating base 10 for base 16 when
comparing the version.
By doing them all in one place we'll hopefully reduce the chances of
such future mistakes, furthermore it now becomes easier to see at a
glance what the oldest supported version is, which makes it easier to
reason about any future deprecation similar to the recent
e48a623dea (Merge branch 'ab/http-drop-old-curl', 2021-08-24).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Support for ancient versions of cURL library (pre 7.19.4) has been
dropped.
* ab/http-drop-old-curl:
http: rename CURLOPT_FILE to CURLOPT_WRITEDATA
http: drop support for curl < 7.19.3 and < 7.17.0 (again)
http: drop support for curl < 7.19.4
http: drop support for curl < 7.16.0
http: drop support for curl < 7.11.1
In the last commit we dropped support for curl < 7.11.1, let's
continue that and drop support for versions older than 7.16.0. This
allows us to get rid of some now-obsolete #ifdefs.
Choosing 7.16.0 is a somewhat arbitrary cutoff:
1. It came out in October of 2006, almost 15 years ago.
Besides being a nice round number, around 10 years is
a common end-of-life support period, even for conservative
distributions.
2. That version introduced the curl_multi interface, which
gives us a lot of bang for the buck in removing #ifdefs
RHEL 5 came with curl 7.15.5[1] (released in August 2006). RHEL 5's
extended life cycle program ended on 2020-11-30[1]. RHEL 6 comes with
curl 7.19.7 (released in November 2009), and RHEL 7 comes with
7.29.0 (released in February 2013).
1. http://lore.kernel.org/git/873e1f31-2a96-5b72-2f20-a5816cad1b51@jupiterrise.com
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Many "printf"-like helper functions we have have been annotated
with __attribute__() to catch placeholder/parameter mismatches.
* ab/attribute-format:
advice.h: add missing __attribute__((format)) & fix usage
*.h: add a few missing __attribute__((format))
*.c static functions: add missing __attribute__((format))
sequencer.c: move static function to avoid forward decl
*.c static functions: don't forward-declare __attribute__
Add missing __attribute__((format)) function attributes to various
"static" functions that take printf arguments.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When looking for things that hardcoded a non-zero "hint" parameter to
strbuf_fread() I discovered that since f2561fda36 (Add git-imap-send,
derived from isync 1.0.1., 2006-03-10) we've been passing a hardcoded
4096 in imap-send.c to read stdin.
Since we're not doing anything unusual here let's use a less verbose
pattern used in a lot of other places (the hint of "0" will default to
8192). We don't need to take a FILE * here either, so we can use "0"
instead of "stdin". While we're at it improve the error message if we
can't read the input to use error_errno().
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
git imap-send does not parse the default git config settings and thus ignore
core.askpass value.
Rewrite config parsing to support core settings.
Reported-by: Philippe Blain <levraiphilippeblain@gmail.com>
Signed-off-by: Nicolas Morey-Chaisemartin <nmoreychaisemartin@suse.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
Whenever GIT_CURL_VERBOSE is set, teach Git to behave as if
GIT_TRACE_CURL=1 and GIT_TRACE_CURL_NO_DATA=1 is set, instead of setting
CURLOPT_VERBOSE.
This is to prevent inadvertent revelation of sensitive data. In
particular, GIT_CURL_VERBOSE redacts neither the "Authorization" header
nor any cookies specified by GIT_REDACT_COOKIES.
Unifying the tracing mechanism also has the future benefit that any
improvements to the tracing mechanism will benefit both users of
GIT_CURL_VERBOSE and GIT_TRACE_CURL, and we do not need to remember to
implement any improvement twice.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In d8193743e0 (usage.c: add BUG() function, 2017-05-12), a new macro
was introduced to use for reporting bugs instead of die(). It was then
subsequently used to convert one single caller in 588a538ae5
(setup_git_env: convert die("BUG") to BUG(), 2017-05-12).
The cover letter of the patch series containing this patch
(cf 20170513032414.mfrwabt4hovujde2@sigill.intra.peff.net) is not
terribly clear why only one call site was converted, or what the plan
is for other, similar calls to die() to report bugs.
Let's just convert all remaining ones in one fell swoop.
This trick was performed by this invocation:
sed -i 's/die("BUG: /BUG("/g' $(git grep -l 'die("BUG' \*.c)
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is more consistent with the project style. The majority of Git's
source files use dashes in preference to underscores in their file names.
Signed-off-by: Stefan Beller <sbeller@google.com>
Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When trying to send a patch using 'imap-send' with 'curl' and the
following configuration:
[imap]
folder = "[Gmail]/Drafts"
host = imaps://imap.gmail.com
port = 993
sslverify = false
results in the following error,
curl_easy_perform() failed: URL using bad/illegal format or missing URL
This is a consequence of not URI-encoding the folder portion of
the URL which contains characters such as '[' which are not
allowed in a URI. According to RFC3986, these characters should be
URI-encoded.
So, URI-encode the folder before adding it to the URI to ensure it doesn't
contain characters that aren't allowed in a URI.
Reported-by: Doron Behar <doron.behar@gmail.com>
Signed-off-by: Nicolas Morey-Chaisemartin <NMoreyChaisemartin@suse.com>
Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Error checking in "git imap-send" for empty response has been
improved.
* rs/imap-send-next-arg-fix:
imap-send: handle missing response codes gracefully
imap-send: handle NULL return of next_arg()
The mailing address for the FSF has changed over the years. Rather than
updating the address across all files, refer readers to gnu.org, as the
GNU GPL documentation now suggests for license notices. The mailing
address is retained in the full license files (COPYING and LGPL-2.1).
The old address is still present in t/diff-lib/COPYING. This is
intentional, as the file is used in tests and the contents are not
expected to change.
Signed-off-by: Todd Zullinger <tmz@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Response codes are optional. Exit parse_response_code() early if it's
passed a NULL string, indicating that we reached the end of the reply.
This avoids dereferencing said NULL pointer.
Noticed-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
next_arg() returns NULL if it runs out of arguments. Most call sites
already handle that gracefully. Check in the remaining cases as well.
Replace the NULL pointer with an empty string at the bottom of
get_cmd_result() -- it's nicely reported as an unexpected response a
few lines down. Error out explicitly at the remaining sites.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe
Junio C Hamano
Philippe Blain
Elijah Newren
Jeff King
Josh Soref
Calvin Wan
Glen Choo
Christian Hesse
Carlo Marcelo Arenas Belón
Ævar Arnfjörð Bjarmason
Nicolas Morey-Chaisemartin
Jonathan Tan
Nguyễn Thái Ngọc Duy
Johannes Schindelin
Stefan Beller
Brandon Williams
Todd Zullinger