Commit Graph

249 Commits (86c9c14eb9c7bfa20efd8d65f1aaa685282b7221)

Author SHA1 Message Date
Jörg Thalheim d30bf28d09 imap-send: improve error messages with configuration hints
Replace basic error messages with more helpful ones that guide users
on how to resolve configuration issues. When imap.host or imap.folder
are missing, provide the exact git config commands needed to fix the
problem, along with examples of typical values.

Use the advise() API to display hints in a multi-line format with
proper "hint:" prefixes for each line.

Signed-off-by: Jörg Thalheim <joerg@thalheim.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-20 14:14:44 -07:00
Jörg Thalheim 1d304ce130 imap-send: fix confusing 'store' terminology in error message
The error message 'no imap store specified' is misleading because
it refers to 'store' when the actual missing configuration is
'imap.folder'. Update the message to use the correct terminology
that matches the configuration variable name.

This reduces confusion for users who might otherwise look for
non-existent 'imap.store' configuration when they see this error.

Signed-off-by: Jörg Thalheim <joerg@thalheim.io>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-20 14:14:43 -07:00
Junio C Hamano 0ed16dc3c4 Merge branch 'ag/imap-send-resurrection' into jt/imap-send-message-fix
* ag/imap-send-resurrection:
  imap-send: fix minor mistakes in the logs
  imap-send: display the destination mailbox when sending a message
  imap-send: display port alongwith host when git credential is invoked
  imap-send: add ability to list the available folders
  imap-send: enable specifying the folder using the command line
  imap-send: add PLAIN authentication method to OpenSSL
  imap-send: add support for OAuth2.0 authentication
  imap-send: gracefully fail if CRAM-MD5 authentication is requested without OpenSSL
  imap-send: fix memory leak in case auth_cram_md5 fails
  imap-send: fix bug causing cfg->folder being set to NULL
2025-06-20 08:29:34 -07:00
Aditya Garg 5ec81b33b0 imap-send: fix minor mistakes in the logs
Some minor mistakes have been found in the logs. Most of them include
error messages starting with a capital letter, and ending with a period.
Abbreviations like "IMAP" and "OK" should also be in uppercase. Another
mistake was that the error message showing unknown authentication
mechanism used was displaying the host rather than the mechanism in the
logs. Fix them.

Signed-off-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-20 08:11:18 -07:00
Aditya Garg bf22c370b9 imap-send: display the destination mailbox when sending a message
Whenever we sent a message using the `imap-send` command, it would
display a log showing the number of messages which are to be sent.
For example:

    sending 1 message
     100% (1/1) done

This had been made more informative by adding the name of the destination
folder as well:

    Sending 1 message to Drafts folder...
     100% (1/1) done

Signed-off-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-20 08:11:17 -07:00
Aditya Garg 2dacd35731 imap-send: display port alongwith host when git credential is invoked
When requesting for passsword, git credential helper used to display
only the host name. For example:

    Password for 'imaps://gargaditya08%40live.com@outlook.office365.com':

Now, it will display the port along with the host name:

    Password for 'imaps://gargaditya08%40live.com@outlook.office365.com:993':

This has been done to make credential helpers more specific for ports.
Also, this behaviour will also mimic git send-email, which displays
the port along with the host name when requesting for a password.

Signed-off-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-20 08:11:17 -07:00
Aditya Garg 067a91b03f imap-send: add ability to list the available folders
Various IMAP servers have different ways to name common folders.
For example, the folder where all deleted messages are stored is often
named "[Gmail]/Trash" on Gmail servers, and "Deleted" on Outlook.
Similarly, the Drafts folder is simply named "Drafts" on Outlook, but
on Gmail it is named "[Gmail]/Drafts".

This commit adds a `--list` command to the `imap-send` tool that lists
the available folders on the IMAP server, allowing users to see
which folders are available and how they are named. A sample output
looks like this when run against a Gmail server:

    Fetching the list of available folders...
    * LIST (\HasNoChildren) "/" "INBOX"
    * LIST (\HasChildren \Noselect) "/" "[Gmail]"
    * LIST (\All \HasNoChildren) "/" "[Gmail]/All Mail"
    * LIST (\Drafts \HasNoChildren) "/" "[Gmail]/Drafts"
    * LIST (\HasNoChildren \Important) "/" "[Gmail]/Important"
    * LIST (\HasNoChildren \Sent) "/" "[Gmail]/Sent Mail"
    * LIST (\HasNoChildren \Junk) "/" "[Gmail]/Spam"
    * LIST (\Flagged \HasNoChildren) "/" "[Gmail]/Starred"
    * LIST (\HasNoChildren \Trash) "/" "[Gmail]/Trash"

For OpenSSL, this is achived by running the 'IMAP LIST' command and
parsing the response. This command is specified in RFC6154:
https://datatracker.ietf.org/doc/html/rfc6154#section-5.1

For libcurl, the example code published in the libcurl documentation
is used to implement this functionality:
https://curl.se/libcurl/c/imap-list.html

Signed-off-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-20 08:11:17 -07:00
Aditya Garg 3168514e6b imap-send: enable specifying the folder using the command line
Some users may very often want to imap-send messages to a folder
other than the default set in the config. Add a command line
argument for the same.

While at it, fix minor mark-up inconsistencies in the existing
documentation text.

Signed-off-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-20 08:11:17 -07:00
Aditya Garg ea8681e3a4 imap-send: add PLAIN authentication method to OpenSSL
The current implementation for PLAIN in imap-send works just fine
if using curl, but if attempted to use for OpenSSL, it is treated
as an invalid mechanism. The default implementation for OpenSSL is
IMAP LOGIN command rather than AUTH PLAIN. Since AUTH PLAIN is
still used today by many email providers in form of app passwords,
lets add an implementation that can use AUTH PLAIN if specified.

Signed-off-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-20 08:11:17 -07:00
Aditya Garg 103d7b12b7 imap-send: add support for OAuth2.0 authentication
OAuth2.0 is a new way of authentication supported by various email providers
these days. OAUTHBEARER and XOAUTH2 are the two most common mechanisms used
for OAuth2.0. OAUTHBEARER is described in RFC5801[1] and RFC7628[2], whereas
XOAUTH2 is Google's proprietary mechanism (See [3]).

[1]: https://datatracker.ietf.org/doc/html/rfc5801
[2]: https://datatracker.ietf.org/doc/html/rfc7628
[3]: https://developers.google.com/workspace/gmail/imap/xoauth2-protocol#initial_client_response

Signed-off-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-20 08:11:16 -07:00
Aditya Garg b9e766604d imap-send: gracefully fail if CRAM-MD5 authentication is requested without OpenSSL
Unlike PLAIN, XOAUTH2 and OAUTHBEARER, CRAM-MD5 authentication is not
supported by libcurl and requires OpenSSL. If the user tries to use
CRAM-MD5 authentication without OpenSSL, the previous behaviour was to
attempt to authenticate and fail with a die(error). Handle this in a
better way by first checking if OpenSSL is available and then attempting
to authenticate. If OpenSSL is not available, print an error message and
exit gracefully.

Signed-off-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-20 08:11:16 -07:00
Aditya Garg ac4e02c503 imap-send: fix memory leak in case auth_cram_md5 fails
This patch fixes a memory leak by running free(response) in case
auth_cram_md5 fails.

Signed-off-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-20 08:11:16 -07:00
Aditya Garg 44ba4b0bbb imap-send: fix bug causing cfg->folder being set to NULL
6d1f198f34 (imap-send: fix leaking memory in `imap_server_conf`, 2024-06-07)
resulted a change in static int git_imap_config which resulted in cfg->folder
being incorrectly set to NULL in case imap.user, imap.pass, imap.tunnel and
imap.authmethod were defined. Because of this, since Git 2.46.0,
git-imap-send is not usable at all. The bug seems to have been unnoticed for
a long time, likely due to better options like git-send-email.

Signed-off-by: Aditya Garg <gargaditya08@live.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-20 08:11:16 -07:00
Jeff King 30325e23ba curl: fix integer variable typechecks with curl_easy_setopt()
As discussed in the previous commit, we should be passing long integers,
not regular ones, to curl_easy_setopt(), and compiling against curl 8.14
loudly complains if we don't.

That patch fixed integer constants by adding an "L". This one deals with
actual variables.

Arguably these variables could just be declared as "long" in the first
place. But it's actually kind of awkward due to other code which uses
them:

  - port is conceptually a short, and we even call htons() on it (though
    weirdly it is defined as a regular int).

  - ssl_verify is conceptually a bool, and we assign to it from
    git_config_bool().

So I think we could probably switch these out for longs without hurting
anything, but it just feels a bit weird. Doubly so because if you don't
set USE_CURL_FOR_IMAP_SEND set, then the current types are fine!

So let's just cast these to longs in the curl calls, which makes what's
going on obvious. There aren't that many spots to modify (and as you can
see from the context, we already have some similar casts).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-06-04 14:17:53 -07:00
Johannes Schindelin fa8cd29676 imap-send: explicitly verify the peer certificate
It is a bug to obtain the peer certificate without verifying it.

Having said that, from my reading of
https://www.openssl.org/docs/man1.1.1/man3/SSL_set_verify.html, it would
appear that Git is saved by the fact that it calls
`SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL)` already early on.

In other words, that `SSL_VERIFY_PEER` combined with the `NULL`
parameter (i.e. no overridden callback) would _already_ verify the peer
certificate.  The fact that we later call `SSL_get_peer_certificate()`
is mistaken by CodeQL to mean that that peer certificate still needs to
be verified, but that had already happened at that point.

Nevertheless, it is better to verify the peer certificate explicitly
than to rely on some side effect that is really hard to reason about
(and that took me more than one business day to analyze fully). It also
makes it easier for static analyzers to validate the correctness of the
code.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-03-25 15:48:58 -07:00
Patrick Steinhardt 6c27d22276 credential: stop using `the_repository`
Stop using `the_repository` in the "credential" subsystem by passing in
a repository when filling, approving or rejecting credentials.

Adjust callers accordingly by using `the_repository`. While there may be
some callers that have a repository available in their context, this
trivial conversion allows for easier verification and bubbles up the use
of `the_repository` by one level.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-12-18 10:44:31 -08:00
Patrick Steinhardt 41f43b8243 global: mark code units that generate warnings with `-Wsign-compare`
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-12-06 20:20:02 +09:00
Junio C Hamano 4c1b7e364e Merge branch 'bc/drop-ancient-libcurl-and-perl'
Drop support for older libcURL and Perl.

* bc/drop-ancient-libcurl-and-perl:
  gitweb: make use of s///r
  Require Perl 5.26.0
  INSTALL: document requirement for libcurl 7.61.0
  git-curl-compat: remove check for curl 7.56.0
  git-curl-compat: remove check for curl 7.53.0
  git-curl-compat: remove check for curl 7.52.0
  git-curl-compat: remove check for curl 7.44.0
  git-curl-compat: remove check for curl 7.43.0
  git-curl-compat: remove check for curl 7.39.0
  git-curl-compat: remove check for curl 7.34.0
  git-curl-compat: remove check for curl 7.25.0
  git-curl-compat: remove check for curl 7.21.5
2024-12-04 10:14:48 +09:00
Usman Akinyemi e226ba81a2 imap: replace atoi() with strtol_i() for UIDVALIDITY and UIDNEXT parsing
Replace unsafe uses of atoi() with strtol_i() to improve error handling
when parsing UIDVALIDITY, UIDNEXT, and APPENDUID in IMAP commands.
Invalid values, such as those with letters, now trigger error messages and
prevent malformed status responses.
I did not add any test for this commit as we do not have any test
for git-imap-send(1) at this point.

Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-24 14:03:44 -04:00
brian m. carlson 6545b26eeb git-curl-compat: remove check for curl 7.34.0
libcurl 7.34.0 was released in December 2013, which is well over ten
years ago, and no major operating system vendor is still providing
security support for it.  Debian 8 and Ubuntu 14.04, both of which are
out of mainstream security support, have supported a newer version, and
RHEL 8, which is still in support, also has a newer version.

Remove the check for this version and use this functionality
unconditionally.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2024-10-23 16:16:35 -04:00
Junio C Hamano 4c22e57bab Merge branch 'jk/no-openssl-with-openssl-sha1'
The "imap-send" now allows to be compiled with NO_OPENSSL and
OPENSSL_SHA1 defined together.

* jk/no-openssl-with-openssl-sha1:
  imap-send: handle NO_OPENSSL even when openssl exists
2024-09-20 11:16:31 -07:00
Jeff King 997950a750 imap-send: handle NO_OPENSSL even when openssl exists
If NO_OPENSSL is defined, then imap-send.c defines a fallback "SSL"
type, which is just a void pointer that remains NULL. This works, but it
has one problem: it is using the type name "SSL", which conflicts with
the upstream name, if some other part of the system happens to include
openssl. For example:

  $ make NO_OPENSSL=Nope OPENSSL_SHA1=Yes imap-send.o
      CC imap-send.o
  imap-send.c:35:15: error: conflicting types for ‘SSL’; have ‘void *’
     35 | typedef void *SSL;
        |               ^~~
  In file included from /usr/include/openssl/evp.h:26,
                   from sha1/openssl.h:4,
                   from hash.h:10,
                   from object.h:4,
                   from commit.h:4,
                   from refs.h:4,
                   from setup.h:4,
                   from imap-send.c:32:
  /usr/include/openssl/types.h:187:23: note: previous declaration of ‘SSL’ with type ‘SSL’ {aka ‘struct ssl_st’}
    187 | typedef struct ssl_st SSL;
        |                       ^~~
  make: *** [Makefile:2761: imap-send.o] Error 1

This is not a terribly common combination in practice:

  1. Why are we disabling openssl support but still using its sha1? The
     answer is that you may use the same build options across many
     versions, and some older versions of Git no longer build with
     modern versions of openssl.

  2. Why are we using a totally unsafe sha1 that does not detect
     collisions? You're right, we shouldn't. But in preparation for
     using unsafe sha1 for non-cryptographic checksums, it would be nice
     to be able to turn it on without hassle.

We can make this work by adjusting the way imap-send handles its
fallback. One solution is something like this:

  #ifdef NO_OPENSSL
  #define git_SSL void *
  #else
  #define git_SSL SSL
  #endif

But we can observe that we only need this definition in one spot: the
struct which holds the variable. So rather than play around with macros
that may cause unexpected effects, we can just directly use the correct
type in that struct.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-09-12 10:24:51 -07:00
Junio C Hamano 2b30d66c43 Merge branch 'jk/mark-unused-parameters'
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-26 11:32:23 -07:00
Jeff King 4647f24302 imap-send: mark unused parameter in ssl_socket_connect() fallback
Commit cea1ff7f1f (imap-send: drop global `imap_server_conf` variable,
2024-06-07) added an imap_server_conf parameter to several functions.
But when compiled with NO_OPENSSL, the ssl_socket_connect() fallback
just returns immediately, so its parameters all need to be annotated to
avoid triggering -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-08-17 09:46:10 -07:00
Patrick Steinhardt 219de841d9 global: prepare for hiding away repo-less config functions
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>
2024-08-13 10:01:05 -07:00
Jeff King e3d2364c45 imap-send: free all_msgs strbuf in "out" label
We read stdin into a strbuf, but most code paths never release it,
causing a leak (albeit a minor one, as we leak only when exiting from
the main function of the program).

Commit 56f4f4a29d (imap-send: minimum leakfix, 2024-06-04) did the
minimum to plug the one instance we see in the test suite, when we read
an empty input. But it was sufficient only because aside from this noop
invocation, we don't test imap-send at all!

The right spot to free is in the "out" label, which is hit by all code
paths before leaving the function. We couldn't do that in 56f4f4a29d
because there was no unified exit path. That came separately in
3aca5f7fb0 (imap-send: fix leaking memory in `imap_server_conf`,
2024-06-04), which cleaned up many other leaks (but not this one).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07 10:32:53 -07:00
Junio C Hamano f5598fcb7b Merge branch 'jc/t1517-more' into jk/imap-send-plug-all-msgs-leak
* jc/t1517-more:
  imap-send: minimum leakfix
  t1517: more coverage for commands that work without repository
2024-06-07 10:32:20 -07:00
Patrick Steinhardt 6d1f198f34 imap-send: fix leaking memory in `imap_server_conf`
We never free any of the config strings that we populate into the
`struct imap_server_conf`. Fix this by creating a common exit path where
we can free resources.

While at it, drop the unused member `imap_server_conf::name`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07 10:30:55 -07:00
Patrick Steinhardt cea1ff7f1f imap-send: drop global `imap_server_conf` variable
In "imap-send.c", we have a global `sturct imap_server_conf` variable
that keeps track of the configuration of the IMAP server. This variable
is being populated mostly via the Git configuration.

Refactor the code to allocate the structure on the stack instead of
having it globally. This change allows us to track its lifetime more
closely.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07 10:30:54 -07:00
Patrick Steinhardt b567004b4b global: improve const correctness when assigning string constants
We're about to enable `-Wwrite-strings`, which changes the type of
string constants to `const char[]`. Fix various sites where we assign
such constants to non-const variables.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-07 10:30:48 -07:00
Junio C Hamano 56f4f4a29d imap-send: minimum leakfix
EVen with the minimum "no-op" invocation t1517 makes, "git imap-send"
leaks an empty strbuf it used to read a 0-byte string into.

There are a few other topics cooking in 'next' that plugs many
other leaks in this program, so let's minimally fix this one, barely
enough to make CI pass, leaving the rest for the other topic.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-06-04 11:48:20 -07:00
Patrick Steinhardt 1b261c20ed config: clarify memory ownership in `git_config_string()`
The out parameter of `git_config_string()` is a `const char **` even
though we transfer ownership of memory to the caller. This is quite
misleading and has led to many memory leaks all over the place. Adapt
the parameter to instead be `char **`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-05-27 11:20:00 -07:00
Junio C Hamano c5c9acf77d Merge branch 'bc/credential-scheme-enhancement'
The credential helper protocol, together with the HTTP layer, have
been enhanced to support authentication schemes different from
username & password pair, like Bearer and NTLM.

* bc/credential-scheme-enhancement:
  credential: add method for querying capabilities
  credential-cache: implement authtype capability
  t: add credential tests for authtype
  credential: add support for multistage credential rounds
  t5563: refactor for multi-stage authentication
  docs: set a limit on credential line length
  credential: enable state capability
  credential: add an argument to keep state
  http: add support for authtype and credential
  docs: indicate new credential protocol fields
  credential: add a field called "ephemeral"
  credential: gate new fields on capability
  credential: add a field for pre-encoded credentials
  http: use new headers for each object request
  remote-curl: reset headers on new request
  credential: add an authtype field
2024-05-08 10:18:44 -07:00
brian m. carlson ca9ccbf674 credential: gate new fields on capability
We support the new credential and authtype fields, but we lack a way to
indicate to a credential helper that we'd like them to be used.  Without
some sort of indication, the credential helper doesn't know if it should
try to provide us a username and password, or a pre-encoded credential.
For example, the helper might prefer a more restricted Bearer token if
pre-encoded credentials are possible, but might have to fall back to
more general username and password if not.

Let's provide a simple way to indicate whether Git (or, for that matter,
the helper) is capable of understanding the authtype and credential
fields.  We send this capability when we generate a request, and the
other side may reply to indicate to us that it does, too.

For now, don't enable sending capabilities for the HTTP code.  In a
future commit, we'll introduce appropriate handling for that code,
which requires more in-depth work.

The logic for determining whether a capability is supported may seem
complex, but it is not.  At each stage, we emit the capability to the
following stage if all preceding stages have declared it.  Thus, if the
caller to git credential fill didn't declare it, then we won't send it
to the helper, and if fill's caller did send but the helper doesn't
understand it, then we won't send it on in the response.  If we're an
internal user, then we know about all capabilities and will request
them.

For "git credential approve" and "git credential reject", we set the
helper capability before calling the helper, since we assume that the
input we're getting from the external program comes from a previous call
to "git credential fill", and thus we'll invoke send a capability to the
helper if and only if we got one from the standard input, which is the
correct behavior.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-04-16 22:39:06 -07:00
René Scharfe 21b5821acd imap-send: increase command size limit
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>
2024-04-15 11:34:17 -07:00
René Scharfe 39bb692152 imap-send: use xsnprintf to format command
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>
2024-04-02 10:29:34 -07:00
Junio C Hamano 33d03f61b9 Merge branch 'pb/imap-send-wo-curl-build-fix'
Build fix.

* pb/imap-send-wo-curl-build-fix:
  imap-send: add missing "strbuf.h" include under NO_CURL
2024-02-08 13:20:34 -08:00
Philippe Blain 30bced3a67 imap-send: add missing "strbuf.h" include under NO_CURL
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>
2024-02-01 18:20:41 -08:00
Junio C Hamano 492ee03f60 Merge branch 'en/header-cleanup'
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
2024-01-08 14:05:15 -08:00
Elijah Newren eea0e59ffb treewide: remove unnecessary includes in source 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>
2023-12-26 12:04:31 -08:00
Junio C Hamano 66e959f431 Merge branch 'jk/config-cleanup'
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
2023-12-20 10:14:55 -08:00
Jeff King 92cecce0de config: use config_error_nonbool() instead of custom messages
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>
2023-12-09 08:26:22 +09:00
Jeff King 0dda4ce9f6 imap-send: don't use git_die_config() inside callback
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>
2023-12-09 08:26:21 +09:00
Josh Soref d05b08cd52 doc: switch links to https
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>
2023-11-26 10:07:05 +09:00
Josh Soref 65175d9ea2 doc: update links to current pages
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>
2023-11-26 10:07:05 +09:00
Jeff King 2c3c3d88fc imap-send: mark unused parameters with NO_OPENSSL
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>
2023-08-29 17:56:26 -07:00
Junio C Hamano ce481ac8b3 Merge branch 'cw/compat-util-header-cleanup'
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-17 11:30:42 -07:00
Junio C Hamano daa2589b63 Merge branch 'jk/imap-send-unused-variable-cleanup'
"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()
2023-07-14 10:46:07 -07:00
Calvin Wan da9502ff4d treewide: remove unnecessary includes for wrapper.h
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-07-05 11:41:59 -07:00
Jeff King d378637d2f imap-send: drop unused fields from imap_cmd_cb
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>
2023-07-05 10:16:53 -07:00