Skip to content

Commit ba3e16c

Browse files
committed
fix unit test
1 parent c1c021f commit ba3e16c

File tree

6 files changed

+53
-31
lines changed

6 files changed

+53
-31
lines changed

integrations/api_repo_lfs_migrate_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"code.gitea.io/gitea/models"
1313
"code.gitea.io/gitea/models/db"
1414
"code.gitea.io/gitea/modules/lfs"
15+
"code.gitea.io/gitea/modules/migrations"
1516
"code.gitea.io/gitea/modules/setting"
1617
api "code.gitea.io/gitea/modules/structs"
1718

@@ -25,6 +26,7 @@ func TestAPIRepoLFSMigrateLocal(t *testing.T) {
2526
oldAllowLocalNetworks := setting.Migrations.AllowLocalNetworks
2627
setting.ImportLocalPaths = true
2728
setting.Migrations.AllowLocalNetworks = true
29+
_ = migrations.Init()
2830

2931
user := db.AssertExistsAndLoadBean(t, &models.User{ID: 1}).(*models.User)
3032
session := loginUser(t, user.Name)
@@ -47,4 +49,5 @@ func TestAPIRepoLFSMigrateLocal(t *testing.T) {
4749

4850
setting.ImportLocalPaths = oldImportLocalPaths
4951
setting.Migrations.AllowLocalNetworks = oldAllowLocalNetworks
52+
_ = migrations.Init()
5053
}

integrations/mirror_push_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"code.gitea.io/gitea/models"
1515
"code.gitea.io/gitea/models/db"
1616
"code.gitea.io/gitea/modules/git"
17+
"code.gitea.io/gitea/modules/migrations"
1718
"code.gitea.io/gitea/modules/repository"
1819
"code.gitea.io/gitea/modules/setting"
1920
mirror_service "code.gitea.io/gitea/services/mirror"
@@ -29,6 +30,7 @@ func testMirrorPush(t *testing.T, u *url.URL) {
2930
defer prepareTestEnv(t)()
3031

3132
setting.Migrations.AllowLocalNetworks = true
33+
_ = migrations.Init()
3234

3335
user := db.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User)
3436
srcRepo := db.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository)

modules/hostmatcher/hostmatcher.go

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
package hostmatcher
66

77
import (
8-
"fmt"
98
"net"
109
"path/filepath"
1110
"strings"
@@ -19,9 +18,9 @@ type HostMatchList struct {
1918
SettingKeyHint string
2019
SettingValue string
2120

22-
// host name or built-in network name
23-
names []string
24-
ipNets []*net.IPNet
21+
// patterns for host names or built-in networks
22+
patterns []string
23+
ipNets []*net.IPNet
2524
}
2625

2726
// MatchBuiltinAll all hosts are matched
@@ -48,56 +47,58 @@ func ParseHostMatchList(settingKeyHint string, hostList string) *HostMatchList {
4847
if err == nil {
4948
hl.ipNets = append(hl.ipNets, ipNet)
5049
} else {
51-
hl.names = append(hl.names, s)
50+
hl.patterns = append(hl.patterns, s)
5251
}
5352
}
5453
return hl
5554
}
5655

5756
// ParseSimpleMatchList parse a simple matchlist (no built-in networks, no CIDR support)
58-
func ParseSimpleMatchList(settingKeyHint string, matchList string, includeLocalNetwork bool) *HostMatchList {
57+
func ParseSimpleMatchList(settingKeyHint string, matchList string) *HostMatchList {
5958
hl := &HostMatchList{
6059
SettingKeyHint: settingKeyHint,
61-
SettingValue: matchList + fmt.Sprintf("(local-network:%v)", includeLocalNetwork),
60+
SettingValue: matchList,
6261
}
6362
for _, s := range strings.Split(matchList, ",") {
6463
s = strings.ToLower(strings.TrimSpace(s))
6564
if s == "" {
6665
continue
6766
}
6867
if s == MatchBuiltinLoopback || s == MatchBuiltinPrivate || s == MatchBuiltinExternal {
69-
// for built-in names, we convert it from "private" => "[p]rivate" for internal usage and keep the same result as `matchlist`
70-
hl.names = append(hl.names, "["+s[:1]+"]"+s[1:])
68+
// for built-in patterns, we convert it from "private" => "[p]rivate" for internal usage and keep the same result as `matchlist`
69+
hl.patterns = append(hl.patterns, "["+s[:1]+"]"+s[1:])
7170
} else {
7271
// we keep the same result as `matchlist`, so no CIDR support here
73-
hl.names = append(hl.names, s)
72+
hl.patterns = append(hl.patterns, s)
7473
}
7574
}
76-
if includeLocalNetwork {
77-
hl.names = append(hl.names, MatchBuiltinPrivate)
78-
}
7975
return hl
8076
}
8177

78+
// AppendPattern appends more patterns to match
79+
func (hl *HostMatchList) AppendPattern(pattern string) {
80+
hl.patterns = append(hl.patterns, pattern)
81+
}
82+
8283
// IsEmpty checks if the check list is empty
8384
func (hl *HostMatchList) IsEmpty() bool {
84-
return hl == nil || (len(hl.names) == 0 && len(hl.ipNets) == 0)
85+
return hl == nil || (len(hl.patterns) == 0 && len(hl.ipNets) == 0)
8586
}
8687

87-
func (hl *HostMatchList) checkNames(host string) bool {
88+
func (hl *HostMatchList) checkPattern(host string) bool {
8889
host = strings.ToLower(strings.TrimSpace(host))
89-
for _, name := range hl.names {
90-
switch name {
90+
for _, pattern := range hl.patterns {
91+
switch pattern {
9192
case "":
9293
case MatchBuiltinExternal:
9394
case MatchBuiltinPrivate:
9495
case MatchBuiltinLoopback:
95-
// ignore empty string or built-in network names
96+
// ignore empty string or built-in network patterns
9697
continue
9798
case MatchBuiltinAll:
9899
return true
99100
default:
100-
if matched, _ := filepath.Match(name, host); matched {
101+
if matched, _ := filepath.Match(pattern, host); matched {
101102
return true
102103
}
103104
}
@@ -106,8 +107,8 @@ func (hl *HostMatchList) checkNames(host string) bool {
106107
}
107108

108109
func (hl *HostMatchList) checkIP(ip net.IP) bool {
109-
for _, name := range hl.names {
110-
switch name {
110+
for _, pattern := range hl.patterns {
111+
switch pattern {
111112
case "":
112113
continue
113114
case MatchBuiltinAll:
@@ -139,7 +140,7 @@ func (hl *HostMatchList) MatchHostName(host string) bool {
139140
if hl == nil {
140141
return false
141142
}
142-
if hl.checkNames(host) {
143+
if hl.checkPattern(host) {
143144
return true
144145
}
145146
if ip := net.ParseIP(host); ip != nil {
@@ -154,7 +155,7 @@ func (hl *HostMatchList) MatchIPAddr(ip net.IP) bool {
154155
return false
155156
}
156157
host := ip.String() // nil-safe, we will get "<nil>" if ip is nil
157-
return hl.checkNames(host) || hl.checkIP(ip)
158+
return hl.checkPattern(host) || hl.checkIP(ip)
158159
}
159160

160161
// MatchHostOrIP checks if the host or IP matches an allow/deny(block) list

modules/hostmatcher/hostmatcher_test.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,22 +132,29 @@ func TestHostOrIPMatchesList(t *testing.T) {
132132
}
133133
test(cases)
134134

135-
hl = ParseSimpleMatchList("", "loopback, *.domain.com", true)
135+
hl = ParseSimpleMatchList("", "loopback, *.domain.com")
136136
cases = []tc{
137137
{"loopback", nil, true},
138138
{"", net.ParseIP("127.0.0.1"), false},
139139
{"sub.domain.com", nil, true},
140140
{"other.com", nil, false},
141-
{"", net.ParseIP("192.168.1.1"), true},
142141
{"", net.ParseIP("1.1.1.1"), false},
143142
}
144143
test(cases)
145144

146-
hl = ParseSimpleMatchList("", "external", false)
145+
hl = ParseSimpleMatchList("", "external")
147146
cases = []tc{
148147
{"", net.ParseIP("192.168.1.1"), false},
149148
{"", net.ParseIP("1.1.1.1"), false},
150149
{"external", nil, true},
151150
}
152151
test(cases)
152+
153+
hl = ParseSimpleMatchList("", "")
154+
cases = []tc{
155+
{"", net.ParseIP("192.168.1.1"), false},
156+
{"", net.ParseIP("1.1.1.1"), false},
157+
{"external", nil, false},
158+
}
159+
test(cases)
153160
}

modules/migrations/migrate.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,9 @@ func IsMigrateURLAllowed(remoteURL string, doer *models.User) error {
7575
hostName, _, err := net.SplitHostPort(u.Host)
7676
if err != nil {
7777
// u.Host can be "host" or "host:port"
78+
err = nil
7879
hostName = u.Host
7980
}
80-
if err != nil {
81-
return &models.ErrInvalidCloneAddr{Host: u.Host, IsURLError: true}
82-
}
8381
addrList, err := net.LookupIP(hostName)
8482
if err != nil {
8583
return &models.ErrInvalidCloneAddr{Host: u.Host, NotResolvedIP: true}
@@ -469,7 +467,16 @@ func migrateRepository(downloader base.Downloader, uploader base.Uploader, opts
469467
// Init migrations service
470468
func Init() error {
471469
// TODO: maybe we can deprecate these legacy ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS/BLOCKED_DOMAINS, use ALLOWED_HOST_LIST/BLOCKED_HOST_LIST instead
472-
allowList = hostmatcher.ParseSimpleMatchList("migrations.ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS", setting.Migrations.AllowedDomains, setting.Migrations.AllowLocalNetworks)
473-
blockList = hostmatcher.ParseSimpleMatchList("migrations.BLOCKED_DOMAINS", setting.Migrations.BlockedDomains, false)
470+
471+
blockList = hostmatcher.ParseSimpleMatchList("migrations.BLOCKED_DOMAINS", setting.Migrations.BlockedDomains)
472+
473+
allowList = hostmatcher.ParseSimpleMatchList("migrations.ALLOWED_DOMAINS/ALLOW_LOCALNETWORKS", setting.Migrations.AllowedDomains)
474+
if allowList.IsEmpty() {
475+
// the default policy is that migration module can access external hosts
476+
allowList.AppendPattern(hostmatcher.MatchBuiltinExternal)
477+
}
478+
if setting.Migrations.AllowLocalNetworks {
479+
allowList.AppendPattern(hostmatcher.MatchBuiltinPrivate)
480+
}
474481
return nil
475482
}

modules/migrations/migrate_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ func TestMigrateWhiteBlocklist(t *testing.T) {
2222
nonAdminUser := db.AssertExistsAndLoadBean(t, &models.User{Name: "user2"}).(*models.User)
2323

2424
setting.Migrations.AllowedDomains = "github.com"
25+
setting.Migrations.AllowLocalNetworks = false
2526
assert.NoError(t, Init())
2627

2728
err := IsMigrateURLAllowed("https://gitlab.com/gitlab/gitlab.git", nonAdminUser)
@@ -47,6 +48,7 @@ func TestMigrateWhiteBlocklist(t *testing.T) {
4748
assert.Error(t, err)
4849

4950
setting.Migrations.AllowLocalNetworks = true
51+
assert.NoError(t, Init())
5052
err = IsMigrateURLAllowed("https://10.0.0.1/go-gitea/gitea.git", nonAdminUser)
5153
assert.NoError(t, err)
5254

0 commit comments

Comments
 (0)