Skip to content

Commit 9497733

Browse files
bfabiobfabio
committed
fix: DisableNetwork and DisableExternalChecks logic
Fix cases where DisableNetwork and DisableExternalChecks still went out to the the network.
1 parent f6dd036 commit 9497733

File tree

3 files changed

+54
-46
lines changed

3 files changed

+54
-46
lines changed

‎fields.go‎

Lines changed: 39 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@ func validateFieldsV0(publiccode PublicCode, parser Parser, network bool) error
2020

2121
var vr ValidationResults
2222

23-
if publiccodev0.URL != nil && network {
24-
if reachable, err := parser.isReachable(*(*url.URL)(publiccodev0.URL), network); !reachable {
23+
checksNetwork := network && !parser.disableExternalChecks
24+
25+
if checksNetwork && publiccodev0.URL != nil {
26+
if reachable, err := parser.isReachable(*(*url.URL)(publiccodev0.URL)); !reachable {
2527
vr = append(vr, newValidationErrorf("url", "'%s' not reachable: %s", publiccodev0.URL, err.Error()))
2628
}
2729

@@ -30,17 +32,17 @@ func validateFieldsV0(publiccode PublicCode, parser Parser, network bool) error
3032
}
3133
}
3234

33-
if publiccodev0.LandingURL != nil {
34-
if reachable, err := parser.isReachable(*(*url.URL)(publiccodev0.LandingURL), network); !reachable {
35+
if checksNetwork && publiccodev0.LandingURL != nil {
36+
if reachable, err := parser.isReachable(*(*url.URL)(publiccodev0.LandingURL)); !reachable {
3537
vr = append(vr, newValidationErrorf(
3638
"landingURL",
3739
"'%s' not reachable: %s", publiccodev0.LandingURL, err.Error(),
3840
))
3941
}
4042
}
4143

42-
if publiccodev0.Roadmap != nil {
43-
if reachable, err := parser.isReachable(*(*url.URL)(publiccodev0.Roadmap), network); !reachable {
44+
if checksNetwork && publiccodev0.Roadmap != nil {
45+
if reachable, err := parser.isReachable(*(*url.URL)(publiccodev0.Roadmap)); !reachable {
4446
vr = append(vr, newValidationErrorf(
4547
"roadmap",
4648
"'%s' not reachable: %s", publiccodev0.Roadmap, err.Error(),
@@ -52,9 +54,12 @@ func validateFieldsV0(publiccode PublicCode, parser Parser, network bool) error
5254
if _, err := isRelativePathOrURL(*publiccodev0.Logo, "logo"); err != nil {
5355
vr = append(vr, err)
5456
} else if !parser.disableExternalChecks {
55-
validLogo, err := parser.validLogo(toCodeHostingURL(*publiccodev0.Logo, parser.currentBaseURL), network)
56-
if !validLogo {
57-
vr = append(vr, newValidationError("logo", err.Error()))
57+
u := toAbsoluteURL(*publiccodev0.Logo, parser.currentBaseURL, network)
58+
if u != nil {
59+
validLogo, err := parser.validLogo(*u, network)
60+
if !validLogo {
61+
vr = append(vr, newValidationError("logo", err.Error()))
62+
}
5863
}
5964
}
6065
}
@@ -65,9 +70,12 @@ func validateFieldsV0(publiccode PublicCode, parser Parser, network bool) error
6570
if _, err := isRelativePathOrURL(*publiccodev0.MonochromeLogo, "monochromeLogo"); err != nil {
6671
vr = append(vr, err)
6772
} else if !parser.disableExternalChecks {
68-
validLogo, err := parser.validLogo(toCodeHostingURL(*publiccodev0.MonochromeLogo, parser.currentBaseURL), network)
69-
if !validLogo {
70-
vr = append(vr, newValidationError("monochromeLogo", err.Error()))
73+
u := toAbsoluteURL(*publiccodev0.MonochromeLogo, parser.currentBaseURL, network)
74+
if u != nil {
75+
validLogo, err := parser.validLogo(*u, network)
76+
if !validLogo {
77+
vr = append(vr, newValidationError("monochromeLogo", err.Error()))
78+
}
7179
}
7280
}
7381
}
@@ -109,11 +117,12 @@ func validateFieldsV0(publiccode PublicCode, parser Parser, network bool) error
109117
if _, err := isRelativePathOrURL(*publiccodev0.Legal.AuthorsFile, "legal.authorsFile"); err != nil {
110118
vr = append(vr, err)
111119
} else if !parser.disableExternalChecks {
112-
exists, err := parser.fileExists(toCodeHostingURL(*publiccodev0.Legal.AuthorsFile, parser.currentBaseURL), network)
113-
if !exists {
114-
u := toCodeHostingURL(*publiccodev0.Legal.AuthorsFile, parser.currentBaseURL)
115-
116-
vr = append(vr, newValidationErrorf("legal.authorsFile", "'%s' does not exist: %s", urlutil.DisplayURL(&u), err.Error()))
120+
u := toAbsoluteURL(*publiccodev0.Legal.AuthorsFile, parser.currentBaseURL, network)
121+
if u != nil {
122+
exists, err := parser.fileExists(*u, network)
123+
if !exists {
124+
vr = append(vr, newValidationErrorf("legal.authorsFile", "'%s' does not exist: %s", urlutil.DisplayURL(u), err.Error()))
125+
}
117126
}
118127
}
119128
}
@@ -142,17 +151,17 @@ func validateFieldsV0(publiccode PublicCode, parser Parser, network bool) error
142151
})
143152
}
144153

145-
if !parser.disableExternalChecks && network && desc.Documentation != nil {
146-
if reachable, err := parser.isReachable(*(*url.URL)(desc.Documentation), network); !reachable {
154+
if checksNetwork && desc.Documentation != nil {
155+
if reachable, err := parser.isReachable(*(*url.URL)(desc.Documentation)); !reachable {
147156
vr = append(vr, newValidationErrorf(
148157
fmt.Sprintf("description.%s.documentation", lang),
149158
"'%s' not reachable: %s", desc.Documentation, err.Error(),
150159
))
151160
}
152161
}
153162

154-
if !parser.disableExternalChecks && network && desc.APIDocumentation != nil {
155-
if reachable, err := parser.isReachable(*(*url.URL)(desc.APIDocumentation), network); !reachable {
163+
if checksNetwork && desc.APIDocumentation != nil {
164+
if reachable, err := parser.isReachable(*(*url.URL)(desc.APIDocumentation)); !reachable {
156165
vr = append(vr, newValidationErrorf(
157166
fmt.Sprintf("description.%s.apiDocumentation", lang),
158167
"'%s' not reachable: %s", desc.APIDocumentation, err.Error(),
@@ -165,12 +174,15 @@ func validateFieldsV0(publiccode PublicCode, parser Parser, network bool) error
165174
if _, err := isRelativePathOrURL(v, keyName); err != nil {
166175
vr = append(vr, err)
167176
} else if !parser.disableExternalChecks {
168-
isImage, err := parser.isImageFile(toCodeHostingURL(v, parser.currentBaseURL), network)
169-
if !isImage {
170-
vr = append(vr, newValidationErrorf(
171-
keyName,
172-
"'%s' is not an image: %s", v, err.Error(),
173-
))
177+
u := toAbsoluteURL(v, parser.currentBaseURL, network)
178+
if u != nil {
179+
isImage, err := parser.isImageFile(*u, network)
180+
if !isImage {
181+
vr = append(vr, newValidationErrorf(
182+
keyName,
183+
"'%s' is not an image: %s", v, err.Error(),
184+
))
185+
}
174186
}
175187
}
176188
}

‎parser_test.go‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -890,15 +890,6 @@ func TestUrlMissingWithoutPath(t *testing.T) {
890890
}
891891
}
892892

893-
func TestIsReachable(t *testing.T) {
894-
parser, _ := NewParser(ParserConfig{DisableNetwork: true})
895-
896-
u, _ := url.Parse("https://google.com/404")
897-
if reachable, _ := parser.isReachable(*u, false); !reachable {
898-
t.Errorf("isReachable() returned false with DisableNetwork enabled")
899-
}
900-
}
901-
902893
// Test that the exported YAML passes validation again, and that re-exporting it
903894
// matches the first export (lossless roundtrip).
904895
func TestExport(t *testing.T) {

‎validations.go‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,10 @@ func getHeaderFromDomain(domain Domain, url string) map[string]string {
6868

6969
// isReachable checks whether the URL resource is reachable.
7070
// An URL resource is reachable if it returns HTTP 200.
71-
func (p *Parser) isReachable(u url.URL, network bool) (bool, error) {
72-
// Don't check if the network checks are disabled or if we are running in WASM
73-
// because we'd most likely fail due to CORS errors.
74-
if !network || runtime.GOARCH == "wasm" {
71+
func (p *Parser) isReachable(u url.URL) (bool, error) {
72+
// Don't check if we are running in WASM because we'd most likely
73+
// fail due to CORS errors.
74+
if runtime.GOARCH == "wasm" {
7575
return true, nil
7676
}
7777

@@ -91,19 +91,24 @@ func (p *Parser) isReachable(u url.URL, network bool) (bool, error) {
9191
return true, nil
9292
}
9393

94-
// toURL turns the passed string into an URL, trying to resolve
94+
// toAbsoluteURL turns the passed string into an URL, trying to resolve
9595
// code hosting URLs to their raw URL.
9696
//
9797
// It supports relative paths and turns them into remote URLs or file:// URLs
9898
// depending on the value of baseURL
99-
func toCodeHostingURL(file string, baseURL *url.URL) url.URL {
99+
func toAbsoluteURL(file string, baseURL *url.URL, network bool) *url.URL {
100100
// Check if file is an absolute URL
101101
if uri, err := url.ParseRequestURI(file); err == nil {
102+
if !network {
103+
return nil
104+
}
105+
106+
// this uses the network to detect the git branch
102107
if raw := vcsurl.GetRawFile(uri); raw != nil {
103-
return *raw
108+
return raw
104109
}
105110

106-
return *uri
111+
return uri
107112
}
108113

109114
// We always pass the computed base URL here, with a fallback to the cwd,
@@ -112,7 +117,7 @@ func toCodeHostingURL(file string, baseURL *url.URL) url.URL {
112117
u := *baseURL
113118
u.Path = path.Join(u.Path, file)
114119

115-
return u
120+
return &u
116121
}
117122

118123
// fileExists returns true if the file resource exists.
@@ -134,7 +139,7 @@ func (p *Parser) fileExists(u url.URL, network bool) (bool, error) {
134139
}
135140

136141
if network {
137-
reachable, err := p.isReachable(u, network)
142+
reachable, err := p.isReachable(u)
138143

139144
return reachable, err
140145
}

0 commit comments

Comments
 (0)