If <msg> or <patch> files can't be opened, then mailinfo() returns an
error before it even initializes mi->p_hdr_data or mi->s_hdr_data.
When cmd_mailinfo() then calls clear_mailinfo(), we dereference the
NULL pointers trying to free their contents.
Signed-off-by: Juan F. Codagnone <jcodagnone@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git mailinfo" was loose in decoding quoted printable and produced
garbage when the two letters after the equal sign are not
hexadecimal. This has been fixed.
* rs/mailinfo-qp-decode-fix:
mailinfo: don't decode invalid =XY quoted-printable sequences
Decode =XY in quoted-printable segments only if X and Y are hexadecimal
digits, otherwise just copy them. That's at least better than
interpreting negative results from hexval() as a character.
Reported-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>
Gcc 7 adds -Wimplicit-fallthrough, which can warn when a
switch case falls through to the next case. The general idea
is that the compiler can't tell if this was intentional or
not, so you should annotate any intentional fall-throughs as
such, leaving it to complain about any unannotated ones.
There's a GNU __attribute__ which can be used for
annotation, but of course we'd have to #ifdef it away on
non-gcc compilers. Gcc will also recognize
specially-formatted comments, which matches our current
practice. Let's extend that practice to all of the
unannotated sites (which I did look over and verify that
they were behaving as intended).
Ideally in each case we'd actually give some reasons in the
comment about why we're falling through, or what we're
falling through to. And gcc does support that with
-Wimplicit-fallthrough=2, which relaxes the comment pattern
matching to anything that contains "fallthrough" (or a
variety of spelling variants). However, this isn't the
default for -Wimplicit-fallthrough, nor for -Wextra. In the
name of simplicity, it's probably better for us to support
the default level, which requires "fallthrough" to be the
only thing in the comment (modulo some window dressing like
"else" and some punctuation; see the gcc manual for the
complete set of patterns).
This patch suppresses all warnings due to
-Wimplicit-fallthrough. We might eventually want to add that
to the DEVELOPER Makefile knob, but we should probably wait
until gcc 7 is more widely adopted (since earlier versions
will complain about the unknown warning type).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Clean up at the end and jump there instead of returning early.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A common pattern to free a piece of memory and assign NULL to the
pointer that used to point at it has been replaced with a new
FREE_AND_NULL() macro.
* ab/free-and-null:
*.[ch] refactoring: make use of the FREE_AND_NULL() macro
coccinelle: make use of the "expression" FREE_AND_NULL() rule
coccinelle: add a rule to make "expression" code use FREE_AND_NULL()
coccinelle: make use of the "type" FREE_AND_NULL() rule
coccinelle: add a rule to make "type" code use FREE_AND_NULL()
git-compat-util: add a FREE_AND_NULL() wrapper around free(ptr); ptr = NULL
Fix configuration codepath to pay proper attention to commondir
that is used in multi-worktree situation, and isolate config API
into its own header file.
* bw/config-h:
config: don't implicitly use gitdir or commondir
config: respect commondir
setup: teach discover_git_directory to respect the commondir
config: don't include config.h by default
config: remove git_config_iter
config: create config.h
Apply the result of the just-added coccinelle rule. This manually
excludes a few occurrences, mostly things that resulted in many
FREE_AND_NULL() on one line, that'll be manually fixed in a subsequent
change.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
While POSIX states that it is okay to pass EOF to isspace() (and it seems
to be implied that EOF should *not* be treated as whitespace), and also to
pass EOF to ungetc() (which seems to be intended to fail without buffering
the character), it is much better to handle these cases explicitly. Not
only does it reduce head-scratching (and helps static analysis avoid
reporting false positives), it also lets us handle files containing
nothing but whitespace by erroring out.
Reported via Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If a patch e-mail had its first paragraph after an in-body header
indented (even after a blank line after the in-body header line),
the indented line was mistook as a continuation of the in-body
header. This has been fixed.
* lt/mailinfo-in-body-header-continuation:
mailinfo: fix in-body header continuations
An empty line should stop any pending in-body headers, and start the
actual body parsing.
This also modifies the original test for the in-body headers to actually
have a real commit body that starts with spaces, and changes the test to
check that the long line matches _exactly_, and doesn't get extra data
from the body.
Fixes:6b4b013f1884 ("mailinfo: handle in-body header continuations")
Cc: Jonathan Tan <jonathantanmy@google.com>
Cc: Jeff King <peff@peff.net>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since 6b4b013f18 (mailinfo: handle in-body header continuations,
2016-09-20, v2.11.0) mailinfo.c has contained new code with an
assert of the form:
assert(call_a_function(...))
The function in question, check_header, has side effects. This
means that when NDEBUG is defined during a release build the
function call is omitted entirely, the side effects do not
take place and tests (fortunately) start failing.
Since the only time that mi->inbody_header_accum is appended to is
in check_inbody_header, and appending onto a blank
mi->inbody_header_accum always happens when is_inbody_header is
true, this guarantees a prefix that causes check_header to always
return true.
Therefore replace the assert with an if !check_header + DIE
combination to reflect this.
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Jeff King <peff@peff.net>
Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
An author name, that spelled a backslash-quoted double quote in the
human readable part "My \"double quoted\" name", was not unquoted
correctly while applying a patch from a piece of e-mail.
* kd/mailinfo-quoted-string:
mailinfo: unescape quoted-pair in header fields
t5100-mailinfo: replace common path prefix with variable
rfc2822 has provisions for quoted strings in structured header fields,
but also allows for escaping these with so-called quoted-pairs.
The only thing git currently does is removing exterior quotes, but
quotes within are left alone.
Remove exterior quotes and remove escape characters so that they don't
show up in the author field.
Signed-off-by: Kevin Daudt <me@ikke.info>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Mailinfo currently handles multi-line headers, but it does not handle
multi-line in-body headers. Teach it to handle such headers, for
example, for this input:
From: author <author@example.com>
Date: Fri, 9 Jun 2006 00:44:16 -0700
Subject: a very long
broken line
Subject: another very long
broken line
interpret the in-body subject to be "another very long broken line"
instead of "another very long".
An existing test (t/t5100/msg0015) has an indented line immediately
after an in-body header - it has been modified to reflect the new
functionality.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The is_scissors_line takes a struct strbuf * when a char * would
suffice. Make it take char *.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The check_header function contains logic specific to in-body headers,
although it is invoked during both the processing of actual headers and
in-body headers. Separate out the in-body header part into its own
function.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
handle_message_id() duplicates the contents of the strbuf that is passed
to it. Its only caller proceeds to release the strbuf immediately after
that. Reuse it instead and make that change of object ownership more
obvious by inlining this short function.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
git_mailinfo_config() expects "struct mailinfo *". But in
setup_mailinfo(), "mi" is already "struct mailinfo *". &mi would make
it "struct mailinfo **" and git_mailinfo_config() would damage some
other memory when it assigns some value to mi->use_scissors.
This is caught by t4150.20. git_mailinfo_config() breaks
mi->name.alloc and makes strbuf_release() in clear_mailinfo() attempt
to free strbuf_slopbuf.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The top-level mailinfo() would instead punt when the code in the
deeper part of the callchain detects an unrecoverable error in the
input.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of dying in convert_to_utf8(), just report an error and let
the callers handle it. Between the two callers:
- decode_header() silently punts when it cannot parse a broken
RFC2047 encoded text (e.g. when it sees anything other than B or
Q after it sees "=?<charset>") by jumping to release_return,
returning the string it successfully parsed out so far, to the
caller. A piece of string that convert_to_utf8() cannot handle
can be treated the same way.
- handle_commit_msg() doesn't cope with a malformed line well, so
die there for now. We'll lift this even higher in later changes
in this series.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the bulk of the code from builtin/mailinfo.c to mailinfo.c
so that new callers can start calling mailinfo() directly.
Note that a few calls to exit() and die() need to be cleaned up
for the API to be truly useful, which will come in later steps.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It was pointed out that the current behaviour might mispart a patch comment
so remove this behaviour for now.
[jc: this fixes "From: line in the middle" check in t5100 test.]
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Junio C Hamano <junkio@cox.net>
We exited prematurely from header parsing loop when the header
field did not have a space after the colon but we insisted on
it, and we got the check wrong because we forgot that we strip
the trailing whitespace before we do the check.
The space after the colon is not even required by RFC2822, so
stop requiring it. While we are at it, the header line is
specified to be more strict than "anything with a colon in it"
(there must be one or more characters before the colon, and they
must not be controls, SP or non US-ASCII), so implement that
check as well, lest we mistakenly think something like:
Bogus not a header line: this is not.
as a header line.
Signed-off-by: Junio C Hamano <junkio@cox.net>
- handle_from is fixed to not mangle it's input line.
- Then handle_inbody_header is allowed to look in
the body of a commit message for additional headers
that we haven't already seen.
This allows patches with all of the right information in
unfortunate places to be imported.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Junio C Hamano <junkio@cox.net>
Only count lines of the form '^.*: ' and '^From ' as email
header lines.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Junio C Hamano <junkio@cox.net>
This prepares for detecting non-email patches that don't have
mail headers. In which case we have already read the first
line so handle_body should not ignore it.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Junio C Hamano <junkio@cox.net>
- Move handle_info into main so it is called once
after everything has been parsed. This allows the removal
of a static variable and removes two duplicate calls.
- Move parsing of inbody headers into handle_commit.
This means we parse the in-body headers after we have decoded
the character set, and it removes code duplication between
handle_multipart_one_part and handle_body.
- Change the flag indicating that we have seen an in body
prefix header into another bit in seen.
This is a little more general and allows the possibility of parsing
in body headers after the body message has begun.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Junio C Hamano <junkio@cox.net>
B and Q decoding is not appropriate for in body headers, so move
it up to where we explicitly know we have a real email header.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Junio C Hamano <junkio@cox.net>
Currently we only use the return value from read_one_header line
to tell if the line we have read is a header or not. So make
it a flag. This paves the way for better email detection.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Junio C Hamano <junkio@cox.net>
Sometimes people just include the whole format-patch output in
the commit e-mail. Detect it and skip the bogus ">From " line.
Signed-off-by: Junio C Hamano <junkio@cox.net>
Quoted-Printable (RFC 2045) and the "Q" encoding (RFC 2047) are
subtly different; the latter is used on the mail header and an
underscore needs to be decoded to 0x20.
Signed-off-by: Junio C Hamano <junkio@cox.net>
Systems using some uClibc versions do not properly support
iconv stuff. This patch allows Git to be built on those
systems by passing NO_ICONV=YesPlease to make. The only
drawback is mailinfo won't do charset conversion in those
systems.
Signed-off-by: Fernando J. Pereda <ferdy@gentoo.org>
Signed-off-by: Junio C Hamano <junkio@cox.net>
If the first part uses quoted-printable to protect iso8859-1
name in the commit log, and the second part was plain ascii text
patchfile without even Content-Transfer-Encoding subheader, we
incorrectly tried to decode the patch as quoted printable.
Signed-off-by: Junio C Hamano <junkio@cox.net>
This was a stupid typo that did not follow
http://www.iana.org/assignments/character-sets
Long noticed but neglected by JC, but finally reported by
Marco.
Signed-off-by: Junio C Hamano <junkio@cox.net>
An isolated developer could have a local-only e-mail, which will
be stripped out by mailinfo because it lacks '@'. Define a
fallback parser to accomodate that.
At the same time, reject authorless patch in git-am.
Signed-off-by: Junio C Hamano <junkio@cox.net>
Added an AIX clause in the Makefile; that clause likely
will be wrong for any AIX pre-5.2, but I can only test
on 5.3. mailinfo.c was missing the compat header file,
and convert-objects.c needs to define a specific
_XOPEN_SOURCE as well as _XOPEN_SOURCE_EXTENDED.
Signed-off-by: E. Jason Riedy <ejr@cs.berkeley.edu>
Signed-off-by: Junio C Hamano <junkio@cox.net>
This attempts to clean up the way various compatibility
functions are defined and used.
- A new header file, git-compat-util.h, is introduced. This
looks at various NO_XXX and does necessary function name
replacements, equivalent of -Dstrcasestr=gitstrcasestr in the
Makefile.
- Those function name replacements are removed from the Makefile.
- Common features such as usage(), die(), xmalloc() are moved
from cache.h to git-compat-util.h; cache.h includes
git-compat-util.h itself.
Signed-off-by: Junio C Hamano <junkio@cox.net>
Specifying the value for a single letter, single dash option
parameter with equal sign looked funny, and more importantly
calling the flag to override encoding from utf-8 to something
else "-u" (obviously abbreviated from "utf-8") did not make any
sense. So spell it out.
Signed-off-by: Junio C Hamano <junkio@cox.net>
Junio C Hamano
Juan F. Codagnone
René Scharfe
Jeff King
Ville Skyttä
Ævar Arnfjörð Bjarmason
Brandon Williams
Johannes Schindelin
Linus Torvalds
Kyle J. McKay
Kevin Daudt
Jonathan Tan
Nguyễn Thái Ngọc Duy
Lukas Sandström
Junio C Hamano
Eric W. Biederman
Fernando J. Pereda
Jason Riedy