Re: [PATCH] trailers: stop recognizing URLs as trailers
From: Junio C Hamano <hidden>
Date: 2026-08-02 22:36:06
kristofferhaugsbakk@fastmail.com writes:
From: Kristoffer Haugsbakk <redacted> An HTTPS URL starts with an alphanumeric scheme followed by a colon. That means that they will be recognized as trailers in a trailer block. That turns out to be a problem in practice. Let’s stop recognizing these as trailers by failing the trailer parsing when we: 1. find the separator; 2. the separator and the next two characters form `://`; and 3. we haven’t parsed any whitespace yet.
When I read the problem description, I would have expected you to say "If we find <token>: at the beginning of the line, check <token> against known URL schemes like https, ftp, etc. and declare that the line is not a trailer, if it matches". Checking against "://" is much more robust, as it is less likely to happen in random text, and we avoid maintaining a whitelist of scheme names. You are certainly smarter than I am ;-). Shouldn't we restrict the token preceding "://" more strictly than simply prohibiting whitespace?
Helped-by: Jeff King [off-list ref] Signed-off-by: Kristoffer Haugsbakk <redacted> ---
quoted hunk ↗ jump to hunk
diff --git a/trailer.c b/trailer.c index 6d8ec7fa8d8..971ae459596 100644 --- a/trailer.c +++ b/trailer.c@@ -635,8 +635,13 @@ static ssize_t find_separator(const char *line, const char *separators) int whitespace_found = 0; const char *c; for (c = line; *c; c++) { - if (strchr(separators, *c)) + if (strchr(separators, *c)) { + /* avoid accidental URL matches (://) */ + if (*c == ':' && c[1] == '/' && c[2] == '/' &&
How do we know the references to c[1] and c[2] do not access an unmapped piece of memory? The answer is that line[] is NUL terminated, so c[0] == ':' guarantees that c[1] is safe to read and unless it is NUL (and c[1] =='/' certainly means it is not NUL), c[2] is safe to read. OK. Makes sense to me. Thanks.
+ !whitespace_found)
+ return -1;
return c - line;
+ }
if (!whitespace_found && (isalnum(*c) || *c == '-'))
continue;
if (c != line && (*c == ' ' || *c == '\t')) {