git-interpret-trailers and period characters in the key

git-interpret-trailers and period characters in the key

[Brendan Jackman]

Hi folks,

Just debugging one of my scripts and I found that
git-interpret-trailers behaves surprisingly on trailer keys containing
'.' characters:

❯❯  cat commit.txt
my commit title

My-Footer: foo

❯❯  git interpret-trailers --parse commit.txt
My-Footer: foo

❯❯  cat commit2.txt
my commit title

My-Footer: foo
My-6.11-Version: bar

❯❯  git interpret-trailers --parse commit.txt

Basically, as soon as any trailer key contains a period (which in my
case, it does because the trailer keys refer to versions of of
software, i.e. "this commit was backported from the following Linux
kernel commit which appeared in version 6.1"), it stops parsing the
trailer block.

My guess is that this is just that it doesn't allow periods in the
trailer key, and once there's one line in the block that isn't a
trailer, it no longer meets the requirements described in the man
page.

I can't find anything in the man page about why the period character
should break this. Am I missing anything there?

Cheers,
Brendan

❯❯  git --version
git version 2.49.0.472.ge94155a9ec-goog

(IIUC that -goog in the version string is just noting that we have
monitoring logic added to our internal Git built to spot people
leaking IP, there's no actual feature customisation)

Re: git-interpret-trailers and period characters in the key

[Christian Couder]

Hi,

On Tue, Apr 1, 2025 at 3:27 PM Brendan Jackman <jackmanb@google.com> wrote:

> Basically, as soon as any trailer key contains a period (which in my
> case, it does because the trailer keys refer to versions of of
> software, i.e. "this commit was backported from the following Linux
> kernel commit which appeared in version 6.1"), it stops parsing the
> trailer block.
>
> My guess is that this is just that it doesn't allow periods in the
> trailer key, and once there's one line in the block that isn't a
> trailer, it no longer meets the requirements described in the man
> page.

Yeah, it's also my guess that the trailer block is not considered a
trailer block anymore as your trailer key is not considered a valid
trailer key.

> I can't find anything in the man page about why the period character
> should break this. Am I missing anything there?

We tried to be quite strict when implementing trailers to avoid
regular text to be too easily considered trailers.

Having a config option or something to be a bit more lenient and
accept more characters in trailer keys could help some people, and it
might not be very difficult to implement. On the other hand if people
start to have a lot of weird trailers around, and abuse the config
option to make it too lenient, then it could be a bad thing in general
as more and more regular text might be interpreted as trailers.

I also agree that our doc about this could be improved. Patches welcome.

Best,
Christian.

Re: git-interpret-trailers and period characters in the key

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

> Having a config option or something to be a bit more lenient and
> accept more characters in trailer keys could help some people, and it
> might not be very difficult to implement. On the other hand if people
> start to have a lot of weird trailers around, and abuse the config
> option to make it too lenient, then it could be a bad thing in general
> as more and more regular text might be interpreted as trailers.
>
> I also agree that our doc about this could be improved. Patches welcome.

Thanks for a concise summary.  

I agree that loosening the rule, or even adding an option to loosen
the rule, is detrimental to the ecosystem at large, and
documentation can be improved.

In retrospect, I supsect that it even was a mistake to special case
the #BUGID syntax when the trailer was pretty much about lines that
look similar to E-Mail-Header: fields.  Let's not make it worse.

Thanks.