Centralize link URL validation

This commit is contained in:
2026-06-02 18:38:16 +00:00
parent c4cfd3fc74
commit 42fb4aa82a
10 changed files with 166 additions and 46 deletions

27
docs/internal/link.md Normal file
View File

@@ -0,0 +1,27 @@
# Link URL Policy
## Purpose
`internal/link` defines shared validation for configured and persisted HTTP link URLs.
## Inputs and outputs
Input is a URL string. Output is either nil for an accepted URL or a concise validation error that callers wrap with field context.
## Validation behavior
Accepted URLs must parse successfully, use `http` or `https`, include a host, and omit query strings and fragments.
## Boundaries
This package validates URL shape only. It does not construct destination output URLs, choose primary URLs, infer public URLs from backend configuration, or read configuration files.
## Tests
Before changing link URL policy, inspect tests under `internal/link` and callers in `internal/config`, `internal/state`, and `internal/publish`.
## Invariants
- Configured `links.base_url`, persisted `links.primary_url`, persisted output `url`, and publish link planning use the same URL policy.
- Callers own field-specific error context.
- URL path construction remains in `internal/publish`.

View File

@@ -202,6 +202,7 @@ Use this current layout unless the project has a documented reason to differ:
- `internal/config`: configuration structs, defaults, loading, precedence, and validation. - `internal/config`: configuration structs, defaults, loading, precedence, and validation.
- `internal/bundle`: storage-backed source bundle discovery and validation over the public manifest contract. - `internal/bundle`: storage-backed source bundle discovery and validation over the public manifest contract.
- `internal/state`: `.distributor.json` parsing, validation, comparison, and output metadata. - `internal/state`: `.distributor.json` parsing, validation, comparison, and output metadata.
- `internal/link`: shared HTTP URL validation for configured and persisted link metadata.
- `internal/storage`: backend interfaces, shared path/resource types, backend registry, and storage errors. - `internal/storage`: backend interfaces, shared path/resource types, backend registry, and storage errors.
- `internal/adapters/local`: local filesystem backend. - `internal/adapters/local`: local filesystem backend.
- `internal/adapters/ssh`: SSH/SFTP backend. - `internal/adapters/ssh`: SSH/SFTP backend.

View File

@@ -2,9 +2,10 @@ package config
import ( import (
"fmt" "fmt"
"net/url"
"regexp" "regexp"
"strings" "strings"
"gitea.maximumdirect.net/eric/distributor/internal/link"
) )
var idPattern = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]*$`) var idPattern = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]*$`)
@@ -187,7 +188,7 @@ func validateLinks(errs ValidationErrors, context string, links *Links) Validati
} }
if links.BaseURL == "" { if links.BaseURL == "" {
errs = append(errs, context+".base_url is required") errs = append(errs, context+".base_url is required")
} else if err := validateLinkBaseURL(links.BaseURL); err != nil { } else if err := link.ValidateHTTPURL(links.BaseURL); err != nil {
errs = append(errs, context+".base_url "+err.Error()) errs = append(errs, context+".base_url "+err.Error())
} }
switch links.Primary { switch links.Primary {
@@ -198,26 +199,6 @@ func validateLinks(errs ValidationErrors, context string, links *Links) Validati
return errs return errs
} }
func validateLinkBaseURL(value string) error {
parsed, err := url.Parse(value)
if err != nil {
return fmt.Errorf("must be a valid URL")
}
if parsed.Scheme != "http" && parsed.Scheme != "https" {
return fmt.Errorf("must use http or https")
}
if parsed.Host == "" {
return fmt.Errorf("must include a host")
}
if parsed.RawQuery != "" {
return fmt.Errorf("must not include a query string")
}
if parsed.Fragment != "" {
return fmt.Errorf("must not include a fragment")
}
return nil
}
func validateTransferPolicy(errs ValidationErrors, context string, policy TransferPolicy) ValidationErrors { func validateTransferPolicy(errs ValidationErrors, context string, policy TransferPolicy) ValidationErrors {
if policy.OnDestinationSame != TransferActionSkip && policy.OnDestinationSame != TransferActionFail { if policy.OnDestinationSame != TransferActionSkip && policy.OnDestinationSame != TransferActionFail {
errs = append(errs, context+".on_destination_same must be skip or fail") errs = append(errs, context+".on_destination_same must be skip or fail")

View File

@@ -1,6 +1,9 @@
package config package config
import "testing" import (
"strings"
"testing"
)
func TestValidatePublishTransformPolicy(t *testing.T) { func TestValidatePublishTransformPolicy(t *testing.T) {
tests := publishTransformPolicyCases() tests := publishTransformPolicyCases()
@@ -145,6 +148,28 @@ func TestValidateLinks(t *testing.T) {
} }
} }
func TestValidateLinksReportsFieldContext(t *testing.T) {
cfg := Config{Pipelines: []Pipeline{{
ID: "reports",
Source: Backend{Backend: BackendLocal, Path: "/source"},
Destinations: []Destination{{
ID: "web",
Backend: BackendLocal,
Path: "/destination",
Links: &Links{BaseURL: "https://reports.example.com/archive?preview=1", Primary: LinkPrimaryAuto},
}},
}}}
ApplyDefaults(&cfg)
err := Validate(cfg)
if err == nil {
t.Fatal("Validate() error = nil, want error")
}
want := "pipelines[0].destinations[0].links.base_url must not include a query string"
if !strings.Contains(err.Error(), want) {
t.Fatalf("Validate() error = %q, want %q", err, want)
}
}
type publishTransformPolicyCase struct { type publishTransformPolicyCase struct {
name string name string
publish PublishPolicy publish PublishPolicy

26
internal/link/url.go Normal file
View File

@@ -0,0 +1,26 @@
package link
import (
"fmt"
"net/url"
)
func ValidateHTTPURL(value string) error {
parsed, err := url.Parse(value)
if err != nil {
return fmt.Errorf("must be a valid URL")
}
if parsed.Scheme != "http" && parsed.Scheme != "https" {
return fmt.Errorf("must use http or https")
}
if parsed.Host == "" {
return fmt.Errorf("must include a host")
}
if parsed.RawQuery != "" {
return fmt.Errorf("must not include a query string")
}
if parsed.Fragment != "" {
return fmt.Errorf("must not include a fragment")
}
return nil
}

39
internal/link/url_test.go Normal file
View File

@@ -0,0 +1,39 @@
package link
import (
"strings"
"testing"
)
func TestValidateHTTPURL(t *testing.T) {
tests := []struct {
name string
value string
wantErr string
}{
{name: "http", value: "http://reports.example.com/archive"},
{name: "https", value: "https://reports.example.com/archive"},
{name: "missing host", value: "https:///archive", wantErr: "must include a host"},
{name: "unsupported scheme", value: "ftp://reports.example.com/archive", wantErr: "must use http or https"},
{name: "query string", value: "https://reports.example.com/archive?preview=1", wantErr: "must not include a query string"},
{name: "fragment", value: "https://reports.example.com/archive#top", wantErr: "must not include a fragment"},
{name: "parse failure", value: "http://[::1", wantErr: "must be a valid URL"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidateHTTPURL(tt.value)
if tt.wantErr == "" {
if err != nil {
t.Fatalf("ValidateHTTPURL() error = %v", err)
}
return
}
if err == nil {
t.Fatal("ValidateHTTPURL() error = nil, want error")
}
if !strings.Contains(err.Error(), tt.wantErr) {
t.Fatalf("ValidateHTTPURL() error = %q, want %q", err, tt.wantErr)
}
})
}
}

View File

@@ -6,6 +6,7 @@ import (
"strings" "strings"
"gitea.maximumdirect.net/eric/distributor/internal/config" "gitea.maximumdirect.net/eric/distributor/internal/config"
"gitea.maximumdirect.net/eric/distributor/internal/link"
"gitea.maximumdirect.net/eric/distributor/internal/state" "gitea.maximumdirect.net/eric/distributor/internal/state"
) )
@@ -13,6 +14,9 @@ func PlanLinks(req Request, outputs []Output) ([]Output, string, error) {
if req.Links == nil { if req.Links == nil {
return outputs, "", nil return outputs, "", nil
} }
if err := link.ValidateHTTPURL(req.Links.BaseURL); err != nil {
return nil, "", fmt.Errorf("link base URL: %w", err)
}
linked := make([]Output, 0, len(outputs)) linked := make([]Output, 0, len(outputs))
for _, output := range outputs { for _, output := range outputs {
outputURL, err := OutputURL(req.Links.BaseURL, req.DestinationBundlePath, output.DestinationPath) outputURL, err := OutputURL(req.Links.BaseURL, req.DestinationBundlePath, output.DestinationPath)

View File

@@ -1,6 +1,7 @@
package publish package publish
import ( import (
"strings"
"testing" "testing"
"gitea.maximumdirect.net/eric/distributor/internal/config" "gitea.maximumdirect.net/eric/distributor/internal/config"
@@ -147,3 +148,22 @@ func TestPlanLinksLeavesOutputsUnchangedWithoutConfig(t *testing.T) {
t.Fatalf("output URL = %q, want empty", linked[0].URL) t.Fatalf("output URL = %q, want empty", linked[0].URL)
} }
} }
func TestPlanLinksValidatesBaseURLBeforePlanning(t *testing.T) {
_, _, err := PlanLinks(Request{
DestinationBundlePath: "daily",
Links: &config.Links{
BaseURL: "https://reports.example.com/archive?preview=1",
Primary: config.LinkPrimaryAuto,
},
}, []Output{{
DestinationPath: "report.md",
Kind: state.OutputKindSource,
}})
if err == nil {
t.Fatal("PlanLinks() error = nil, want error")
}
if !strings.Contains(err.Error(), "link base URL: must not include a query string") {
t.Fatalf("PlanLinks() error = %q, want link base URL context", err)
}
}

View File

@@ -179,6 +179,23 @@ func TestParseRejectsInvalidOutputMetadata(t *testing.T) {
} }
} }
func TestValidateReportsURLFieldContext(t *testing.T) {
source := validManifest(t)
state := *withState(t, source, func(s *DistributorState) {
s.Links = &LinkState{PrimaryURL: "https://reports.example.com/archive#top"}
})
err := Validate(state)
assertStateErrorContains(t, err, "state links.primary_url")
assertStateErrorContains(t, err, "must not include a fragment")
state = *withState(t, source, func(s *DistributorState) {
s.Outputs[0].URL = "https://reports.example.com/archive?preview=1"
})
err = Validate(state)
assertStateErrorContains(t, err, "state outputs[0].url")
assertStateErrorContains(t, err, "must not include a query string")
}
func TestParseRejectsMalformedPublishedTimestamp(t *testing.T) { func TestParseRejectsMalformedPublishedTimestamp(t *testing.T) {
body := strings.Replace(validStateJSON(t), `"published_at": "2026-05-30T11:12:00Z"`, `"published_at": "May 30"`, 1) body := strings.Replace(validStateJSON(t), `"published_at": "2026-05-30T11:12:00Z"`, `"published_at": "May 30"`, 1)
_, err := Parse([]byte(body)) _, err := Parse([]byte(body))

View File

@@ -2,9 +2,9 @@ package state
import ( import (
"fmt" "fmt"
"net/url"
"gitea.maximumdirect.net/eric/distributor/internal/bundle" "gitea.maximumdirect.net/eric/distributor/internal/bundle"
"gitea.maximumdirect.net/eric/distributor/internal/link"
"gitea.maximumdirect.net/eric/distributor/internal/storage" "gitea.maximumdirect.net/eric/distributor/internal/storage"
) )
@@ -30,7 +30,7 @@ func Validate(s DistributorState) error {
return fmt.Errorf("state source.manifest: %w", err) return fmt.Errorf("state source.manifest: %w", err)
} }
if s.Links != nil && s.Links.PrimaryURL != "" { if s.Links != nil && s.Links.PrimaryURL != "" {
if err := validateStateURL(s.Links.PrimaryURL); err != nil { if err := link.ValidateHTTPURL(s.Links.PrimaryURL); err != nil {
return fmt.Errorf("state links.primary_url: %w", err) return fmt.Errorf("state links.primary_url: %w", err)
} }
} }
@@ -70,7 +70,7 @@ func validateOutput(index int, output OutputFile) error {
return fmt.Errorf("state outputs[%d].transform is required for generated output", index) return fmt.Errorf("state outputs[%d].transform is required for generated output", index)
} }
if output.URL != "" { if output.URL != "" {
if err := validateStateURL(output.URL); err != nil { if err := link.ValidateHTTPURL(output.URL); err != nil {
return fmt.Errorf("state outputs[%d].url: %w", index, err) return fmt.Errorf("state outputs[%d].url: %w", index, err)
} }
} }
@@ -82,23 +82,3 @@ func validateOutput(index int, output OutputFile) error {
} }
return nil return nil
} }
func validateStateURL(value string) error {
parsed, err := url.Parse(value)
if err != nil {
return err
}
if parsed.Scheme != "http" && parsed.Scheme != "https" {
return fmt.Errorf("must use http or https")
}
if parsed.Host == "" {
return fmt.Errorf("must include a host")
}
if parsed.RawQuery != "" {
return fmt.Errorf("must not include a query string")
}
if parsed.Fragment != "" {
return fmt.Errorf("must not include a fragment")
}
return nil
}