oppi.li
automatically adds a newline to patches that are missing one.
Signed-off-by: oppiliappan me@oppi.li
+18
-22
appview/pulls/pulls.go
+18
-22
appview/pulls/pulls.go
···
23
"tangled.org/core/appview/pages"
24
"tangled.org/core/appview/pages/markup"
25
"tangled.org/core/appview/reporesolver"
26
"tangled.org/core/appview/xrpcclient"
27
"tangled.org/core/idresolver"
28
"tangled.org/core/patchutil"
···
47
notifier notify.Notifier
48
enforcer *rbac.Enforcer
49
logger *slog.Logger
50
}
51
52
func New(
···
58
config *config.Config,
59
notifier notify.Notifier,
60
enforcer *rbac.Enforcer,
61
logger *slog.Logger,
62
) *Pulls {
63
return &Pulls{
···
70
notifier: notifier,
71
enforcer: enforcer,
72
logger: logger,
73
}
74
}
75
···
965
patch := comparison.FormatPatchRaw
966
combined := comparison.CombinedPatchRaw
967
968
-
if !patchutil.IsPatchValid(patch) {
969
s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.")
970
return
971
}
···
982
}
983
984
func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, title, body, targetBranch, patch string, isStacked bool) {
985
-
if !patchutil.IsPatchValid(patch) {
986
s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.")
987
return
988
}
···
1073
patch := comparison.FormatPatchRaw
1074
combined := comparison.CombinedPatchRaw
1075
1076
-
if !patchutil.IsPatchValid(patch) {
1077
s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.")
1078
return
1079
}
···
1337
return
1338
}
1339
1340
-
if patch == "" || !patchutil.IsPatchValid(patch) {
1341
s.pages.Notice(w, "patch-error", "Invalid patch format. Please provide a valid git diff or format-patch.")
1342
return
1343
}
···
1754
s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev)
1755
}
1756
1757
-
// validate a resubmission against a pull request
1758
-
func validateResubmittedPatch(pull *models.Pull, patch string) error {
1759
-
if patch == "" {
1760
-
return fmt.Errorf("Patch is empty.")
1761
-
}
1762
-
1763
-
if patch == pull.LatestPatch() {
1764
-
return fmt.Errorf("Patch is identical to previous submission.")
1765
-
}
1766
-
1767
-
if !patchutil.IsPatchValid(patch) {
1768
-
return fmt.Errorf("Invalid patch format. Please provide a valid diff.")
1769
-
}
1770
-
1771
-
return nil
1772
-
}
1773
-
1774
func (s *Pulls) resubmitPullHelper(
1775
w http.ResponseWriter,
1776
r *http.Request,
···
1787
return
1788
}
1789
1790
-
if err := validateResubmittedPatch(pull, patch); err != nil {
1791
s.pages.Notice(w, "resubmit-error", err.Error())
1792
return
1793
}
1794
1795
// validate sourceRev if branch/fork based
1796
if pull.IsBranchBased() || pull.IsForkBased() {
1797
if sourceRev == pull.LatestSha() {
···
23
"tangled.org/core/appview/pages"
24
"tangled.org/core/appview/pages/markup"
25
"tangled.org/core/appview/reporesolver"
26
+
"tangled.org/core/appview/validator"
27
"tangled.org/core/appview/xrpcclient"
28
"tangled.org/core/idresolver"
29
"tangled.org/core/patchutil"
···
48
notifier notify.Notifier
49
enforcer *rbac.Enforcer
50
logger *slog.Logger
51
+
validator *validator.Validator
52
}
53
54
func New(
···
60
config *config.Config,
61
notifier notify.Notifier,
62
enforcer *rbac.Enforcer,
63
+
validator *validator.Validator,
64
logger *slog.Logger,
65
) *Pulls {
66
return &Pulls{
···
73
notifier: notifier,
74
enforcer: enforcer,
75
logger: logger,
76
+
validator: validator,
77
}
78
}
79
···
969
patch := comparison.FormatPatchRaw
970
combined := comparison.CombinedPatchRaw
971
972
+
if err := s.validator.ValidatePatch(&patch); err != nil {
973
+
s.logger.Error("failed to validate patch", "err", err)
974
s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.")
975
return
976
}
···
987
}
988
989
func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, f *reporesolver.ResolvedRepo, user *oauth.User, title, body, targetBranch, patch string, isStacked bool) {
990
+
if err := s.validator.ValidatePatch(&patch); err != nil {
991
+
s.logger.Error("patch validation failed", "err", err)
992
s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.")
993
return
994
}
···
1079
patch := comparison.FormatPatchRaw
1080
combined := comparison.CombinedPatchRaw
1081
1082
+
if err := s.validator.ValidatePatch(&patch); err != nil {
1083
+
s.logger.Error("failed to validate patch", "err", err)
1084
s.pages.Notice(w, "pull", "Invalid patch format. Please provide a valid diff.")
1085
return
1086
}
···
1344
return
1345
}
1346
1347
+
if err := s.validator.ValidatePatch(&patch); err != nil {
1348
+
s.logger.Error("faield to validate patch", "err", err)
1349
s.pages.Notice(w, "patch-error", "Invalid patch format. Please provide a valid git diff or format-patch.")
1350
return
1351
}
···
1762
s.resubmitPullHelper(w, r, f, user, pull, patch, combined, sourceRev)
1763
}
1764
1765
func (s *Pulls) resubmitPullHelper(
1766
w http.ResponseWriter,
1767
r *http.Request,
···
1778
return
1779
}
1780
1781
+
if err := s.validator.ValidatePatch(&patch); err != nil {
1782
s.pages.Notice(w, "resubmit-error", err.Error())
1783
return
1784
}
1785
1786
+
if patch == pull.LatestPatch() {
1787
+
s.pages.Notice(w, "resubmit-error", "Patch is identical to previous submission.")
1788
+
return
1789
+
}
1790
+
1791
// validate sourceRev if branch/fork based
1792
if pull.IsBranchBased() || pull.IsForkBased() {
1793
if sourceRev == pull.LatestSha() {
+1
appview/state/router.go
+1
appview/state/router.go
+25
appview/validator/patch.go
+25
appview/validator/patch.go
···
···
1
+
package validator
2
+
3
+
import (
4
+
"fmt"
5
+
"strings"
6
+
7
+
"tangled.org/core/patchutil"
8
+
)
9
+
10
+
func (v *Validator) ValidatePatch(patch *string) error {
11
+
if patch == nil || *patch == "" {
12
+
return fmt.Errorf("patch is empty")
13
+
}
14
+
15
+
// add newline if not present to diff style patches
16
+
if !patchutil.IsFormatPatch(*patch) && !strings.HasSuffix(*patch, "\n") {
17
+
*patch = *patch + "\n"
18
+
}
19
+
20
+
if err := patchutil.IsPatchValid(*patch); err != nil {
21
+
return err
22
+
}
23
+
24
+
return nil
25
+
}
+2
knotserver/xrpc/merge_check.go
+2
knotserver/xrpc/merge_check.go
+18
-7
patchutil/patchutil.go
+18
-7
patchutil/patchutil.go
···
1
package patchutil
2
3
import (
4
"fmt"
5
"log"
6
"os"
···
42
// IsPatchValid checks if the given patch string is valid.
43
// It performs very basic sniffing for either git-diff or git-format-patch
44
// header lines. For format patches, it attempts to extract and validate each one.
45
-
func IsPatchValid(patch string) bool {
46
if len(patch) == 0 {
47
-
return false
48
}
49
50
lines := strings.Split(patch, "\n")
51
if len(lines) < 2 {
52
-
return false
53
}
54
55
firstLine := strings.TrimSpace(lines[0])
···
60
strings.HasPrefix(firstLine, "Index: ") ||
61
strings.HasPrefix(firstLine, "+++ ") ||
62
strings.HasPrefix(firstLine, "@@ ") {
63
-
return true
64
}
65
66
// check if it's format-patch
···
70
// it's safe to say it's broken.
71
patches, err := ExtractPatches(patch)
72
if err != nil {
73
-
return false
74
}
75
-
return len(patches) > 0
76
}
77
78
-
return false
79
}
80
81
func IsFormatPatch(patch string) bool {
···
1
package patchutil
2
3
import (
4
+
"errors"
5
"fmt"
6
"log"
7
"os"
···
43
// IsPatchValid checks if the given patch string is valid.
44
// It performs very basic sniffing for either git-diff or git-format-patch
45
// header lines. For format patches, it attempts to extract and validate each one.
46
+
var (
47
+
EmptyPatchError error = errors.New("patch is empty")
48
+
GenericPatchError error = errors.New("patch is invalid")
49
+
FormatPatchError error = errors.New("patch is not a valid format-patch")
50
+
)
51
+
52
+
func IsPatchValid(patch string) error {
53
if len(patch) == 0 {
54
+
return EmptyPatchError
55
}
56
57
lines := strings.Split(patch, "\n")
58
if len(lines) < 2 {
59
+
return EmptyPatchError
60
}
61
62
firstLine := strings.TrimSpace(lines[0])
···
67
strings.HasPrefix(firstLine, "Index: ") ||
68
strings.HasPrefix(firstLine, "+++ ") ||
69
strings.HasPrefix(firstLine, "@@ ") {
70
+
return nil
71
}
72
73
// check if it's format-patch
···
77
// it's safe to say it's broken.
78
patches, err := ExtractPatches(patch)
79
if err != nil {
80
+
return fmt.Errorf("%w: %w", FormatPatchError, err)
81
}
82
+
if len(patches) == 0 {
83
+
return EmptyPatchError
84
+
}
85
+
86
+
return nil
87
}
88
89
+
return GenericPatchError
90
}
91
92
func IsFormatPatch(patch string) bool {
+13
-12
patchutil/patchutil_test.go
+13
-12
patchutil/patchutil_test.go
···
1
package patchutil
2
3
import (
4
"reflect"
5
"testing"
6
)
···
9
tests := []struct {
10
name string
11
patch string
12
-
expected bool
13
}{
14
{
15
name: `empty patch`,
16
patch: ``,
17
-
expected: false,
18
},
19
{
20
name: `single line patch`,
21
patch: `single line`,
22
-
expected: false,
23
},
24
{
25
name: `valid diff patch`,
···
31
-old line
32
+new line
33
context`,
34
-
expected: true,
35
},
36
{
37
name: `valid patch starting with ---`,
···
41
-old line
42
+new line
43
context`,
44
-
expected: true,
45
},
46
{
47
name: `valid patch starting with Index`,
···
53
-old line
54
+new line
55
context`,
56
-
expected: true,
57
},
58
{
59
name: `valid patch starting with +++`,
···
63
-old line
64
+new line
65
context`,
66
-
expected: true,
67
},
68
{
69
name: `valid patch starting with @@`,
···
72
+new line
73
context
74
`,
75
-
expected: true,
76
},
77
{
78
name: `valid format patch`,
···
90
+new content
91
--
92
2.48.1`,
93
-
expected: true,
94
},
95
{
96
name: `invalid format patch`,
97
patch: `From 1234567890123456789012345678901234567890 Mon Sep 17 00:00:00 2001
98
From: Author <author@example.com>
99
This is not a valid patch format`,
100
-
expected: false,
101
},
102
{
103
name: `not a patch at all`,
···
105
just some
106
random text
107
that isn't a patch`,
108
-
expected: false,
109
},
110
}
111
112
for _, tt := range tests {
113
t.Run(tt.name, func(t *testing.T) {
114
result := IsPatchValid(tt.patch)
115
-
if result != tt.expected {
116
t.Errorf("IsPatchValid() = %v, want %v", result, tt.expected)
117
}
118
})
···
1
package patchutil
2
3
import (
4
+
"errors"
5
"reflect"
6
"testing"
7
)
···
10
tests := []struct {
11
name string
12
patch string
13
+
expected error
14
}{
15
{
16
name: `empty patch`,
17
patch: ``,
18
+
expected: EmptyPatchError,
19
},
20
{
21
name: `single line patch`,
22
patch: `single line`,
23
+
expected: EmptyPatchError,
24
},
25
{
26
name: `valid diff patch`,
···
32
-old line
33
+new line
34
context`,
35
+
expected: nil,
36
},
37
{
38
name: `valid patch starting with ---`,
···
42
-old line
43
+new line
44
context`,
45
+
expected: nil,
46
},
47
{
48
name: `valid patch starting with Index`,
···
54
-old line
55
+new line
56
context`,
57
+
expected: nil,
58
},
59
{
60
name: `valid patch starting with +++`,
···
64
-old line
65
+new line
66
context`,
67
+
expected: nil,
68
},
69
{
70
name: `valid patch starting with @@`,
···
73
+new line
74
context
75
`,
76
+
expected: nil,
77
},
78
{
79
name: `valid format patch`,
···
91
+new content
92
--
93
2.48.1`,
94
+
expected: nil,
95
},
96
{
97
name: `invalid format patch`,
98
patch: `From 1234567890123456789012345678901234567890 Mon Sep 17 00:00:00 2001
99
From: Author <author@example.com>
100
This is not a valid patch format`,
101
+
expected: FormatPatchError,
102
},
103
{
104
name: `not a patch at all`,
···
106
just some
107
random text
108
that isn't a patch`,
109
+
expected: GenericPatchError,
110
},
111
}
112
113
for _, tt := range tests {
114
t.Run(tt.name, func(t *testing.T) {
115
result := IsPatchValid(tt.patch)
116
+
if !errors.Is(result, tt.expected) {
117
t.Errorf("IsPatchValid() = %v, want %v", result, tt.expected)
118
}
119
})