{"thread":{"id":"65758","subject":"trailers: --only-trailers normalizes URLs to trailers","startedAt":"2026-06-04T21:28:12Z","lastAt":"2026-08-21T15:55:10Z","messageCount":18,"participants":["Kristoffer Haugsbakk","Jeff King","kristofferhaugsbakk@fastmail.com","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"544765","messageId":"ae4a32e7-bacb-4c88-b2a0-5aeaff60b904@app.fastmail.com","threadId":"65758","inReplyTo":null,"subject":"trailers: --only-trailers normalizes URLs to trailers","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-06-04T21:27:51Z","receivedAt":"2026-06-04T21:28:12Z","isPatch":false,"body":"The following is a bug that follows straightforwardly from the documented\nor discussed behavior. In that sense it is not a bug. But it is a bug in\nthe sense that it makes things inconvenient and violates a design goal.\n\n> Thank you for filling out a Git bug report!\n> Please answer the following questions to help us understand your issue.\n>\n> What did you do before the bug happened? (Steps to reproduce your issue)\n\nRan what is the equivalent of\n\n    git interpret-trailers --only-trailers\n\nWith\n\n    git log --format=\"%(trailers:only)\"\n\n> What did you expect to happen? (Expected behavior)\n\nFor URLs like https://www.digsm.xyz/ to be left intact.\n\n(Well, did I expect that? It follows from the discussed behavior...)\n\n> What happened instead? (Actual behavior)\n\nURLs on a line by themselves in eligible trailer blocks get\nnormalized/canonicalized to a “trailer” with key e.g. `https`:\n\n    https: //www.digsm.xyz/\n\n> What's different between what you expected and what actually happened?\n\nIn an ideal world to have some special-casing of URLs so that they are\nnot detected as trailers. Does anyone realistically want trailers like\nthis?:\n\n    file: //...\n    http: //...\n    https: //...\n\nMaybe a C-style comment?\n\n    https: // I changed my mind about providing a URL here.\n        This comment is a placeholder.\n    Comment: // But next up we have a URL\n    https: https://protocoltwiceover.net\n\nAnd this is where my imagination ends.\n\nJust special-casing `https` would go a long way.\n\n> Anything else you want to add:\n\nYes, after this [System Info] part.\n\n> Please review the rest of the bug report below.\n> You can delete any lines you don't wish to share.\n\n[System Info]\ngit version:\ngit version 2.54.0\ncpu: x86_64\nbuilt from commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nrust: disabled\ngettext: enabled\nlibcurl: 7.81.0\nOpenSSL: OpenSSL 3.0.2 15 Mar 2022\nzlib: 1.2.11\nSHA-1: SHA1_DC\nSHA-256: SHA256_BLK\ndefault-ref-format: files\ndefault-hash: sha1\nuname: Linux 6.8.0-117-generic #117~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Thu May  7 22:17:46 UTC  x86_64\ncompiler info: gnuc: 11.4\nlibc info: glibc: 2.35\n$SHELL (typically, interactive shell): /bin/bash\n\n[Enabled Hooks]\ncommit-msg\npost-applypatch\npost-commit\nsendemail-validate\n\n***\n\nThat things like `--format='%(trailers:only)'` normalize trailers is\nknown and has been discussed before.[1] There’s been discussion around\nthe key capitalization and prefix normalization. But this is not about\nthat. This is just about normalizing the separator part.\n\n🔗 1: https://lore.kernel.org/git/87blk0rjob.fsf@0x63.nu/\n\nOne design goal for trailers (either by implementers or reviewers or both)\nhas been to avoid false positives.[2] That meant trying to avoid\ndetecting trailers that were not intended. For example:\n\n    Everything was better in the past. Let me not even start on this\n    rant: it is not good for my blood pressure.\n\nThis will not be picked up as a trailer block unless `rant` is configured\nas a trailer key.\n\n† 2: See e.g. Jonathan Tan’s series about among other things adding the\n     25% rule\n\n     https://lore.kernel.org/git/xmqq7f96sa9i.fsf@gitster.mtv.corp.google.com/\n\nBut that’s pretty innocuous. Just a misplaced rant. The topic of this\nbug report is not a big deal either, but it is:\n\n1. Structured data that gets mangled in this normalize mode\n2. That can naturally go at the end of the message on its own line\n\nAnd these two points are very relevant for people who never use\ntrailers. Or, wait. I guess it isn’t if they don’t use trailers and thus\nwill never normalize them. But it is relevant if they work on a project\nwhere someone else does that.\n\nIN INTENDED TRAILER BLOCKS [3]\n\nAnd then there are things that can go wrong if you intend to write trailer blocks:\n\n1. “Non-trailer lines” that are URLs get normalized as trailers (NTL for\n   short)\n2. User error line wrapping turns one trailer into an empty trailer plus\n   a `https` trailer (LW for short)\n3. Normalizing trailers along the way (as in patches in flight or\n   something) introduces this strange lossiness (NL for short)\n\nI did (2) (LW for short) four years ago it seems:\n\n    See:\n    https://digsm.yxz/blog/important-context/?bigtechtracker=86b0c5a1e2b73b08fd54c727f4458649ed9fe3ad1b6e8ac9460c070113509a1e\n\n† 3: Are all-caps titles good or bad? Let me know.\n\nIN THE LINUX KERNEL\n\nThere are some hits for the `http` and `https` trailers when trailers\nare normalized. The baseline:\n\n   $ git log --extended-regexp --grep='https?: //' --oneline | wc -l\n   12\n\nWith normalization:\n\n    $ git log --format='%(trailers:only)' |\n          grep --extended-regexp '^https?: //' | wc -l\n    245\n\nNote that I have no idea how the Linux Kernel is run. But I don’t\nimagine that there are uses for `https: //...` trailers.\n\nAnd trailer usage is complicated. There are for example on-purpose\nindented `Link` “trailers”, presumably for the purpose of *excluding*\nthem as `Link` trailers. See:\n\n    commit d80a9cb1a64ab9c817b6262c7e4e433b6a3581a0\n\n    <body>\n\n    [ljs@kernel.org: avoid bisection hazard]\n      Link: https://lkml.kernel.org/r/d0cc6161-77a4-42ba-a411-96c23c78df1b@lucifer.local\n    Link: https://lkml.kernel.org/r/c2be872d64ef9573b80727d9ab5446cf002f17b5.1774029655.git.ljs@kernel.org\n    Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>\n    [MORE BELOW]\n\nIs that indented link for that `[]` comment? I dunno.\n\nBut what’s the main topic here are intended non-trailer lines which are\nURLs that get treated as trailers (NTL). Like this invented example:\n\nReported-by: ...\nhttps://digsm.xyz/?avastvirus=5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03\nSigned-off-by: ...\n\nOr this real example where the URLs are clearly part of a “comment”\nnon-trailer run.\n\n    8236fc613d44e59f6736d6c3e9efffaf26ab7f00\n    Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>\n    [bhelgaas: squash fixes:\n    https://lore.kernel.org/r/20260108013956.14351-2-bagasdotme@gmail.com\n    https://lore.kernel.org/r/20260108013956.14351-3-bagasdotme@gmail.com]\n    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>\n    Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>\n    Link: https://patch.msgid.link/20251210132907.58799-4-xueshuai@linux.alibaba.com\n\n(These are shown as they are written in the commit message. Normalizing\nthe messages would create `https` trailers.)\n\nHere are examples of line-wrapping mistake commits (LW) for `Link`,\n`Closes`, or `Fixes` (sometimes these point to bug URLs and not\ncommits):\n\n    5bd97f5c5f241a5610c4412d1b93995a26241f81\n    Link: https://patch.msgid.link/20260216-work-xattr-socket-v1-4-c2efa4f74cb7@kernel.org\n    Link:\n    https://lore.kernel.org/3cnmtqmakpbb2uwhenrj7kdqu3uefykiykjllgfbtpkiwhaa4s@sghkevv7jned [1]\n    Acked-by: Darrick J. Wong <djwong@kernel.org>\n    Reviewed-by: Jan Kara <jack@suse.cz>\n    Signed-off-by: Christian Brauner <brauner@kernel.org>\n\nand:\n\n• 24abe1f238e7d7ac56be6374c52a3c13dab84f69\n• 27e21516914dc130a79aa895a5a26e18f0213a5a\n• be3536a4bdda53ff5a91b7e542b167d12bddb317\n\nFinally there is this commit which has a trailer in the commit message\nitself with the key `https` (NL).\n\n    commit 496c0c4c53bbe1bad97e82cd12103df61a6e459d\n    ...\n    ...\n\n\tnet: wan: fsl_ucc_hdlc: free tx_skbuff in uhdlc_memclean\n\n        <body>\n\n\thttps: //sashiko.dev/#/patchset/20260429114208.941011-1-holger.brunck%40hitachienergy.com\n\tFixes: c19b6d246a35 (\"drivers/net: support hdlc function for QE-UCC\")\n\tSigned-off-by: Holger Brunck <holger.brunck@hitachienergy.com>\n\tLink: https://patch.msgid.link/20260507155332.3452319-1-holger.brunck@hitachienergy.com\n\tSigned-off-by: Jakub Kicinski <kuba@kernel.org>\n\nHow could this have happened? Follow the patch-id link.\n\nhttps://patch.msgid.link/20260507155332.3452319-1-holger.brunck@hitachienergy.com\n\n    https://sashiko.dev/#/patchset/20260429114208.941011-1-holger.brunck%40hitachienergy.com\n    Fixes: c19b6d246a35 (\"drivers/net: support hdlc function for QE-UCC\")\n    Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>\n\nSo it was just a non-trailer URL line as this person submitted it. But\npresumably the person who applied it put the message through a round of\nnormalization.\n\nCheers, good night\n\n-- \nKristoffer\n"},{"id":"544991","messageId":"20260609004340.GF358144@coredump.intra.peff.net","threadId":"65758","inReplyTo":"ae4a32e7-bacb-4c88-b2a0-5aeaff60b904@app.fastmail.com","subject":"Re: trailers: --only-trailers normalizes URLs to trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-09T00:43:40Z","receivedAt":"2026-06-09T00:43:42Z","isPatch":false,"body":"On Thu, Jun 04, 2026 at 11:27:51PM +0200, Kristoffer Haugsbakk wrote:\n\n> The following is a bug that follows straightforwardly from the documented\n> or discussed behavior. In that sense it is not a bug. But it is a bug in\n> the sense that it makes things inconvenient and violates a design goal.\n\nYeah, though if you'll allow me to nitpick your subject a moment: I\ndon't think --only-trailers is really the culprit here. It demonstrates\nthe problem because it normalizes the \"trailer\" it found. But the loose\ntrailer matching is the more fundamental issue. For example:\n\ngit interpret-trailers --trailer=foo=bar <<\\EOF\nsubject\n\nbody\n\nhttp://example.com\nEOF\n\nwill stick the new \"foo: bar\" trailer right up against the (now-broken)\n\"http:\" trailer. When it should come in its own stanza, which it would\nif you added a line \"other\" at the end, since that tells us that \"http:\"\ncan't be a trailer.\n\n> > What's different between what you expected and what actually happened?\n> \n> In an ideal world to have some special-casing of URLs so that they are\n> not detected as trailers. Does anyone realistically want trailers like\n> this?:\n> \n>     file: //...\n>     http: //...\n>     https: //...\n\nI could even see those as trailers, if somebody really wanted to allow\narbitrary values that might just happen to start with \"//\". But without\nthe whitespace after the colon, it is quite questionable.\n\n> Just special-casing `https` would go a long way.\n\nAgreed, though I think a rule like: \":// (with no whitespace)\" is not a\nvalid separator. Something like this:\n\ndiff --git a/trailer.c b/trailer.c\nindex 6d8ec7fa8d..342ed81c78 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -635,8 +635,12 @@ static ssize_t find_separator(const char *line, const char *separators)\n \tint whitespace_found = 0;\n \tconst char *c;\n \tfor (c = line; *c; c++) {\n-\t\tif (strchr(separators, *c))\n+\t\tif (strchr(separators, *c)) {\n+\t\t\t/* special case to avoid accidental URL matches */\n+\t\t\tif (*c == ':' && c[1] == '/' && c[2] == '/')\n+\t\t\t\treturn -1;\n \t\t\treturn c - line;\n+\t\t}\n \t\tif (!whitespace_found && (isalnum(*c) || *c == '-'))\n \t\t\tcontinue;\n \t\tif (c != line && (*c == ' ' || *c == '\\t')) {\n\n-Peff\n"},{"id":"545145","messageId":"d8f7f827-27da-41fc-af8d-72d383b24fff@app.fastmail.com","threadId":"65758","inReplyTo":"20260609004340.GF358144@coredump.intra.peff.net","subject":"Re: trailers: --only-trailers normalizes URLs to trailers","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-06-10T14:21:29Z","receivedAt":"2026-06-10T14:21:51Z","isPatch":false,"body":"On Tue, Jun 9, 2026, at 02:43, Jeff King wrote:\n> On Thu, Jun 04, 2026 at 11:27:51PM +0200, Kristoffer Haugsbakk wrote:\n>\n>> The following is a bug that follows straightforwardly from the documented\n>> or discussed behavior. In that sense it is not a bug. But it is a bug in\n>> the sense that it makes things inconvenient and violates a design goal.\n>\n> Yeah, though if you'll allow me to nitpick your subject a moment: I\n> don't think --only-trailers is really the culprit here. It demonstrates\n> the problem because it normalizes the \"trailer\" it found. But the loose\n> trailer matching is the more fundamental issue. For example:\n>\n>[snip]\n\nYeah, this is more precise. I focused a ton on the normalized output\nbecause that’s what makes it obvious. But the fundamental problem is\ninterpreting URLs like trailers.\n\n>\n>> > What's different between what you expected and what actually happened?\n>>\n>> In an ideal world to have some special-casing of URLs so that they are\n>> not detected as trailers. Does anyone realistically want trailers like\n>> this?:\n>>\n>>     file: //...\n>>     http: //...\n>>     https: //...\n>\n> I could even see those as trailers, if somebody really wanted to allow\n> arbitrary values that might just happen to start with \"//\". But without\n> the whitespace after the colon, it is quite questionable.\n>\n>> Just special-casing `https` would go a long way.\n>\n> Agreed, though I think a rule like: \":// (with no whitespace)\" is not a\n> valid separator. Something like this:\n\nYes, matching on `://` strictly is a better proposal. No need to care\nabout `http`, `https`, `file`, etc. And both of these would *still* have\nto be true for this change to be a false negative w.r.t. the user’s\nintentions:\n\n• They really input a trailer that looks like a URL, but it’s not meant\n  to be a URL\n• They really wanted the value to start with `//`\n\nAnd again I don’t think that is likely to ever happen (with a knock\non wood).\n\nThanks!\n\n>\n> diff --git a/trailer.c b/trailer.c\n> index 6d8ec7fa8d..342ed81c78 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -635,8 +635,12 @@ static ssize_t find_separator(const char *line,\n> const char *separators)\n>  \tint whitespace_found = 0;\n>  \tconst char *c;\n>  \tfor (c = line; *c; c++) {\n> -\t\tif (strchr(separators, *c))\n> +\t\tif (strchr(separators, *c)) {\n> +\t\t\t/* special case to avoid accidental URL matches */\n> +\t\t\tif (*c == ':' && c[1] == '/' && c[2] == '/')\n> +\t\t\t\treturn -1;\n>  \t\t\treturn c - line;\n> +\t\t}\n>  \t\tif (!whitespace_found && (isalnum(*c) || *c == '-'))\n>  \t\t\tcontinue;\n>  \t\tif (c != line && (*c == ' ' || *c == '\\t')) {\n"},{"id":"545245","messageId":"20260611065616.GE2191159@coredump.intra.peff.net","threadId":"65758","inReplyTo":"d8f7f827-27da-41fc-af8d-72d383b24fff@app.fastmail.com","subject":"Re: trailers: --only-trailers normalizes URLs to trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-06-11T06:56:16Z","receivedAt":"2026-06-11T06:56:18Z","isPatch":false,"body":"On Wed, Jun 10, 2026 at 04:21:29PM +0200, Kristoffer Haugsbakk wrote:\n\n> > Yeah, though if you'll allow me to nitpick your subject a moment: I\n> > don't think --only-trailers is really the culprit here. It demonstrates\n> > the problem because it normalizes the \"trailer\" it found. But the loose\n> > trailer matching is the more fundamental issue. For example:\n> >\n> >[snip]\n> \n> Yeah, this is more precise. I focused a ton on the normalized output\n> because that’s what makes it obvious. But the fundamental problem is\n> interpreting URLs like trailers.\n\nThat makes sense. As the author of --only-trailers I immediately\nwondered if I had introduced a bug in it, so I was partially motivated\nby exonerating myself. ;) I agree that using it is the simplest way to\ndemonstrate the problem.\n\n> > Agreed, though I think a rule like: \":// (with no whitespace)\" is not a\n> > valid separator. Something like this:\n> \n> Yes, matching on `://` strictly is a better proposal. No need to care\n> about `http`, `https`, `file`, etc. And both of these would *still* have\n> to be true for this change to be a false negative w.r.t. the user’s\n> intentions:\n> \n> • They really input a trailer that looks like a URL, but it’s not meant\n>   to be a URL\n> • They really wanted the value to start with `//`\n> \n> And again I don’t think that is likely to ever happen (with a knock\n> on wood).\n\nI didn't spend much effort on the patch I showed beyond running it once.\nIt would probably need tests and a doc update. I wasn't planning to run\nwith it, but if you feel like doing so, please feel free to use it as\nyou like.\n\n-Peff\n"},{"id":"545247","messageId":"01433a29-133a-4705-98e1-c8f29d66e581@app.fastmail.com","threadId":"65758","inReplyTo":"20260611065616.GE2191159@coredump.intra.peff.net","subject":"Re: trailers: --only-trailers normalizes URLs to trailers","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-06-11T07:03:41Z","receivedAt":"2026-06-11T07:04:03Z","isPatch":false,"body":"On Thu, Jun 11, 2026, at 08:56, Jeff King wrote:\n>>[snip]\n>> \n>> And again I don’t think that is likely to ever happen (with a knock\n>> on wood).\n>\n> I didn't spend much effort on the patch I showed beyond running it once.\n> It would probably need tests and a doc update. I wasn't planning to run\n> with it, but if you feel like doing so, please feel free to use it as\n> you like.\n\nYeah, I want to add some tests on top\nand make a sumbission (but sent a\nmessage to first confirm that you weren't\ncooking anything more ;) ). Thanks.\n\n-- \nSent from mobile\n"},{"id":"549444","messageId":"URLs_not_trailers.b13@msgid.xyz","threadId":"65758","inReplyTo":"20260609004340.GF358144@coredump.intra.peff.net","subject":"[PATCH] trailers: stop recognizing URLs as trailers","fromName":"","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-08-02T19:57:17Z","receivedAt":"2026-08-02T19:57:57Z","isPatch":true,"body":"From: Kristoffer Haugsbakk <code@khaugsbakk.name>\n\nAn HTTPS URL starts with an alphanumeric scheme followed by a colon.\nThat means that they will be recognized as trailers in a trailer block.\nThat turns out to be a problem in practice. Let’s stop recognizing these\nas trailers by failing the trailer parsing when we:\n\n1. find the separator;\n2. the separator and the next two characters form `://`; and\n3. we haven’t parsed any whitespace yet.\n\nThe simplest example of how this can be a problem is for people who do\nnot use trailers but may leave URLs at the end of the commit message.\nNow, while these authors might not use trailers themselves, other\nauthors may have used trailers and this metadata confusion can become a\nproblem once someone tries to extract that metadata (and non-metadata).\n\nLet’s now look at some examples in the Linux Kernel[1] to see how this\nis a problem in practice.\n\nThere are commits which contain intended non-trailer lines which start\nwith URLs. These are comments. Example with just the trailers:[2]\n\n    Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>\n    [bhelgaas: squash fixes:\n    https://lore.kernel.org/r/20260108013956.14351-2-bagasdotme@gmail.com\n    https://lore.kernel.org/r/20260108013956.14351-3-bagasdotme@gmail.com]\n    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>\n    Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>\n    Link: https://patch.msgid.link/20251210132907.58799-4-xueshuai@linux.alibaba.com\n\nThose `[]` pairs delimit the “squash fixes” comment.\n\nNow, any of these two commands:\n\n     git log --format='%(trailers:only)' -1 <commit>\n     git log -1 --format=%B <commit> |\n         git interpret-trailers --only-trailers\n\nWill both wrongly (according to the surmised user intent) include these\ntwo URL lines as trailers and also mangle the URLs, e.g.:\n\n    https: //lore.kernel.org/r/20260108013956.14351-2-bagasdotme@gmail.com\n\nBecause the `--only-trailers` mode (or `only` for the git-log(1) format)\nnormalizes the output to a colon and a space.\n\nAnother example is linewrapping mistakes; a `Link` trailer with a\nURL where the URL ended up on the next line, presumably because the\nuser’s editor linewrapped the “too long” line. Example with just the\ntrailers:[3]\n\n    Link: https://patch.msgid.link/20260216-work-xattr-socket-v1-4-c2efa4f74cb7@kernel.org\n    Link:\n    https://lore.kernel.org/3cnmtqmakpbb2uwhenrj7kdqu3uefykiykjllgfbtpkiwhaa4s@sghkevv7jned [1]\n    Acked-by: Darrick J. Wong <djwong@kernel.org>\n    Reviewed-by: Jan Kara <jack@suse.cz>\n    Signed-off-by: Christian Brauner <brauner@kernel.org>\n\nNow, this intended trailer is already ruined, but interpreting the URL\nas a standalone trailer only compounds the mistake.\n\nYet another example is the trailer machinery normalizing the trailer\nblock before application, resulting in a `https` trailer key in the\ncommit message itself. Example with just the trailers:[4]\n\n    https: //sashiko.dev/#/patchset/20260429114208.941011-1-holger.brunck%40hitachienergy.com\n    Fixes: c19b6d246a35 (\"drivers/net: support hdlc function for QE-UCC\")\n    Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>\n    Link: https://patch.msgid.link/20260507155332.3452319-1-holger.brunck@hitachienergy.com\n    Signed-off-by: Jakub Kicinski <kuba@kernel.org>\n\nWe have a helpful `Link` that points to the original patch.[5] Following\nit we can see that that `https` trailer was indeed a URL\noriginally (again just the trailer block here):\n\n    https://sashiko.dev/#/patchset/20260429114208.941011-1-holger.brunck%40hitachienergy.com\n    Fixes: c19b6d246a35 (\"drivers/net: support hdlc function for QE-UCC\")\n    Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>\n\nSo how did it end up as a `https` trailer? My theory is that the trailer\nblock was normalized on patch application, causing a URL comment to be\nwrongly normalized and cemented in the commit message as a trailer.[6]\n\n† 1: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/\n† 2: commit 8236fc613d44e59f6736d6c3e9efffaf26ab7f00\n† 3: commit 5bd97f5c5f241a5610c4412d1b93995a26241f81\n† 4: commit 496c0c4c53bbe1bad97e82cd12103df61a6e459d\n† 5: https://patch.msgid.link/20260507155332.3452319-1-holger.brunck@hitachienergy.com\n† 6: There are only four commits in the Linux Kernel of this kind, and\n     three of them have the same recurring person in the signoff chain.\n\nHelped-by: Jeff King <peff@peff.net>\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n\nNotes (series):\n    Topic name: trailers-no-urls\n    \n    Topic summary: Stop recognizing URLs in trailer blocks as trailers.\n    \n    Note to the maintainer: this is based on `master` with topic\n    kh/doc-trailers merged into it.\n    \n    I used Peff’s suggestion from the previous email. I just shortened the\n    comment, added the parentheses (://) and added the condition that\n    whitespace has not been found for the case I discussed of someone writing\n    out `<key>: //` (note the space). (Or for that matter: `<key> ://`.) I\n    can’t imagine that that is a likely case, but I just want to avoid matching\n    URLs, so we don’t have to reject this case.\n    \n    t/u-trailer.c: `expected_contents[]` is not formatted like the other ones\n    in this file. But this is what clang-format(1) gave me.\n\n Documentation/git-interpret-trailers.adoc | 13 ++++--\n t/t7513-interpret-trailers.sh             | 19 +++++++++\n t/unit-tests/u-trailer.c                  | 52 +++++++++++++++++++++++\n trailer.c                                 |  7 ++-\n 4 files changed, 87 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.adoc b/Documentation/git-interpret-trailers.adoc\nindex b4988d39eab..903d598dcb0 100644\n--- a/Documentation/git-interpret-trailers.adoc\n+++ b/Documentation/git-interpret-trailers.adoc\n@@ -123,9 +123,16 @@ OTHER RULES\n What was covered in the previous section are the rules that are relevant\n for regular use. The following points are included for completeness.\n \n-This command ignores comment lines (see `core.commentString` in\n-linkgit:git-config[1]). This is for use with the `prepare-commit-msg`\n-and `commit-msg` hooks.\n+--\n+* This command ignores comment lines (see `core.commentString` in\n+  linkgit:git-config[1]). This is for use with the `prepare-commit-msg`\n+  and `commit-msg` hooks.\n+\n+* Candidate trailer lines that have `:` as the separator, that have no\n+  whitespace before the value part, and that start with `//` are not\n+  recognized as trailers. This is to avoid accidentally interpreting\n+  URLs as trailers (e.g. lines that start with `https://`).\n+--\n \n OPTIONS\n -------\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 818a8dafbd2..e3555b6d51d 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1989,4 +1989,23 @@ test_expect_success 'handling of --- lines in conjunction with cut-lines' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'URLs and lines that are not quite URLs' '\n+\tcat >expect <<-\\EOF &&\n+\thttps: //www.a-trailer.org\n+\thttps: //www.another-trailer.org\n+\tSigned-off-by: somebody <somebody@somewhere>\n+\tEOF\n+\tgit interpret-trailers --only-trailers >actual <<-\\EOF &&\n+\tsubject\n+\n+\tbody\n+\n+\thttps://www.not-a-trailer.org\n+\thttps ://www.a-trailer.org\n+\thttps: //www.another-trailer.org\n+\tSigned-off-by: somebody <somebody@somewhere>\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/t/unit-tests/u-trailer.c b/t/unit-tests/u-trailer.c\nindex 3d60ea1603d..7404b165fac 100644\n--- a/t/unit-tests/u-trailer.c\n+++ b/t/unit-tests/u-trailer.c\n@@ -318,3 +318,55 @@ void test_trailer__one_non_trailer_no_git_trailers(void)\n \t\t\t   0,\n \t\t\t   expected_contents);\n }\n+\n+void test_trailer__URL(void)\n+{\n+\tstruct contents expected_contents[] = { 0 };\n+\n+\tt_trailer_iterator(\"Subject: foo bar\\n\"\n+\t\t\t   \"\\n\"\n+\t\t\t   /*\n+\t\t\t    * We do not want to match URLs as trailers.\n+\t\t\t    */\n+\t\t\t   \"https://www.example.org\\n\",\n+\t\t\t   0,\n+\t\t\t   expected_contents);\n+}\n+\n+void test_trailer__not_a_URL_space_after_separator(void)\n+{\n+\tstruct contents expected_contents[] = {\n+\t\t{ .raw = \"https: //www.example.org\\n\",\n+\t\t  .key = \"https\",\n+\t\t  .val = \"//www.example.org\" },\n+\t\t{ 0 },\n+\t};\n+\n+\tt_trailer_iterator(\"Subject: foo bar\\n\"\n+\t\t\t   \"\\n\"\n+\t\t\t   /*\n+\t\t\t    * This has a space after ':' so it's not a URL.\n+\t\t\t    */\n+\t\t\t   \"https: //www.example.org\\n\",\n+\t\t\t   1,\n+\t\t\t   expected_contents);\n+}\n+\n+void test_trailer__not_a_URL_space_before_separator(void)\n+{\n+\tstruct contents expected_contents[] = {\n+\t\t{ .raw = \"https ://www.example.org\\n\",\n+\t\t  .key = \"https\",\n+\t\t  .val = \"//www.example.org\" },\n+\t\t{ 0 },\n+\t};\n+\n+\tt_trailer_iterator(\"Subject: foo bar\\n\"\n+\t\t\t   \"\\n\"\n+\t\t\t   /*\n+\t\t\t    * This has a space before ':' so it's not a URL.\n+\t\t\t    */\n+\t\t\t   \"https ://www.example.org\\n\",\n+\t\t\t   1,\n+\t\t\t   expected_contents);\n+}\ndiff --git a/trailer.c b/trailer.c\nindex 6d8ec7fa8d8..971ae459596 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -635,8 +635,13 @@ static ssize_t find_separator(const char *line, const char *separators)\n \tint whitespace_found = 0;\n \tconst char *c;\n \tfor (c = line; *c; c++) {\n-\t\tif (strchr(separators, *c))\n+\t\tif (strchr(separators, *c)) {\n+\t\t\t/* avoid accidental URL matches (://) */\n+\t\t\tif (*c == ':' && c[1] == '/' && c[2] == '/' &&\n+\t\t\t    !whitespace_found)\n+\t\t\t\treturn -1;\n \t\t\treturn c - line;\n+\t\t}\n \t\tif (!whitespace_found && (isalnum(*c) || *c == '-'))\n \t\t\tcontinue;\n \t\tif (c != line && (*c == ' ' || *c == '\\t')) {\n-- \n2.54.0.22.g9e26862b904\n\n"},{"id":"549456","messageId":"xmqqmrv42lrg.fsf@gitster.g","threadId":"65758","inReplyTo":"URLs_not_trailers.b13@msgid.xyz","subject":"Re: [PATCH] trailers: stop recognizing URLs as trailers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-02T22:36:03Z","receivedAt":"2026-08-02T22:36:06Z","isPatch":true,"body":"kristofferhaugsbakk@fastmail.com writes:\n\n> From: Kristoffer Haugsbakk <code@khaugsbakk.name>\n>\n> An HTTPS URL starts with an alphanumeric scheme followed by a colon.\n> That means that they will be recognized as trailers in a trailer block.\n> That turns out to be a problem in practice. Let’s stop recognizing these\n> as trailers by failing the trailer parsing when we:\n>\n> 1. find the separator;\n> 2. the separator and the next two characters form `://`; and\n> 3. we haven’t parsed any whitespace yet.\n\nWhen I read the problem description, I would have expected you to\nsay \"If we find <token>: at the beginning of the line, check <token>\nagainst known URL schemes like https, ftp, etc. and declare that the\nline is not a trailer, if it matches\".  Checking against \"://\" is\nmuch more robust, as it is less likely to happen in random text, and\nwe avoid maintaining a whitelist of scheme names.  You are certainly\nsmarter than I am ;-).\n\nShouldn't we restrict the token preceding \"://\" more strictly than\nsimply prohibiting whitespace?\n\n> Helped-by: Jeff King <peff@peff.net>\n> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n> ---\n\n> diff --git a/trailer.c b/trailer.c\n> index 6d8ec7fa8d8..971ae459596 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -635,8 +635,13 @@ static ssize_t find_separator(const char *line, const char *separators)\n>  \tint whitespace_found = 0;\n>  \tconst char *c;\n>  \tfor (c = line; *c; c++) {\n> -\t\tif (strchr(separators, *c))\n> +\t\tif (strchr(separators, *c)) {\n> +\t\t\t/* avoid accidental URL matches (://) */\n> +\t\t\tif (*c == ':' && c[1] == '/' && c[2] == '/' &&\n\nHow do we know the references to c[1] and c[2] do not access an\nunmapped piece of memory?  The answer is that line[] is NUL\nterminated, so c[0] == ':' guarantees that c[1] is safe to read and\nunless it is NUL (and c[1] =='/' certainly means it is not NUL),\nc[2] is safe to read.\n\nOK.  Makes sense to me.\n\nThanks.\n\n> +\t\t\t    !whitespace_found)\n> +\t\t\t\treturn -1;\n>  \t\t\treturn c - line;\n> +\t\t}\n>  \t\tif (!whitespace_found && (isalnum(*c) || *c == '-'))\n>  \t\t\tcontinue;\n>  \t\tif (c != line && (*c == ' ' || *c == '\\t')) {\n"},{"id":"549486","messageId":"53cc61d7-6206-453b-a0d4-a2fce00a2c29@app.fastmail.com","threadId":"65758","inReplyTo":"xmqqmrv42lrg.fsf@gitster.g","subject":"Re: [PATCH] trailers: stop recognizing URLs as trailers","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-08-03T12:11:02Z","receivedAt":"2026-08-03T12:12:24Z","isPatch":true,"body":"On Mon, Aug 3, 2026, at 00:36, Junio C Hamano wrote:\n> kristofferhaugsbakk@fastmail.com writes:\n>\n>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>\n>>\n>> An HTTPS URL starts with an alphanumeric scheme followed by a colon.\n>> That means that they will be recognized as trailers in a trailer block.\n>> That turns out to be a problem in practice. Let’s stop recognizing these\n>> as trailers by failing the trailer parsing when we:\n>>\n>> 1. find the separator;\n>> 2. the separator and the next two characters form `://`; and\n>> 3. we haven’t parsed any whitespace yet.\n>\n> When I read the problem description, I would have expected you to\n> say \"If we find <token>: at the beginning of the line, check <token>\n> against known URL schemes like https, ftp, etc. and declare that the\n> line is not a trailer, if it matches\".  Checking against \"://\" is\n> much more robust, as it is less likely to happen in random text, and\n> we avoid maintaining a whitelist of scheme names.  You are certainly\n> smarter than I am ;-).\n\nThe credit for being smart goes to Peff.\n\nhttps://lore.kernel.org/git/20260609004340.GF358144@coredump.intra.peff.net/T/#m03ac1a456648090c04cdf5141b7a3e638f1213d1\n\n> Shouldn't we restrict the token preceding \"://\" more strictly than\n> simply prohibiting whitespace?\n\nRight now (with this code) we know that:\n\n1. We have either parsed only alphanumerics and hyphens (whitespace is\n   ruled out); or\n2. We haven’t even parsed (1), but just found a line that starts with\n   `://`.\n\nIn both cases we bail out of the parsing with `-1`, i.e. “not a\ntrailer”.\n\nWikipedia[1] tells me that this current check *does* have a false positive:\n\n    A non-empty scheme component followed by a colon (:), consisting of\n    a sequence of characters beginning with a letter and followed by any\n    combination of letters, digits, plus (+), period (.), or hyphen (-).\n\n🔗 1: https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Syntax\n\nA URL *must* begin with a letter, but a trailer can just be a\ndigit. Which means that this is not the start of a URL:\n\n    1://\n\nBut the current code will reject it as a URL.\n\nThere are also other false positives like the strange but legal trailer\nkey `-`.\n\nOther than that, the character set of trailers (alphanums and hyphens)\nis a strict subset of URL <scheme>.\n\nI also see that the git-interpret-trailers(1) doc update should say\nalphanumerics and/or hyphens instead of just alphanums.\n\n>\n>> Helped-by: Jeff King <peff@peff.net>\n>> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n>> ---\n>\n>> diff --git a/trailer.c b/trailer.c\n>> index 6d8ec7fa8d8..971ae459596 100644\n>> --- a/trailer.c\n>> +++ b/trailer.c\n>> @@ -635,8 +635,13 @@ static ssize_t find_separator(const char *line, const char *separators)\n>>  \tint whitespace_found = 0;\n>>  \tconst char *c;\n>>  \tfor (c = line; *c; c++) {\n>> -\t\tif (strchr(separators, *c))\n>> +\t\tif (strchr(separators, *c)) {\n>> +\t\t\t/* avoid accidental URL matches (://) */\n>> +\t\t\tif (*c == ':' && c[1] == '/' && c[2] == '/' &&\n>\n> How do we know the references to c[1] and c[2] do not access an\n> unmapped piece of memory?  The answer is that line[] is NUL\n> terminated, so c[0] == ':' guarantees that c[1] is safe to read and\n> unless it is NUL (and c[1] =='/' certainly means it is not NUL),\n> c[2] is safe to read.\n>\n> OK.  Makes sense to me.\n\nI’m mostly a Java programmer so I had the same thought (non-didactically\n;) ). Yes, because of sentinel `NUL` and boolean short-circuiting we can\nincrementally peak one character ahead. This would be wrong in any\nlanguage without `NUL` terminating strings, but here it is\ncorrect. Indeed, checking the length first (which you would need to do\nin Java) would incur a linear cost since you need to scan the string\nuntil you hit the `NUL` terminator.\n\n>\n> Thanks.\n>[snip]\n"},{"id":"549498","messageId":"20260803152025.GA189075@coredump.intra.peff.net","threadId":"65758","inReplyTo":"URLs_not_trailers.b13@msgid.xyz","subject":"Re: [PATCH] trailers: stop recognizing URLs as trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-08-03T15:20:25Z","receivedAt":"2026-08-03T15:20:27Z","isPatch":true,"body":"On Sun, Aug 02, 2026 at 09:57:17PM +0200, kristofferhaugsbakk@fastmail.com wrote:\n\n> There are commits which contain intended non-trailer lines which start\n> with URLs. These are comments. Example with just the trailers:[2]\n> \n>     Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>\n>     [bhelgaas: squash fixes:\n>     https://lore.kernel.org/r/20260108013956.14351-2-bagasdotme@gmail.com\n>     https://lore.kernel.org/r/20260108013956.14351-3-bagasdotme@gmail.com]\n>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>\n>     Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>\n>     Link: https://patch.msgid.link/20251210132907.58799-4-xueshuai@linux.alibaba.com\n> \n> Those `[]` pairs delimit the “squash fixes” comment.\n\nThis example makes me wonder if we ought to be smarter about brackets.\nI.e., could/should we realize that the opening bracket is a comment and\nthen ignore everything up to the closing one? That would help this case\nand other weird cases like:\n\n  Signed-off-by: whomever\n  [peff: there's a really interesting thing going on\n  here: the comment is free-form text that happens to\n  use a colon in a sentence, but we'll interpret it\n  as a trailer with key \"here\"]\n  Signed-off-by: another unlucky soul\n\nThat said, I think there are cases without brackets that are also\nconfusing. Like:\n\n  Let me finish this commit message by telling you all about this\n  amazing url:\n\n  https://example.com\n\nSo I don't think that is a counter-argument against this URL\nfalse-positive check, but just a possible direction for future\nexploration.\n\n> Another example is linewrapping mistakes; a `Link` trailer with a\n> URL where the URL ended up on the next line, presumably because the\n> user’s editor linewrapped the “too long” line. Example with just the\n> trailers:[3]\n> \n>     Link: https://patch.msgid.link/20260216-work-xattr-socket-v1-4-c2efa4f74cb7@kernel.org\n>     Link:\n>     https://lore.kernel.org/3cnmtqmakpbb2uwhenrj7kdqu3uefykiykjllgfbtpkiwhaa4s@sghkevv7jned [1]\n>     Acked-by: Darrick J. Wong <djwong@kernel.org>\n>     Reviewed-by: Jan Kara <jack@suse.cz>\n>     Signed-off-by: Christian Brauner <brauner@kernel.org>\n> \n> Now, this intended trailer is already ruined, but interpreting the URL\n> as a standalone trailer only compounds the mistake.\n\nYeah, this is another interesting example. I agree it is fundamentally\nbroken, but showing the \"https\" trailer is just making it worse.\n\n> diff --git a/trailer.c b/trailer.c\n> index 6d8ec7fa8d8..971ae459596 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -635,8 +635,13 @@ static ssize_t find_separator(const char *line, const char *separators)\n>  \tint whitespace_found = 0;\n>  \tconst char *c;\n>  \tfor (c = line; *c; c++) {\n> -\t\tif (strchr(separators, *c))\n> +\t\tif (strchr(separators, *c)) {\n> +\t\t\t/* avoid accidental URL matches (://) */\n> +\t\t\tif (*c == ':' && c[1] == '/' && c[2] == '/' &&\n> +\t\t\t    !whitespace_found)\n> +\t\t\t\treturn -1;\n>  \t\t\treturn c - line;\n> +\t\t}\n\nAs discussed elsewhere, we are free to match with short-circuiting\nbecause of the NUL termination. But that also means we could write this\nas:\n\n  if (starts_with(c, \"://\") && !whitespace_found)\n\nwhich is perhaps a little more readable.\n\n-Peff\n"},{"id":"549499","messageId":"xmqqtspbz00x.fsf@gitster.g","threadId":"65758","inReplyTo":"20260803152025.GA189075@coredump.intra.peff.net","subject":"Re: [PATCH] trailers: stop recognizing URLs as trailers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-03T15:39:10Z","receivedAt":"2026-08-03T15:39:13Z","isPatch":true,"body":"Jeff King <peff@peff.net> writes:\n\n> As discussed elsewhere, we are free to match with short-circuiting\n> because of the NUL termination. But that also means we could write this\n> as:\n>\n>   if (starts_with(c, \"://\") && !whitespace_found)\n>\n> which is perhaps a little more readable.\n\n\"little more\" -> \"much more\" ;-).\n"},{"id":"549882","messageId":"0594b8ef-57f8-4b5f-a8b5-894123e3cbc5@app.fastmail.com","threadId":"65758","inReplyTo":"20260803152025.GA189075@coredump.intra.peff.net","subject":"Re: [PATCH] trailers: stop recognizing URLs as trailers","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-08-06T20:17:42Z","receivedAt":"2026-08-06T20:18:16Z","isPatch":true,"body":"On Mon, Aug 3, 2026, at 17:20, Jeff King wrote:\n> On Sun, Aug 02, 2026 at 09:57:17PM +0200,\n> kristofferhaugsbakk@fastmail.com wrote:\n>\n>> There are commits which contain intended non-trailer lines which start\n>> with URLs. These are comments. Example with just the trailers:[2]\n>>\n>>     Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>\n>>     [bhelgaas: squash fixes:\n>>     https://lore.kernel.org/r/20260108013956.14351-2-bagasdotme@gmail.com\n>>     https://lore.kernel.org/r/20260108013956.14351-3-bagasdotme@gmail.com]\n>>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>\n>>     Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>\n>>     Link: https://patch.msgid.link/20251210132907.58799-4-xueshuai@linux.alibaba.com\n>>\n>> Those `[]` pairs delimit the “squash fixes” comment.\n>\n> This example makes me wonder if we ought to be smarter about brackets.\n> I.e., could/should we realize that the opening bracket is a comment and\n> then ignore everything up to the closing one? That would help this case\n> and other weird cases like: [snip]\n\nI think that it makes a lot of sense to special-case brackets as\ndelimiting non-trailer runs.\n\n(for other readers) This support for non-trailer lines grew out of Linux\nKernel practices. At least according to this thread:\n\nhttps://lore.kernel.org/git/CA+55aFzN4SnenchxPScn61_apzitGAPtoYEd49iLZPxgK0KQGw@mail.gmail.com/\n\n(See also in particular: https://lore.kernel.org/git/20150905000745.GC11443@sigill.intra.peff.net/ )\n\nAnd part of the back-and-forth in that thread is an inherent tension:\nthe trailer format is loose. All you need is a some\nalphanumerics/hyphens and a colon. So it is simple to accidentally slip\nin a *real* trailer line along with all the cruft. Like Peff’s example\nshows:\n\n>\n>   Signed-off-by: whomever\n>   [peff: there's a really interesting thing going on\n>   here: the comment is free-form text that happens to\n>   use a colon in a sentence, but we'll interpret it\n>   as a trailer with key \"here\"]\n>   Signed-off-by: another unlucky soul\n\nImagine you had internalized the trailer parsing rules (which you\nshouldn’t have to but anyway); it would still be easy to accidentally\nwrite something like the above.\n\nBut with an additional `[]` rule you don’t have to worry:\n\n• A run of non-trailer lines starts with regex `^[`\n• And ends 0 or more lines later with regex `]$`\n• In addition to the existing rules\n\nAnd `[]` are illegal in trailer keys anyway.\n\nThis is a very Linux (and Git project) specific additional rule, but the\nnon-trailer lines rules were always like that.\n\n>\n> That said, I think there are cases without brackets that are also\n> confusing. Like:\n>\n>   Let me finish this commit message by telling you all about this\n>   amazing url:\n>\n>   https://example.com\n\nYeah exactly.\n\n>[snip]\n>> diff --git a/trailer.c b/trailer.c\n>> index 6d8ec7fa8d8..971ae459596 100644\n>> --- a/trailer.c\n>> +++ b/trailer.c\n>> @@ -635,8 +635,13 @@ static ssize_t find_separator(const char *line, const char *separators)\n>>  \tint whitespace_found = 0;\n>>  \tconst char *c;\n>>  \tfor (c = line; *c; c++) {\n>> -\t\tif (strchr(separators, *c))\n>> +\t\tif (strchr(separators, *c)) {\n>> +\t\t\t/* avoid accidental URL matches (://) */\n>> +\t\t\tif (*c == ':' && c[1] == '/' && c[2] == '/' &&\n>> +\t\t\t    !whitespace_found)\n>> +\t\t\t\treturn -1;\n>>  \t\t\treturn c - line;\n>> +\t\t}\n>\n> As discussed elsewhere, we are free to match with short-circuiting\n> because of the NUL termination. But that also means we could write this\n> as:\n>\n>   if (starts_with(c, \"://\") && !whitespace_found)\n>\n> which is perhaps a little more readable.\n\nOh for sure, much more readable.\n\nThanks!\n"},{"id":"550907","messageId":"xmqqcxvcuaak.fsf@gitster.g","threadId":"65758","inReplyTo":"URLs_not_trailers.b13@msgid.xyz","subject":"Re: [PATCH] trailers: stop recognizing URLs as trailers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-20T14:38:27Z","receivedAt":"2026-08-20T14:38:30Z","isPatch":true,"body":"kristofferhaugsbakk@fastmail.com writes:\n\n> From: Kristoffer Haugsbakk <code@khaugsbakk.name>\n>\n> An HTTPS URL starts with an alphanumeric scheme followed by a colon.\n> That means that they will be recognized as trailers in a trailer block.\n> That turns out to be a problem in practice. Let’s stop recognizing these\n> as trailers by failing the trailer parsing when we:\n> ...\n> Helped-by: Jeff King <peff@peff.net>\n> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n\nThis has been on hold waiting for the base topic to settle, but now\nthat the base topic has graduated, the effort can be rebooted.\n\nCan somebody summarize the outstanding issues on this topic (if\nany)?\n\nThanks.\n"},{"id":"550909","messageId":"c097cc44-3033-4f22-8c48-859de8353f99@app.fastmail.com","threadId":"65758","inReplyTo":"xmqqcxvcuaak.fsf@gitster.g","subject":"Re: [PATCH] trailers: stop recognizing URLs as trailers","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-08-20T14:47:52Z","receivedAt":"2026-08-20T14:48:21Z","isPatch":true,"body":"On Thu, Aug 20, 2026, at 16:38, Junio C Hamano wrote:\n> kristofferhaugsbakk@fastmail.com writes:\n>\n>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>\n>>\n>> An HTTPS URL starts with an alphanumeric scheme followed by a colon.\n>> That means that they will be recognized as trailers in a trailer block.\n>> That turns out to be a problem in practice. Let’s stop recognizing these\n>> as trailers by failing the trailer parsing when we:\n>> ...\n>> Helped-by: Jeff King <peff@peff.net>\n>> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n>\n> This has been on hold waiting for the base topic to settle, but now\n> that the base topic has graduated, the effort can be rebooted.\n\nThanks!\n\n> Can somebody summarize the outstanding issues on this topic (if\n> any)?\n\nI have version 2 ready. The only code change is using `starts_with` like\nPeff mentioned. What I wrote about the changes:\n\n    • Use `starts_with` for readability:\n        https://lore.kernel.org/git/20260609004340.GF358144@coredump.intra.peff.net/T/#m74203c474c34f1028a7e3d389ff46fb7e579444c\n    • Explain in the commit message that you can technically get false positive\n      “URL” start fragments:\n\n          https://lore.kernel.org/git/20260609004340.GF358144@coredump.intra.peff.net/T/#m35047d5c7a79abd23c11f97e6b6a0364409805e3\n\nI just have to dust it off.\n"},{"id":"550939","messageId":"V2_URLs_not_trailers.bf3@msgid.xyz","threadId":"65758","inReplyTo":"URLs_not_trailers.b13@msgid.xyz","subject":"[PATCH v2] trailers: stop recognizing URLs as trailers","fromName":"","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-08-20T20:00:02Z","receivedAt":"2026-08-20T20:00:55Z","isPatch":true,"body":"From: Kristoffer Haugsbakk <code@khaugsbakk.name>\n\nAn HTTPS URL starts with an alphanumeric scheme followed by a colon.\nThat means that they will be recognized as trailers in a trailer block.\nThat turns out to be a problem in practice. Let’s stop recognizing these\nas trailers by failing the trailer parsing when we:\n\n1. find the separator;\n2. the separator and the next two characters form `://`; and\n3. we haven’t parsed any whitespace yet.\n\nThe simplest example of how this can be a problem is for people who do\nnot use trailers but may leave URLs at the end of the commit message.\nNow, while these authors might not use trailers themselves, other\nauthors may have used trailers and this metadata confusion can become a\nproblem once someone tries to extract that metadata (and non-metadata).\n\nLet’s now look at some examples in the Linux Kernel[1] to see how this\nis a problem in practice.\n\nThere are commits which contain intended non-trailer lines which start\nwith URLs. These are comments. Example with just the trailers:[2]\n\n    Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>\n    [bhelgaas: squash fixes:\n    https://lore.kernel.org/r/20260108013956.14351-2-bagasdotme@gmail.com\n    https://lore.kernel.org/r/20260108013956.14351-3-bagasdotme@gmail.com]\n    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>\n    Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>\n    Link: https://patch.msgid.link/20251210132907.58799-4-xueshuai@linux.alibaba.com\n\nThose `[]` pairs delimit the “squash fixes” comment.\n\nNow, any of these two commands:\n\n     git log --format='%(trailers:only)' -1 <commit>\n     git log -1 --format=%B <commit> |\n         git interpret-trailers --only-trailers\n\nWill both wrongly (according to the surmised user intent) include these\ntwo URL lines as trailers and also mangle the URLs, e.g.:\n\n    https: //lore.kernel.org/r/20260108013956.14351-2-bagasdotme@gmail.com\n\nBecause the `--only-trailers` mode (or `only` for the git-log(1) format)\nnormalizes the output to a colon and a space.\n\nAnother example is linewrapping mistakes; a `Link` trailer with a\nURL where the URL ended up on the next line, presumably because the\nuser’s editor linewrapped the “too long” line. Example with just the\ntrailers:[3]\n\n    Link: https://patch.msgid.link/20260216-work-xattr-socket-v1-4-c2efa4f74cb7@kernel.org\n    Link:\n    https://lore.kernel.org/3cnmtqmakpbb2uwhenrj7kdqu3uefykiykjllgfbtpkiwhaa4s@sghkevv7jned [1]\n    Acked-by: Darrick J. Wong <djwong@kernel.org>\n    Reviewed-by: Jan Kara <jack@suse.cz>\n    Signed-off-by: Christian Brauner <brauner@kernel.org>\n\nNow, this intended trailer is already ruined, but interpreting the URL\nas a standalone trailer only compounds the mistake.\n\nYet another example is the trailer machinery normalizing the trailer\nblock before application, resulting in a `https` trailer key in the\ncommit message itself. Example with just the trailers:[4]\n\n    https: //sashiko.dev/#/patchset/20260429114208.941011-1-holger.brunck%40hitachienergy.com\n    Fixes: c19b6d246a35 (\"drivers/net: support hdlc function for QE-UCC\")\n    Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>\n    Link: https://patch.msgid.link/20260507155332.3452319-1-holger.brunck@hitachienergy.com\n    Signed-off-by: Jakub Kicinski <kuba@kernel.org>\n\nWe have a helpful `Link` that points to the original patch.[5] Following\nit we can see that that `https` trailer was indeed a URL\noriginally (again just the trailer block here):\n\n    https://sashiko.dev/#/patchset/20260429114208.941011-1-holger.brunck%40hitachienergy.com\n    Fixes: c19b6d246a35 (\"drivers/net: support hdlc function for QE-UCC\")\n    Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>\n\nSo how did it end up as a `https` trailer? My theory is that the trailer\nblock was normalized on patch application, causing a URL comment to be\nwrongly normalized and cemented in the commit message as a trailer.[6]\n\n† 1: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/\n† 2: commit 8236fc613d44e59f6736d6c3e9efffaf26ab7f00\n† 3: commit 5bd97f5c5f241a5610c4412d1b93995a26241f81\n† 4: commit 496c0c4c53bbe1bad97e82cd12103df61a6e459d\n† 5: https://patch.msgid.link/20260507155332.3452319-1-holger.brunck@hitachienergy.com\n† 6: There are only four commits in the Linux Kernel of this kind, and\n     three of them have the same recurring person in the signoff chain.\n\n***\n\nNote that this check has some benign false positives. A trailer key\ncan start with a digit, but a URL scheme can not start with a digit.\nThat means that a line that starts with `1://` will be rejected even\nthough it cannot be a URL. I don’t think this will reject any real\ntrailers, so I think the implementation simplicity is worth it.\n\nAnd these false positives are just for a limited start fragment check;\na mere heuristic, not a URL parser.\n\nHelped-by: Jeff King <peff@peff.net>\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n\nNotes (series):\n    Topic name (applied): trailers-no-urls\n    \n    Topic summary: Stop recognizing URLs in trailer blocks as trailers.\n    \n    Note to the maintainer: This has been rebased on `master` since the\n    dependent topic kh/doc-trailers has been merged thither.\n    \n    § Link to v1\n    \n    https://lore.kernel.org/git/URLs_not_trailers.b13@msgid.xyz/\n    \n    § Changes in v2\n    \n    • Use `starts_with` for readability:\n        https://lore.kernel.org/git/20260609004340.GF358144@coredump.intra.peff.net/T/#m74203c474c34f1028a7e3d389ff46fb7e579444c\n    • Since `starts_with` is a function, put the simpler conjunct\n      `whitespace_found` before it. It’s better to put the cheaper operations\n      first in a short-circuiting expression. Right?\n    • Explain in the commit message that you can technically get false positive\n      “URL” start fragments:\n    \n          https://lore.kernel.org/git/20260609004340.GF358144@coredump.intra.peff.net/T/#m35047d5c7a79abd23c11f97e6b6a0364409805e3\n\n Documentation/git-interpret-trailers.adoc | 13 ++++--\n t/t7513-interpret-trailers.sh             | 19 +++++++++\n t/unit-tests/u-trailer.c                  | 52 +++++++++++++++++++++++\n trailer.c                                 |  6 ++-\n 4 files changed, 86 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.adoc b/Documentation/git-interpret-trailers.adoc\nindex b4988d39eab..903d598dcb0 100644\n--- a/Documentation/git-interpret-trailers.adoc\n+++ b/Documentation/git-interpret-trailers.adoc\n@@ -123,9 +123,16 @@ OTHER RULES\n What was covered in the previous section are the rules that are relevant\n for regular use. The following points are included for completeness.\n \n-This command ignores comment lines (see `core.commentString` in\n-linkgit:git-config[1]). This is for use with the `prepare-commit-msg`\n-and `commit-msg` hooks.\n+--\n+* This command ignores comment lines (see `core.commentString` in\n+  linkgit:git-config[1]). This is for use with the `prepare-commit-msg`\n+  and `commit-msg` hooks.\n+\n+* Candidate trailer lines that have `:` as the separator, that have no\n+  whitespace before the value part, and that start with `//` are not\n+  recognized as trailers. This is to avoid accidentally interpreting\n+  URLs as trailers (e.g. lines that start with `https://`).\n+--\n \n OPTIONS\n -------\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 818a8dafbd2..e3555b6d51d 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1989,4 +1989,23 @@ test_expect_success 'handling of --- lines in conjunction with cut-lines' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'URLs and lines that are not quite URLs' '\n+\tcat >expect <<-\\EOF &&\n+\thttps: //www.a-trailer.org\n+\thttps: //www.another-trailer.org\n+\tSigned-off-by: somebody <somebody@somewhere>\n+\tEOF\n+\tgit interpret-trailers --only-trailers >actual <<-\\EOF &&\n+\tsubject\n+\n+\tbody\n+\n+\thttps://www.not-a-trailer.org\n+\thttps ://www.a-trailer.org\n+\thttps: //www.another-trailer.org\n+\tSigned-off-by: somebody <somebody@somewhere>\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/t/unit-tests/u-trailer.c b/t/unit-tests/u-trailer.c\nindex 3d60ea1603d..7404b165fac 100644\n--- a/t/unit-tests/u-trailer.c\n+++ b/t/unit-tests/u-trailer.c\n@@ -318,3 +318,55 @@ void test_trailer__one_non_trailer_no_git_trailers(void)\n \t\t\t   0,\n \t\t\t   expected_contents);\n }\n+\n+void test_trailer__URL(void)\n+{\n+\tstruct contents expected_contents[] = { 0 };\n+\n+\tt_trailer_iterator(\"Subject: foo bar\\n\"\n+\t\t\t   \"\\n\"\n+\t\t\t   /*\n+\t\t\t    * We do not want to match URLs as trailers.\n+\t\t\t    */\n+\t\t\t   \"https://www.example.org\\n\",\n+\t\t\t   0,\n+\t\t\t   expected_contents);\n+}\n+\n+void test_trailer__not_a_URL_space_after_separator(void)\n+{\n+\tstruct contents expected_contents[] = {\n+\t\t{ .raw = \"https: //www.example.org\\n\",\n+\t\t  .key = \"https\",\n+\t\t  .val = \"//www.example.org\" },\n+\t\t{ 0 },\n+\t};\n+\n+\tt_trailer_iterator(\"Subject: foo bar\\n\"\n+\t\t\t   \"\\n\"\n+\t\t\t   /*\n+\t\t\t    * This has a space after ':' so it's not a URL.\n+\t\t\t    */\n+\t\t\t   \"https: //www.example.org\\n\",\n+\t\t\t   1,\n+\t\t\t   expected_contents);\n+}\n+\n+void test_trailer__not_a_URL_space_before_separator(void)\n+{\n+\tstruct contents expected_contents[] = {\n+\t\t{ .raw = \"https ://www.example.org\\n\",\n+\t\t  .key = \"https\",\n+\t\t  .val = \"//www.example.org\" },\n+\t\t{ 0 },\n+\t};\n+\n+\tt_trailer_iterator(\"Subject: foo bar\\n\"\n+\t\t\t   \"\\n\"\n+\t\t\t   /*\n+\t\t\t    * This has a space before ':' so it's not a URL.\n+\t\t\t    */\n+\t\t\t   \"https ://www.example.org\\n\",\n+\t\t\t   1,\n+\t\t\t   expected_contents);\n+}\ndiff --git a/trailer.c b/trailer.c\nindex 6d8ec7fa8d8..10b1abebfbe 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -635,8 +635,12 @@ static ssize_t find_separator(const char *line, const char *separators)\n \tint whitespace_found = 0;\n \tconst char *c;\n \tfor (c = line; *c; c++) {\n-\t\tif (strchr(separators, *c))\n+\t\tif (strchr(separators, *c)) {\n+\t\t\t/* avoid accidental URL matches */\n+\t\t\tif (!whitespace_found && starts_with(c, \"://\"))\n+\t\t\t\treturn -1;\n \t\t\treturn c - line;\n+\t\t}\n \t\tif (!whitespace_found && (isalnum(*c) || *c == '-'))\n \t\t\tcontinue;\n \t\tif (c != line && (*c == ' ' || *c == '\\t')) {\n\nRange-diff against v1:\n1:  e7ba66a0ce3 ! 1:  2f8d10c1c6d trailers: stop recognizing URLs as trailers\n    @@ Commit message\n         † 6: There are only four commits in the Linux Kernel of this kind, and\n              three of them have the same recurring person in the signoff chain.\n     \n    +    ***\n    +\n    +    Note that this check has some benign false positives. A trailer key\n    +    can start with a digit, but a URL scheme can not start with a digit.\n    +    That means that a line that starts with `1://` will be rejected even\n    +    though it cannot be a URL. I don’t think this will reject any real\n    +    trailers, so I think the implementation simplicity is worth it.\n    +\n    +    And these false positives are just for a limited start fragment check;\n    +    a mere heuristic, not a URL parser.\n    +\n         Helped-by: Jeff King <peff@peff.net>\n         Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n     \n    @@ trailer.c: static ssize_t find_separator(const char *line, const char *separator\n      \tfor (c = line; *c; c++) {\n     -\t\tif (strchr(separators, *c))\n     +\t\tif (strchr(separators, *c)) {\n    -+\t\t\t/* avoid accidental URL matches (://) */\n    -+\t\t\tif (*c == ':' && c[1] == '/' && c[2] == '/' &&\n    -+\t\t\t    !whitespace_found)\n    ++\t\t\t/* avoid accidental URL matches */\n    ++\t\t\tif (!whitespace_found && starts_with(c, \"://\"))\n     +\t\t\t\treturn -1;\n      \t\t\treturn c - line;\n     +\t\t}\n\nbase-commit: 1a3e64c6c4a623626ff0687008732a8e007e2a1c\n-- \n2.55.0.13.g85d2d65e389\n\n"},{"id":"550966","messageId":"20260821004248.GA296777@coredump.intra.peff.net","threadId":"65758","inReplyTo":"c097cc44-3033-4f22-8c48-859de8353f99@app.fastmail.com","subject":"Re: [PATCH] trailers: stop recognizing URLs as trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-08-21T00:42:48Z","receivedAt":"2026-08-21T00:42:50Z","isPatch":true,"body":"On Thu, Aug 20, 2026 at 04:47:52PM +0200, Kristoffer Haugsbakk wrote:\n\n> > Can somebody summarize the outstanding issues on this topic (if\n> > any)?\n> \n> I have version 2 ready. The only code change is using `starts_with` like\n> Peff mentioned. What I wrote about the changes:\n\nYep, v2 looks great to me.\n\n-Peff\n"},{"id":"550978","messageId":"V3_URLs_not_trailers.bfc@msgid.xyz","threadId":"65758","inReplyTo":"URLs_not_trailers.b13@msgid.xyz","subject":"[PATCH v3] trailers: stop recognizing URLs as trailers","fromName":"","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-08-21T05:26:04Z","receivedAt":"2026-08-21T05:26:32Z","isPatch":true,"body":"From: Kristoffer Haugsbakk <code@khaugsbakk.name>\n\nAn HTTPS URL starts with an alphanumeric scheme followed by a colon.\nThat means that they will be recognized as trailers in a trailer block.\nThat turns out to be a problem in practice. Let’s stop recognizing these\nas trailers by failing the trailer parsing when we:\n\n1. find the separator;\n2. the separator and the next two characters form `://`; and\n3. we haven’t parsed any whitespace yet.\n\nThe simplest example of how this can be a problem is for people who do\nnot use trailers but may leave URLs at the end of the commit message.\nNow, while these authors might not use trailers themselves, other\nauthors may have used trailers and this metadata confusion can become a\nproblem once someone tries to extract that metadata (and non-metadata).\n\nLet’s now look at some examples in the Linux Kernel[1] to see how this\nis a problem in practice.\n\nThere are commits which contain intended non-trailer lines which start\nwith URLs. These are comments. Example with just the trailers:[2]\n\n    Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>\n    [bhelgaas: squash fixes:\n    https://lore.kernel.org/r/20260108013956.14351-2-bagasdotme@gmail.com\n    https://lore.kernel.org/r/20260108013956.14351-3-bagasdotme@gmail.com]\n    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>\n    Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>\n    Link: https://patch.msgid.link/20251210132907.58799-4-xueshuai@linux.alibaba.com\n\nThose `[]` pairs delimit the “squash fixes” comment.\n\nNow, any of these two commands:\n\n     git log --format='%(trailers:only)' -1 <commit>\n     git log -1 --format=%B <commit> |\n         git interpret-trailers --only-trailers\n\nWill both wrongly (according to the surmised user intent) include these\ntwo URL lines as trailers and also mangle the URLs, e.g.:\n\n    https: //lore.kernel.org/r/20260108013956.14351-2-bagasdotme@gmail.com\n\nBecause the `--only-trailers` mode (or `only` for the git-log(1) format)\nnormalizes the output to a colon and a space.\n\nAnother example is linewrapping mistakes; a `Link` trailer with a\nURL where the URL ended up on the next line, presumably because the\nuser’s editor linewrapped the “too long” line. Example with just the\ntrailers:[3]\n\n    Link: https://patch.msgid.link/20260216-work-xattr-socket-v1-4-c2efa4f74cb7@kernel.org\n    Link:\n    https://lore.kernel.org/3cnmtqmakpbb2uwhenrj7kdqu3uefykiykjllgfbtpkiwhaa4s@sghkevv7jned [1]\n    Acked-by: Darrick J. Wong <djwong@kernel.org>\n    Reviewed-by: Jan Kara <jack@suse.cz>\n    Signed-off-by: Christian Brauner <brauner@kernel.org>\n\nNow, this intended trailer is already ruined, but interpreting the URL\nas a standalone trailer only compounds the mistake.\n\nYet another example is the trailer machinery normalizing the trailer\nblock before application, resulting in a `https` trailer key in the\ncommit message itself. Example with just the trailers:[4]\n\n    https: //sashiko.dev/#/patchset/20260429114208.941011-1-holger.brunck%40hitachienergy.com\n    Fixes: c19b6d246a35 (\"drivers/net: support hdlc function for QE-UCC\")\n    Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>\n    Link: https://patch.msgid.link/20260507155332.3452319-1-holger.brunck@hitachienergy.com\n    Signed-off-by: Jakub Kicinski <kuba@kernel.org>\n\nWe have a helpful `Link` that points to the original patch.[5] Following\nit we can see that that `https` trailer was indeed a URL\noriginally (again just the trailer block here):\n\n    https://sashiko.dev/#/patchset/20260429114208.941011-1-holger.brunck%40hitachienergy.com\n    Fixes: c19b6d246a35 (\"drivers/net: support hdlc function for QE-UCC\")\n    Signed-off-by: Holger Brunck <holger.brunck@hitachienergy.com>\n\nSo how did it end up as a `https` trailer? My theory is that the trailer\nblock was normalized on patch application, causing a URL comment to be\nwrongly normalized and cemented in the commit message as a trailer.[6]\n\n† 1: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/\n† 2: commit 8236fc613d44e59f6736d6c3e9efffaf26ab7f00\n† 3: commit 5bd97f5c5f241a5610c4412d1b93995a26241f81\n† 4: commit 496c0c4c53bbe1bad97e82cd12103df61a6e459d\n† 5: https://patch.msgid.link/20260507155332.3452319-1-holger.brunck@hitachienergy.com\n† 6: There are only four commits in the Linux Kernel of this kind, and\n     three of them have the same recurring person in the signoff chain.\n\n***\n\nNote that this check has some benign false positives. A trailer key\ncan start with a digit, but a URL scheme can not start with a digit.\nThat means that a line that starts with `1://` will be rejected even\nthough it cannot be a URL. I don’t think this will reject any real\ntrailers, so I think the implementation simplicity is worth it.\n\nAnd these false positives are just for a limited start fragment check;\na mere heuristic, not a URL parser.\n\nHelped-by: Jeff King <peff@peff.net>\nAcked-by: Jeff King <peff@peff.net>\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n\nNotes (series):\n    Topic name (applied): trailers-no-urls\n    \n    Topic summary: Stop recognizing URLs in trailer blocks as trailers.\n    \n    § Link to v2\n    \n    https://lore.kernel.org/git/V2_URLs_not_trailers.bf3@msgid.xyz/\n    \n    § Changes in v2\n    \n    • Add Ack https://lore.kernel.org/git/20260821004248.GA296777@coredump.intra.peff.net/\n\n Documentation/git-interpret-trailers.adoc | 13 ++++--\n t/t7513-interpret-trailers.sh             | 19 +++++++++\n t/unit-tests/u-trailer.c                  | 52 +++++++++++++++++++++++\n trailer.c                                 |  6 ++-\n 4 files changed, 86 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.adoc b/Documentation/git-interpret-trailers.adoc\nindex b4988d39eab..903d598dcb0 100644\n--- a/Documentation/git-interpret-trailers.adoc\n+++ b/Documentation/git-interpret-trailers.adoc\n@@ -123,9 +123,16 @@ OTHER RULES\n What was covered in the previous section are the rules that are relevant\n for regular use. The following points are included for completeness.\n \n-This command ignores comment lines (see `core.commentString` in\n-linkgit:git-config[1]). This is for use with the `prepare-commit-msg`\n-and `commit-msg` hooks.\n+--\n+* This command ignores comment lines (see `core.commentString` in\n+  linkgit:git-config[1]). This is for use with the `prepare-commit-msg`\n+  and `commit-msg` hooks.\n+\n+* Candidate trailer lines that have `:` as the separator, that have no\n+  whitespace before the value part, and that start with `//` are not\n+  recognized as trailers. This is to avoid accidentally interpreting\n+  URLs as trailers (e.g. lines that start with `https://`).\n+--\n \n OPTIONS\n -------\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 818a8dafbd2..e3555b6d51d 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1989,4 +1989,23 @@ test_expect_success 'handling of --- lines in conjunction with cut-lines' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'URLs and lines that are not quite URLs' '\n+\tcat >expect <<-\\EOF &&\n+\thttps: //www.a-trailer.org\n+\thttps: //www.another-trailer.org\n+\tSigned-off-by: somebody <somebody@somewhere>\n+\tEOF\n+\tgit interpret-trailers --only-trailers >actual <<-\\EOF &&\n+\tsubject\n+\n+\tbody\n+\n+\thttps://www.not-a-trailer.org\n+\thttps ://www.a-trailer.org\n+\thttps: //www.another-trailer.org\n+\tSigned-off-by: somebody <somebody@somewhere>\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/t/unit-tests/u-trailer.c b/t/unit-tests/u-trailer.c\nindex 3d60ea1603d..7404b165fac 100644\n--- a/t/unit-tests/u-trailer.c\n+++ b/t/unit-tests/u-trailer.c\n@@ -318,3 +318,55 @@ void test_trailer__one_non_trailer_no_git_trailers(void)\n \t\t\t   0,\n \t\t\t   expected_contents);\n }\n+\n+void test_trailer__URL(void)\n+{\n+\tstruct contents expected_contents[] = { 0 };\n+\n+\tt_trailer_iterator(\"Subject: foo bar\\n\"\n+\t\t\t   \"\\n\"\n+\t\t\t   /*\n+\t\t\t    * We do not want to match URLs as trailers.\n+\t\t\t    */\n+\t\t\t   \"https://www.example.org\\n\",\n+\t\t\t   0,\n+\t\t\t   expected_contents);\n+}\n+\n+void test_trailer__not_a_URL_space_after_separator(void)\n+{\n+\tstruct contents expected_contents[] = {\n+\t\t{ .raw = \"https: //www.example.org\\n\",\n+\t\t  .key = \"https\",\n+\t\t  .val = \"//www.example.org\" },\n+\t\t{ 0 },\n+\t};\n+\n+\tt_trailer_iterator(\"Subject: foo bar\\n\"\n+\t\t\t   \"\\n\"\n+\t\t\t   /*\n+\t\t\t    * This has a space after ':' so it's not a URL.\n+\t\t\t    */\n+\t\t\t   \"https: //www.example.org\\n\",\n+\t\t\t   1,\n+\t\t\t   expected_contents);\n+}\n+\n+void test_trailer__not_a_URL_space_before_separator(void)\n+{\n+\tstruct contents expected_contents[] = {\n+\t\t{ .raw = \"https ://www.example.org\\n\",\n+\t\t  .key = \"https\",\n+\t\t  .val = \"//www.example.org\" },\n+\t\t{ 0 },\n+\t};\n+\n+\tt_trailer_iterator(\"Subject: foo bar\\n\"\n+\t\t\t   \"\\n\"\n+\t\t\t   /*\n+\t\t\t    * This has a space before ':' so it's not a URL.\n+\t\t\t    */\n+\t\t\t   \"https ://www.example.org\\n\",\n+\t\t\t   1,\n+\t\t\t   expected_contents);\n+}\ndiff --git a/trailer.c b/trailer.c\nindex 6d8ec7fa8d8..10b1abebfbe 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -635,8 +635,12 @@ static ssize_t find_separator(const char *line, const char *separators)\n \tint whitespace_found = 0;\n \tconst char *c;\n \tfor (c = line; *c; c++) {\n-\t\tif (strchr(separators, *c))\n+\t\tif (strchr(separators, *c)) {\n+\t\t\t/* avoid accidental URL matches */\n+\t\t\tif (!whitespace_found && starts_with(c, \"://\"))\n+\t\t\t\treturn -1;\n \t\t\treturn c - line;\n+\t\t}\n \t\tif (!whitespace_found && (isalnum(*c) || *c == '-'))\n \t\t\tcontinue;\n \t\tif (c != line && (*c == ' ' || *c == '\\t')) {\n\nInterdiff against v2:\n\nRange-diff against v2:\n1:  2f8d10c1c6d ! 1:  736610daf6e trailers: stop recognizing URLs as trailers\n    @@ Commit message\n         a mere heuristic, not a URL parser.\n     \n         Helped-by: Jeff King <peff@peff.net>\n    +    Acked-by: Jeff King <peff@peff.net>\n         Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n     \n      ## Documentation/git-interpret-trailers.adoc ##\n\nbase-commit: 1a3e64c6c4a623626ff0687008732a8e007e2a1c\n-- \n2.55.0.13.g85d2d65e389\n\n"},{"id":"550979","messageId":"6dfeeec6-c435-4408-8160-d7707806feac@app.fastmail.com","threadId":"65758","inReplyTo":"V3_URLs_not_trailers.bfc@msgid.xyz","subject":"Re: [PATCH v3] trailers: stop recognizing URLs as trailers","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-08-21T05:28:02Z","receivedAt":"2026-08-21T05:28:29Z","isPatch":true,"body":"On Fri, Aug 21, 2026, at 07:26, kristofferhaugsbakk@fastmail.com wrote:\n> From: Kristoffer Haugsbakk <code@khaugsbakk.name>\n>[snip]\n>     § Changes in v2\n>    \n>     • Add Ack https://lore.kernel.org/git/20260821004248.GA296777@coredump.intra.peff.net/\n>\n\nSorry: changes in *v3*.\n"},{"id":"551029","messageId":"xmqqecfrqxic.fsf@gitster.g","threadId":"65758","inReplyTo":"V3_URLs_not_trailers.bfc@msgid.xyz","subject":"Re: [PATCH v3] trailers: stop recognizing URLs as trailers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-21T15:55:07Z","receivedAt":"2026-08-21T15:55:10Z","isPatch":true,"body":"kristofferhaugsbakk@fastmail.com writes:\n\n> Notes (series):\n>     Topic name (applied): trailers-no-urls\n>     \n>     Topic summary: Stop recognizing URLs in trailer blocks as trailers.\n>     \n>     § Link to v2\n>     \n>     https://lore.kernel.org/git/V2_URLs_not_trailers.bf3@msgid.xyz/\n>     \n>     § Changes in v2\n>     \n>     • Add Ack https://lore.kernel.org/git/20260821004248.GA296777@coredump.intra.peff.net/\n\nThat's a change in v3, and the only one.  v2 was already good so let\nme mark it for 'next'.\n\nThanks.\n"}]}