From ea1d09718ce2e3bf043c60bae76dd6bd7e84e9fe Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 2 Mar 2023 06:32:21 +0100 Subject: [PATCH] Fix commit retrieval by tag (#21804) It is not correct to return tag data when commit data is requested, so remove the hacky code that overwrote parts of a commit with parts of a tag. This fixes commit retrieval by tag for both the latest commit in the UI and the commit info on tag webhook events. Fixes: https://github.com/go-gitea/gitea/issues/21687 Replaces: https://github.com/go-gitea/gitea/pull/21693 Screenshot 2022-11-13 at 15 26 37 image --------- Co-authored-by: Lunny Xiao --- modules/git/repo_commit_gogit.go | 39 ------------------ modules/git/repo_commit_nogogit.go | 4 -- modules/git/repo_commit_test.go | 5 ++- modules/git/repo_ref_test.go | 12 ++++-- modules/git/repo_stats_test.go | 4 +- modules/git/repo_tag_test.go | 9 ++-- modules/git/repo_test.go | 6 +-- modules/git/tests/repos/repo1_bare/index | Bin 0 -> 65 bytes modules/git/tests/repos/repo1_bare/logs/HEAD | 1 + .../repos/repo1_bare/logs/refs/heads/master | 1 + .../1c/91d130dc5fb75fd2d9f586a058650889cfe7fb | Bin 0 -> 813 bytes .../28/b55526e7100924d864dd89e35c1ea62e7a5a32 | Bin 0 -> 818 bytes .../36/f97d9a96457e2bab511db30fe2db03893ebc64 | Bin 0 -> 770 bytes .../4b/825dc642cb6eb9a060e54bf8d69288fbee4904 | Bin 0 -> 15 bytes .../93/3305878a3c9ad485c29b87fb662a73a9675c4b | Bin 0 -> 770 bytes .../ce/064814f4a0d337b333e646ece456cd39fab612 | Bin 0 -> 815 bytes .../cf/8b0b492a950b358a7ce7f9d01b18aef48a6b2d | Bin 0 -> 827 bytes .../tests/repos/repo1_bare/refs/heads/master | 2 +- .../repos/repo1_bare/refs/tags/signed-tag | 1 + 19 files changed, 26 insertions(+), 58 deletions(-) create mode 100644 modules/git/tests/repos/repo1_bare/index create mode 100644 modules/git/tests/repos/repo1_bare/objects/1c/91d130dc5fb75fd2d9f586a058650889cfe7fb create mode 100644 modules/git/tests/repos/repo1_bare/objects/28/b55526e7100924d864dd89e35c1ea62e7a5a32 create mode 100644 modules/git/tests/repos/repo1_bare/objects/36/f97d9a96457e2bab511db30fe2db03893ebc64 create mode 100644 modules/git/tests/repos/repo1_bare/objects/4b/825dc642cb6eb9a060e54bf8d69288fbee4904 create mode 100644 modules/git/tests/repos/repo1_bare/objects/93/3305878a3c9ad485c29b87fb662a73a9675c4b create mode 100644 modules/git/tests/repos/repo1_bare/objects/ce/064814f4a0d337b333e646ece456cd39fab612 create mode 100644 modules/git/tests/repos/repo1_bare/objects/cf/8b0b492a950b358a7ce7f9d01b18aef48a6b2d create mode 100644 modules/git/tests/repos/repo1_bare/refs/tags/signed-tag diff --git a/modules/git/repo_commit_gogit.go b/modules/git/repo_commit_gogit.go index 72de158e6..ce0af9361 100644 --- a/modules/git/repo_commit_gogit.go +++ b/modules/git/repo_commit_gogit.go @@ -7,7 +7,6 @@ package git import ( - "fmt" "strings" "github.com/go-git/go-git/v5/plumbing" @@ -67,38 +66,6 @@ func (repo *Repository) IsCommitExist(name string) bool { return err == nil } -func convertPGPSignatureForTag(t *object.Tag) *CommitGPGSignature { - if t.PGPSignature == "" { - return nil - } - - var w strings.Builder - var err error - - if _, err = fmt.Fprintf(&w, - "object %s\ntype %s\ntag %s\ntagger ", - t.Target.String(), t.TargetType.Bytes(), t.Name); err != nil { - return nil - } - - if err = t.Tagger.Encode(&w); err != nil { - return nil - } - - if _, err = fmt.Fprintf(&w, "\n\n"); err != nil { - return nil - } - - if _, err = fmt.Fprintf(&w, t.Message); err != nil { - return nil - } - - return &CommitGPGSignature{ - Signature: t.PGPSignature, - Payload: strings.TrimSpace(w.String()) + "\n", - } -} - func (repo *Repository) getCommit(id SHA1) (*Commit, error) { var tagObject *object.Tag @@ -122,12 +89,6 @@ func (repo *Repository) getCommit(id SHA1) (*Commit, error) { commit := convertCommit(gogitCommit) commit.repo = repo - if tagObject != nil { - commit.CommitMessage = strings.TrimSpace(tagObject.Message) - commit.Author = &tagObject.Tagger - commit.Signature = convertPGPSignatureForTag(tagObject) - } - tree, err := gogitCommit.Tree() if err != nil { return nil, err diff --git a/modules/git/repo_commit_nogogit.go b/modules/git/repo_commit_nogogit.go index 7373d01c8..d5eb72310 100644 --- a/modules/git/repo_commit_nogogit.go +++ b/modules/git/repo_commit_nogogit.go @@ -107,10 +107,6 @@ func (repo *Repository) getCommitFromBatchReader(rd *bufio.Reader, id SHA1) (*Co return nil, err } - commit.CommitMessage = strings.TrimSpace(tag.Message) - commit.Author = tag.Tagger - commit.Signature = tag.Signature - return commit, nil case "commit": commit, err := CommitFromReader(repo, id, io.LimitReader(rd, size)) diff --git a/modules/git/repo_commit_test.go b/modules/git/repo_commit_test.go index af8c0592f..729fb0ba1 100644 --- a/modules/git/repo_commit_test.go +++ b/modules/git/repo_commit_test.go @@ -43,12 +43,13 @@ func TestGetTagCommitWithSignature(t *testing.T) { assert.NoError(t, err) defer bareRepo1.Close() - commit, err := bareRepo1.GetCommit("3ad28a9149a2864384548f3d17ed7f38014c9e8a") + // both the tag and the commit are signed here, this validates only the commit signature + commit, err := bareRepo1.GetCommit("28b55526e7100924d864dd89e35c1ea62e7a5a32") assert.NoError(t, err) assert.NotNil(t, commit) assert.NotNil(t, commit.Signature) // test that signature is not in message - assert.Equal(t, "tag", commit.CommitMessage) + assert.Equal(t, "signed-commit\n", commit.CommitMessage) } func TestGetCommitWithBadCommitID(t *testing.T) { diff --git a/modules/git/repo_ref_test.go b/modules/git/repo_ref_test.go index 776d7ce3e..c08ea1276 100644 --- a/modules/git/repo_ref_test.go +++ b/modules/git/repo_ref_test.go @@ -19,13 +19,14 @@ func TestRepository_GetRefs(t *testing.T) { refs, err := bareRepo1.GetRefs() assert.NoError(t, err) - assert.Len(t, refs, 5) + assert.Len(t, refs, 6) expectedRefs := []string{ BranchPrefix + "branch1", BranchPrefix + "branch2", BranchPrefix + "master", TagPrefix + "test", + TagPrefix + "signed-tag", NotesRef, } @@ -43,9 +44,12 @@ func TestRepository_GetRefsFiltered(t *testing.T) { refs, err := bareRepo1.GetRefsFiltered(TagPrefix) assert.NoError(t, err) - if assert.Len(t, refs, 1) { - assert.Equal(t, TagPrefix+"test", refs[0].Name) + if assert.Len(t, refs, 2) { + assert.Equal(t, TagPrefix+"signed-tag", refs[0].Name) assert.Equal(t, "tag", refs[0].Type) - assert.Equal(t, "3ad28a9149a2864384548f3d17ed7f38014c9e8a", refs[0].Object.String()) + assert.Equal(t, "36f97d9a96457e2bab511db30fe2db03893ebc64", refs[0].Object.String()) + assert.Equal(t, TagPrefix+"test", refs[1].Name) + assert.Equal(t, "tag", refs[1].Type) + assert.Equal(t, "3ad28a9149a2864384548f3d17ed7f38014c9e8a", refs[1].Object.String()) } } diff --git a/modules/git/repo_stats_test.go b/modules/git/repo_stats_test.go index 668ed6799..3d032385e 100644 --- a/modules/git/repo_stats_test.go +++ b/modules/git/repo_stats_test.go @@ -24,9 +24,9 @@ func TestRepository_GetCodeActivityStats(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, code) - assert.EqualValues(t, 9, code.CommitCount) + assert.EqualValues(t, 10, code.CommitCount) assert.EqualValues(t, 3, code.AuthorCount) - assert.EqualValues(t, 9, code.CommitCountInAllBranches) + assert.EqualValues(t, 10, code.CommitCountInAllBranches) assert.EqualValues(t, 10, code.Additions) assert.EqualValues(t, 1, code.Deletions) assert.Len(t, code.Authors, 3) diff --git a/modules/git/repo_tag_test.go b/modules/git/repo_tag_test.go index 589349a72..8ccfec3eb 100644 --- a/modules/git/repo_tag_test.go +++ b/modules/git/repo_tag_test.go @@ -25,11 +25,14 @@ func TestRepository_GetTags(t *testing.T) { assert.NoError(t, err) return } - assert.Len(t, tags, 1) + assert.Len(t, tags, 2) assert.Equal(t, len(tags), total) - assert.EqualValues(t, "test", tags[0].Name) - assert.EqualValues(t, "3ad28a9149a2864384548f3d17ed7f38014c9e8a", tags[0].ID.String()) + assert.EqualValues(t, "signed-tag", tags[0].Name) + assert.EqualValues(t, "36f97d9a96457e2bab511db30fe2db03893ebc64", tags[0].ID.String()) assert.EqualValues(t, "tag", tags[0].Type) + assert.EqualValues(t, "test", tags[1].Name) + assert.EqualValues(t, "3ad28a9149a2864384548f3d17ed7f38014c9e8a", tags[1].ID.String()) + assert.EqualValues(t, "tag", tags[1].Type) } func TestRepository_GetTag(t *testing.T) { diff --git a/modules/git/repo_test.go b/modules/git/repo_test.go index 2a3914819..044b9d406 100644 --- a/modules/git/repo_test.go +++ b/modules/git/repo_test.go @@ -14,10 +14,10 @@ func TestGetLatestCommitTime(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") lct, err := GetLatestCommitTime(DefaultContext, bareRepo1Path) assert.NoError(t, err) - // Time is Sun Jul 21 22:43:13 2019 +0200 + // Time is Sun Nov 13 16:40:14 2022 +0100 // which is the time of commit - // feaf4ba6bc635fec442f46ddd4512416ec43c2c2 (refs/heads/master) - assert.EqualValues(t, 1563741793, lct.Unix()) + // ce064814f4a0d337b333e646ece456cd39fab612 (refs/heads/master) + assert.EqualValues(t, 1668354014, lct.Unix()) } func TestRepoIsEmpty(t *testing.T) { diff --git a/modules/git/tests/repos/repo1_bare/index b/modules/git/tests/repos/repo1_bare/index new file mode 100644 index 0000000000000000000000000000000000000000..65d675154f23ffb2d0196e017d44a5e7017550f5 GIT binary patch literal 65 zcmZ?q402{*U|<4bhL9jvS0E+HV4z^Y<=qr}%;|LA&IJiiy? 1563741799 +0200 push +feaf4ba6bc635fec442f46ddd4512416ec43c2c2 ce064814f4a0d337b333e646ece456cd39fab612 silverwind 1668354026 +0100 push diff --git a/modules/git/tests/repos/repo1_bare/logs/refs/heads/master b/modules/git/tests/repos/repo1_bare/logs/refs/heads/master index cef4ca2dc..46da5fe0b 100644 --- a/modules/git/tests/repos/repo1_bare/logs/refs/heads/master +++ b/modules/git/tests/repos/repo1_bare/logs/refs/heads/master @@ -1 +1,2 @@ 37991dec2c8e592043f47155ce4808d4580f9123 feaf4ba6bc635fec442f46ddd4512416ec43c2c2 silverwind 1563741799 +0200 push +feaf4ba6bc635fec442f46ddd4512416ec43c2c2 ce064814f4a0d337b333e646ece456cd39fab612 silverwind 1668354026 +0100 push diff --git a/modules/git/tests/repos/repo1_bare/objects/1c/91d130dc5fb75fd2d9f586a058650889cfe7fb b/modules/git/tests/repos/repo1_bare/objects/1c/91d130dc5fb75fd2d9f586a058650889cfe7fb new file mode 100644 index 0000000000000000000000000000000000000000..fb50b65f9ff04e722d4df5c734d8d64eb761f61a GIT binary patch literal 813 zcmV+|1Je9>0hN=$TCJP2Xxf!d&!#WIA07Sc z-}rko{0TTwlo3WUgn)${giP`CznT8;VLjH5rUu{sK1DZeAB1KIV%zk|TN3F%IRhBb z9PLkq5Je@VG#Xt$hdF;uMI+|B`^$-rht7qT6PT^;8KmNjgm2tCMa{5fZwl-At?ceGLjp0z2VhpHLt`~~|T~IpX0N;HyZB}>LdTjFq zF7l@L56{<9jqruT;wT6UKle|G7x5{QKjX-fT;KL)2Am#GHi~cTebB~Cf59;O2y2us z(Q_epA>N*DazSBOChX)i#BS@jqmH89jSn6w)NisbKr|Sq zKEn4(PPFFq+6X7=xN~Zc?G8aT6uQR%({_S$js!VG_XbOmqKqND4Fk5fN@vAH~};niGA@JCnu z_3!z6)Bg!1LY7GRS_~FYf>0=a_Ip(SJ6PZ9v8lngzehK1Hvpj>g2c50y4%O*Pc8rf zG?6>&lG|c65Myy2c?(cORdEEv&>#cn;eT#O%WN=JZb^8w&z{L#EE2i{f zDp{`5TdSUEVm^~jFW!M?r|4cH-Ch%`pz7&#OcxOCjAa*vnfw%;_#lA&E?^rm-YuBz zz}NV6*2?Z-9QGZa+xp1k6^WdIMp-WPfn^oTe1GX#JK=j{#R!Rk0KAnMCh%iMGe=Yn0HhHTH69K_~U#mDD#e)F9tuV$mC%km(m@{ITImFHXqaPvM==o#v(HMUn( zlDyj;vP8@8#iz>A9Cq3|tE|O|FHe3E9%f#j#nC)54D~8L&_)2_2crD#DhJV>=IXK6 z)!QoLuVEx%Q9j$6^RN2;!0yv_)I;6m{vC;FZPQ{;f=(R-K=Npw7p>&(l6@8&$7z*5 zR;SrLxVfhH2`L=(B5!Nh!33A^!&=@JAJ1%q47Tw2e$55I3@>}<<=%>|$k`@q=*`dg z5Eawb*CsrD#%s1sHClT+^KxBY6s0CE%3fqU`_+JBR{+jpQw19}dWl+_>)h=Mx1+wf z#s?$l$7^Q2=WdHwM@5CSMwVqJr268sa<%Z*ZeS>YE?!jU<&)_Sdy~)WR5$0bGg2&{ zPo-avmv=s%HQgC*}wMDx!J97h3X@&33Vm3fD-(!hEBf-4BoO w+B0=nHhnM})sQ$Zu`>Mc0{H$)Tub=nj2RgJWsLA8)wL?W{qj=y0e4SSEbv>W@Bjb+ literal 0 HcmV?d00001 diff --git a/modules/git/tests/repos/repo1_bare/objects/36/f97d9a96457e2bab511db30fe2db03893ebc64 b/modules/git/tests/repos/repo1_bare/objects/36/f97d9a96457e2bab511db30fe2db03893ebc64 new file mode 100644 index 0000000000000000000000000000000000000000..c96b843902407e9468f8851fa80fc054dbb045c4 GIT binary patch literal 770 zcmV+d1O5DX0d14HvZGcMMIG}M%Bg1%pbJ#FU1lD{jD&y;E{GXP7Z9MY_iMY#o%lq1 ztzg$l_MBA*o}+$ypT9$szl$Wf0t-&#^UhDQ2zA`e{UXt9*zKT^ob!U@_>^^lEkoI z4PQe!O<7f9Uq4S&G{ZS~ns*2d%@HowK>5dGSmPTXQ4mB)5DK+Xl$)1RZ4A_q8eY2@ z_||YHb<@>)#a4yE*<{=oq^kSRW4+?ewoecRe60M{4gf3atUrm_ZKuJdBTO#p#7zpv zw=+DEDI2tQrM1)IBA$!L&xi%_R9=lc^ldi_EE^tb^$%_+6%=J3u1y;D%-UcYXo}G4rI7Wyu9?uR0&U~S;kG`2#y8}*6I48-3+3Do-kT?PxcE%O8bp__jX$sDQ#?B> zulhl6G1c84gT9Pj(ywxGhZdUNRKFe1Q_7zwv^N3u1`i286*iq(DN}sl>qf0kmy%_3 zI^viZ+2$w#wG4J4YM)gwqq++wU*E22AL3T147JBQtaBdjQiDbgiEY_PxGH&SEx$}` z3nSeUm+>-8{!IY5Jp)=-!z7MU8^(T0&%@iOI(y7c`~C!<{Bq5pl=#|7;-*S^W4N5) z^UxF~$_%+;Z%6oG;xSouf^x_DmVYKe;g(m@k&_*+QBKd9u zA0m_9>7kwHsNy}bRB=vF#@Z^&eN~aN14N^1fBk{|K$8~!I}*i_|8F9H1G4HoaeVlG AYybcN literal 0 HcmV?d00001 diff --git a/modules/git/tests/repos/repo1_bare/objects/4b/825dc642cb6eb9a060e54bf8d69288fbee4904 b/modules/git/tests/repos/repo1_bare/objects/4b/825dc642cb6eb9a060e54bf8d69288fbee4904 new file mode 100644 index 0000000000000000000000000000000000000000..adf64119a33d7621aeeaa505d30adb58afaa5559 GIT binary patch literal 15 WcmbxJ#nW*WL@ocIdmEWK9r>)TVA>E9|chi@fKQQ|1m;JrD|G8lWLE?Ch7x~Q>^Z7+l zzY@MNdOq_CQQtqMLetuB0yEe|mbq8r*sw#@ADN1QH5N5#z+>U|~0sL6( zL+E0M+1b9?LC;X|p6E@XXjS_kr#cw0prS=Jdn5OJ5~|wvVH?V&tT|{3W~{!4F>Xgi z;rBs~{Yg-)Ad~&QTbxVJPbkn1wHPaog-^tb5~GzY!kFXm;Kf?4YDI-oIe3s`^YkcL z`XfD@YJH+F*knvkJ}nfzkoM>eTL=SsnV>$Xgp8|9va2-RV(-l~$B|?wP;gLt7-scm zS5B5{EK1#c%)K4ec`Vq2yWp;ttu>){clYkB;Pza?*L-JiP3XhzDjGyF;GEr345RdM zJ5SwvTk7D7DQBHcHqB18W7Lt;K0-+wa;T{miC{y_DIF@6cW&Y&SG{ygdFGanmtG#V zl+ChiE~!VjS`2+&qU5U;hq!B!+J>cisE&;Ww%)RR*A7`iG%A)~!1Z8Cce-nBO|S?w zr30dN>XXBHKV0nX%LWJYDrEd1Kg)qQP73>>m1-j9ByHVE^;{sSs+O~r!WO7k)ULd0kV*bOeT~H&k&Dn zMBPl5C@Y0L19<^>u|$O+aHRksgo4O&0t-HbFSvpuvSqINk(}!GiN;smLGllb{`$B5 zz3%@cSpZ~S6c|<@DTZYjqWJmWjQ;oH?Qxsx8~N?;+Ue&0M0%!2hI{j*MscA3(>0k3Gy?^E;pJ^ z(;jH6;gw(KV>;=9N;%49M`e@N^gD+)xSP7^$;_(aXur<-%dx92T_c7dD`Oen3?5=& zB?X=H4Xc7I;hXn*A05#Lj_i&~Nb7vWB*|j|sl1Cj_X@Lv6LV-?2=b&!QY6&pkh9yx zP1NQXCXXzVPY*Am@$?XjHd9t)4Et$8w6}z(i!h(yWMWFDj(tXuP!ftOUaEH3f~Or# zqjvQ!0UA;3AMZ;YH(+~p5Q^63T5{fO5FhxKf~nVpJT`L+pC@;kfg9sMko#>E#~r}m&H zC9~{mtIj10?v&|haRypt;cc89-lkc}>^jsV?Rm5x1PSWr&`ehDi6Yy@O8Al=yiXNt zet(HzvWE2zBCFno>&4qBrqUS9riqL4J45ML zt1~RHNXU$$DAP<4iB@0jWrGp4b23kFh(8yMdol%6gbz>bRqsjLcsYexWom2fz7u@N z;xHcI(jBU6V4jxB&BLoed9R_>9(olKq;9IufUzjktsXKSCBHsr+qj?2BkP=FAM|^H tX@=8L;`x@o11ve0?*#chs$T~2%NhNo{g*MsN2(6NZ@;`GegOD(SMvVNmv;aF literal 0 HcmV?d00001 diff --git a/modules/git/tests/repos/repo1_bare/objects/cf/8b0b492a950b358a7ce7f9d01b18aef48a6b2d b/modules/git/tests/repos/repo1_bare/objects/cf/8b0b492a950b358a7ce7f9d01b18aef48a6b2d new file mode 100644 index 0000000000000000000000000000000000000000..1152b25bb8318c8fcb2b11de4e18b0185eeef25f GIT binary patch literal 827 zcmV-B1H}Az0hN={!^kHPlWms3wlCQ+oE z+1}FGT9cLcZCfumNmAr5%Y+aPGLB-390DrOAjJzzk3Y7OV*+b`<h9v z2Wyta=yLImAN_ z##D*&bKCI+=#th#k4y$ay2Gw=-kC+PcK|`jjN8nf!*%IpqL{nz)+J<)0Kt;-?sL-(FZT_ z_%1)Ja-7%pVDO_(pohavsk_^@gi26+d ze6;S@2WbyS_wCYAJn{`gHf|{Gl+X&cVNE{_>?{D{&>rk%J+H-C*Qr>8L7lv+#3wU9 zCJm0wIdl^Ex4mm`76`^`{>e-?7~HLfhL87JzrHOl@`cpL2B!f?4O<<1H> z!c*yCAY>(?x~CS*PS%Ui^+K^Sv$K<6c#;jaz?R!yMDYw&R`1g=NF?EMQ_H?6NpSVy zyM4QAGd+4Y)b{o;9&AXC&+RZ^mFBsXA1A`|=XQ{IA%T<+tqh+ke}Q{Q%RZ FW`6CxpoRbd literal 0 HcmV?d00001 diff --git a/modules/git/tests/repos/repo1_bare/refs/heads/master b/modules/git/tests/repos/repo1_bare/refs/heads/master index c5e92eb22..9b0de2281 100644 --- a/modules/git/tests/repos/repo1_bare/refs/heads/master +++ b/modules/git/tests/repos/repo1_bare/refs/heads/master @@ -1 +1 @@ -feaf4ba6bc635fec442f46ddd4512416ec43c2c2 +ce064814f4a0d337b333e646ece456cd39fab612 diff --git a/modules/git/tests/repos/repo1_bare/refs/tags/signed-tag b/modules/git/tests/repos/repo1_bare/refs/tags/signed-tag new file mode 100644 index 000000000..3998a6850 --- /dev/null +++ b/modules/git/tests/repos/repo1_bare/refs/tags/signed-tag @@ -0,0 +1 @@ +36f97d9a96457e2bab511db30fe2db03893ebc64