From 22c5bff037ef3da51ec11139910e0bd20d2490a0 Mon Sep 17 00:00:00 2001 From: Seongmin Lee Date: Tue, 21 Jul 2026 22:53:54 +0900 Subject: [PATCH] knotmirror/xrpc/gitea: parse mergetag commit header Signed-off-by: Seongmin Lee --- knotmirror/xrpc/gitea/batch.go | 114 ++++++++++++--------------- knotmirror/xrpc/gitea/commit_test.go | 51 +++++++++++- 2 files changed, 98 insertions(+), 67 deletions(-) diff --git a/knotmirror/xrpc/gitea/batch.go b/knotmirror/xrpc/gitea/batch.go index 8d17b5d2..4008a84b 100644 --- a/knotmirror/xrpc/gitea/batch.go +++ b/knotmirror/xrpc/gitea/batch.go @@ -228,91 +228,75 @@ func ReadCommit(oid plumbing.Hash, reader io.Reader) (*object.Commit, error) { ExtraHeaders: make(map[string][]byte), } - payloadSB := new(strings.Builder) - signatureSB := new(strings.Builder) - messageSB := new(strings.Builder) - firstLine := true - message := false - pgpsig := false - - bufReader, ok := reader.(*bufio.Reader) - if !ok { - bufReader = bufio.NewReader(reader) - } - -readLoop: + bufReader := bufio.NewReader(reader) + inHeader := true + var payloadSB, messageSB bytes.Buffer + var headerKey string + var headerValue []byte for { line, err := bufReader.ReadBytes('\n') - if err != nil { - if err == io.EOF { - if message { - _, _ = messageSB.Write(line) - } - _, _ = payloadSB.Write(line) - break readLoop - } + if err != nil && err != io.EOF { return nil, err } - if pgpsig { - if len(line) > 0 && line[0] == ' ' { - _, _ = signatureSB.Write(line[1:]) - continue - } - pgpsig = false + if len(line) == 0 { + break } - if !message { - // This is probably not correct but is copied from go-gits interpretation... - trimmed := bytes.TrimSpace(line) - if len(trimmed) == 0 { - message = true - _, _ = payloadSB.Write(line) - continue + if inHeader { + inHeader = !(len(line) == 1 && line[0] == '\n') // a bare newline ends the header block + k, v, _ := bytes.Cut(line, []byte{' '}) + if len(k) != 0 || !inHeader { + if headerKey != "" { + assignCommitHeader(commit, headerKey, headerValue) + } + headerKey = string(k) // also resets headerValue via the assignment below + headerValue = v + } else { + headerValue = append(headerValue, v...) } - - k, data, _ := bytes.Cut(line, []byte{' '}) - - switch string(k) { - case "tree": - commit.TreeHash = plumbing.NewHash(string(data)) - _, _ = payloadSB.Write(line) - case "parent": - commit.ParentHashes = append(commit.ParentHashes, plumbing.NewHash(string(data))) - _, _ = payloadSB.Write(line) - case "author": - commit.Author.Decode(data) + if headerKey != "gpgsig" && headerKey != "gpgsig-sha256" { _, _ = payloadSB.Write(line) - case "committer": - commit.Committer.Decode(data) - _, _ = payloadSB.Write(line) - case "gpgsig": - fallthrough - case "gpgsig-sha256": // FIXME: no intertop, so only 1 exists at present. - _, _ = signatureSB.Write(data) - _ = signatureSB.WriteByte('\n') - pgpsig = true - default: - commit.ExtraHeaders[string(k)] = bytes.TrimSpace(data) - // If the first line is not any of the known headers, then it is probably the prefix added when git cat-file is called with --batch, and that is not part of the payload - if !firstLine { - // Every subsequent header field is added to the payload - _, _ = payloadSB.Write(line) - } } } else { _, _ = messageSB.Write(line) _, _ = payloadSB.Write(line) } - firstLine = false + if err == io.EOF { + break + } } + commit.Message = messageSB.String() // TODO: pass raw payload so we can verify it without reconstructing the payload - commit.PGPSignature = signatureSB.String() - + // if commit.Signature != nil { + // commit.Signature.Payload = payloadSB.String() + // } return commit, nil } +func assignCommitHeader(commit *object.Commit, headerKey string, headerValue []byte) { + value := bytes.TrimSuffix(headerValue, []byte{'\n'}) + switch headerKey { + case "tree": + commit.TreeHash = plumbing.NewHash(string(value)) + case "parent": + commit.ParentHashes = append(commit.ParentHashes, plumbing.NewHash(string(value))) + case "author": + commit.Author.Decode(value) + case "committer": + commit.Committer.Decode(value) + case "gpgsig", "gpgsig-sha256": + // if there are duplicate "gpgsig" and "gpgsig-sha256" headers, then the signature must have already been invalid + // so we don't need to handle duplicate headers here + commit.PGPSignature = string(value) + case "mergetag": + commit.MergeTag = string(value) + default: + commit.ExtraHeaders[headerKey] = value + } +} + // ParseCatFileTreeLine reads an entry from a tree in a cat-file --batch stream // This carefully avoids allocations - except where fnameBuf is too small. // It is recommended therefore to pass in an fnameBuf large enough to avoid almost all allocations diff --git a/knotmirror/xrpc/gitea/commit_test.go b/knotmirror/xrpc/gitea/commit_test.go index bf97affc..1b578866 100644 --- a/knotmirror/xrpc/gitea/commit_test.go +++ b/knotmirror/xrpc/gitea/commit_test.go @@ -19,6 +19,7 @@ parent 37991dec2c8e592043f47155ce4808d4580f9123 author silverwind 1563741793 +0200 committer silverwind 1563741793 +0200 gpgsig -----BEGIN PGP SIGNATURE----- +` + " " + ` iQIzBAABCAAdFiEEWPb2jX6FS2mqyJRQLmK0HJOGlEMFAl00zmEACgkQLmK0HJOG lEMDFBAAhQKKqLD1VICygJMEB8t1gBmNLgvziOLfpX4KPWdPtBk3v/QJ7OrfMrVK xlC4ZZyx6yMm1Q7GzmuWykmZQJ9HMaHJ49KAbh5MMjjV/+OoQw9coIdo8nagRUld @@ -61,8 +62,7 @@ sD53z/f0J+We4VZjY+pidvA9BGZPFVdR3wd3xGs8/oH6UWaLJAMGkLG6dDb3qDLm mfeFhT57UbE4qukTDIQ0Y0WM40UYRTakRaDY7ubhXgLgx09Cnp9XTVMsHgT6j9/i 1pxsB104XLWjQHTjr1JtiaBQEwFh9r2OKTcpvaLcbNtYpo7CzOs= =FRsO ------END PGP SIGNATURE----- -`, commitFromReader.PGPSignature) +-----END PGP SIGNATURE-----`, commitFromReader.PGPSignature) assert.Equal(t, `tree f1a6cb52b2d16773290cefe49ad0684b50a4f930 parent 37991dec2c8e592043f47155ce4808d4580f9123 author silverwind 1563741793 +0200 @@ -71,3 +71,50 @@ committer silverwind 1563741793 +0200 empty commit`, tcommit.Payload()) assert.Equal(t, "silverwind ", commitFromReader.Author.String()) } + +func TestCommitFromReaderMergeTag(t *testing.T) { + // Built with explicit "\n" concatenation, not a backtick literal: the blank + // mergetag continuation lines are " \n" (space + newline) and gofmt/editors + // strip trailing whitespace from raw literals, which would corrupt the input. + commitString := "tree 635dfb8e1e9d4d75855cc23eb28d35533f55b42f\n" + + "parent c1fa0bb633e4a6b11e83ffc57fa5abe8ebb87891\n" + + "parent 8f80b5b227ef9ea422080487715c841856339aed\n" + + "author Linus Torvalds 1778539129 -0700\n" + + "committer Linus Torvalds 1778539129 -0700\n" + + "mergetag object 8f80b5b227ef9ea422080487715c841856339aed\n" + + " type commit\n" + + " tag linux_kselftest-kunit-fixes-7.1-rc4\n" + + " tagger Shuah Khan 1778535878 -0600\n" + + " \n" + // blank continuation line inside the mergetag: the bug trigger + " linux_kselftest-kunit-fixes-7.1-rc4\n" + + " \n" + + " Fix to decouple KUNIT_DEBUGFS and KUNIT_ALL_TESTS options.\n" + + " -----BEGIN PGP SIGNATURE-----\n" + + " \n" + + " iQIzBAABCgAdFiEEPZKym/RZuOCGeA/kCwJExA0NQxwFAmoCUf0ACgkQCwJExA0N\n" + + " =QdSk\n" + + " -----END PGP SIGNATURE-----\n" + + "\n" + // real header/message separator: truly empty line + "Merge tag 'linux_kselftest-kunit-fixes-7.1-rc4' of git://example\n" + + "\n" + + "Pull kunit fixes from Shuah Khan" + + sha := plumbing.NewHash("50897c955902c93ae71c38698abb910525ebdc89") + + c, err := ReadCommit(sha, strings.NewReader(commitString)) + require.NoError(t, err) + require.NotNil(t, c) + + // Message must be only the merge message, not polluted with the tag body/signature. + assert.Equal(t, "Merge tag 'linux_kselftest-kunit-fixes-7.1-rc4' of git://example\n\nPull kunit fixes from Shuah Khan", c.Message) + + // mergetag is captured, and this commit itself is not gpg-signed. + assert.Contains(t, c.MergeTag, "type commit\n") + assert.Contains(t, c.MergeTag, "-----END PGP SIGNATURE-----") + assert.Empty(t, c.PGPSignature) + + // The broken parser dumped stray continuation lines into ExtraHeaders[""]. + assert.NotContains(t, c.ExtraHeaders, "") + + assert.Equal(t, "Linus Torvalds ", c.Author.String()) +} -- 2.51.2