From df8aa2f8044ba3aa034b51aa0a36c0e98aa72435 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Apr 2026 09:23:27 -0700 Subject: [PATCH] Remove IsValidExternalURL/IsAPIURL and use IsValidURL at call sites (#37364) This PR simplifies URL validation by removing `IsValidExternalURL` and `IsAPIURL` from `modules/validation/helpers.go` and switching repository settings/API callers to `IsValidURL`. It also aligns tracker-format validation and tests with the new helper surface. - **Validation helpers** - Removed `IsValidExternalURL` and `IsAPIURL`. - Updated `IsValidExternalTrackerURLFormat` to depend on `IsValidURL`. - **Caller updates** - Replaced `validation.IsValidExternalURL(...)` with `validation.IsValidURL(...)` in: - `routers/web/repo/setting/setting.go` - `routers/api/v1/repo/repo.go` - **Tests** - Removed tests dedicated to `IsValidExternalURL`. - Updated tracker-format test expectations to match `IsValidURL`-based behavior. ```go // before if !validation.IsValidExternalURL(form.ExternalTrackerURL) { ... } // after if !validation.IsValidURL(form.ExternalTrackerURL) { ... } ``` Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: wxiaoguang <2114189+wxiaoguang@users.noreply.github.com> --- modules/validation/helpers.go | 34 +------------------- modules/validation/helpers_test.go | 49 +---------------------------- routers/api/v1/repo/repo.go | 4 +-- routers/web/repo/setting/setting.go | 4 +-- 4 files changed, 6 insertions(+), 85 deletions(-) diff --git a/modules/validation/helpers.go b/modules/validation/helpers.go index ee05de74bdf..7695529beba 100644 --- a/modules/validation/helpers.go +++ b/modules/validation/helpers.go @@ -4,7 +4,6 @@ package validation import ( - "net" "net/url" "regexp" "slices" @@ -33,10 +32,6 @@ var globalVars = sync.OnceValue(func() *globalVarsStruct { } }) -func isLoopbackIP(ip string) bool { - return net.ParseIP(ip).IsLoopback() -} - // IsValidURL checks if URL is valid func IsValidURL(uri string) bool { if u, err := url.ParseRequestURI(uri); err != nil || @@ -85,36 +80,9 @@ func IsEmailDomainListed(globs []glob.Glob, email string) bool { return false } -// IsAPIURL checks if URL is current Gitea instance API URL -func IsAPIURL(uri string) bool { - return strings.HasPrefix(strings.ToLower(uri), strings.ToLower(setting.AppURL+"api")) -} - -// IsValidExternalURL checks if URL is valid external URL -func IsValidExternalURL(uri string) bool { - if !IsValidURL(uri) || IsAPIURL(uri) { - return false - } - - u, err := url.ParseRequestURI(uri) - if err != nil { - return false - } - - // Currently check only if not loopback IP is provided to keep compatibility - if isLoopbackIP(u.Hostname()) || strings.ToLower(u.Hostname()) == "localhost" { - return false - } - - // TODO: Later it should be added to allow local network IP addresses - // only if allowed by special setting - - return true -} - // IsValidExternalTrackerURLFormat checks if URL matches required syntax for external trackers func IsValidExternalTrackerURLFormat(uri string) bool { - if !IsValidExternalURL(uri) { + if !IsValidURL(uri) { return false } vars := globalVars() diff --git a/modules/validation/helpers_test.go b/modules/validation/helpers_test.go index 75f73b97fca..a3c3acb16c4 100644 --- a/modules/validation/helpers_test.go +++ b/modules/validation/helpers_test.go @@ -6,9 +6,6 @@ package validation import ( "testing" - "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/test" - "github.com/stretchr/testify/assert" ) @@ -47,51 +44,7 @@ func Test_IsValidURL(t *testing.T) { } } -func Test_IsValidExternalURL(t *testing.T) { - defer test.MockVariableValue(&setting.AppURL, "https://try.gitea.io/")() - - cases := []struct { - description string - url string - valid bool - }{ - { - description: "Current instance URL", - url: "https://try.gitea.io/test", - valid: true, - }, - { - description: "Loopback IPv4 URL", - url: "http://127.0.1.1:5678/", - valid: false, - }, - { - description: "Current instance API URL", - url: "https://try.gitea.io/api/v1/user/follow", - valid: false, - }, - { - description: "Local network URL", - url: "http://192.168.1.2/api/v1/user/follow", - valid: true, - }, - { - description: "Local URL", - url: "http://LOCALHOST:1234/whatever", - valid: false, - }, - } - - for _, testCase := range cases { - t.Run(testCase.description, func(t *testing.T) { - assert.Equal(t, testCase.valid, IsValidExternalURL(testCase.url)) - }) - } -} - func Test_IsValidExternalTrackerURLFormat(t *testing.T) { - defer test.MockVariableValue(&setting.AppURL, "https://try.gitea.io/")() - cases := []struct { description string url string @@ -105,7 +58,7 @@ func Test_IsValidExternalTrackerURLFormat(t *testing.T) { { description: "Local external tracker URL with all placeholders", url: "https://127.0.0.1/{user}/{repo}/issues/{index}", - valid: false, + valid: true, }, { description: "External tracker URL with typo placeholder", diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 4a5091fded2..24f486be9db 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -758,7 +758,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { if opts.HasIssues != nil { if *opts.HasIssues && opts.ExternalTracker != nil && !unit_model.TypeExternalTracker.UnitGlobalDisabled() { // Check that values are valid - if !validation.IsValidExternalURL(opts.ExternalTracker.ExternalTrackerURL) { + if !validation.IsValidURL(opts.ExternalTracker.ExternalTrackerURL) { err := errors.New("External tracker URL not valid") ctx.APIError(http.StatusUnprocessableEntity, err) return err @@ -820,7 +820,7 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error { if opts.HasWiki != nil { if *opts.HasWiki && opts.ExternalWiki != nil && !unit_model.TypeExternalWiki.UnitGlobalDisabled() { // Check that values are valid - if !validation.IsValidExternalURL(opts.ExternalWiki.ExternalWikiURL) { + if !validation.IsValidURL(opts.ExternalWiki.ExternalWikiURL) { err := errors.New("External wiki URL not valid") ctx.APIError(http.StatusUnprocessableEntity, "Invalid external wiki URL") return err diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 118f16d8f1f..2c3ce2bc02e 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -527,7 +527,7 @@ func handleSettingsPostAdvanced(ctx *context.Context) { } if form.EnableWiki && form.EnableExternalWiki && !unit_model.TypeExternalWiki.UnitGlobalDisabled() { - if !validation.IsValidExternalURL(form.ExternalWikiURL) { + if !validation.IsValidURL(form.ExternalWikiURL) { ctx.Flash.Error(ctx.Tr("repo.settings.external_wiki_url_error")) ctx.Redirect(repo.Link() + "/settings") return @@ -557,7 +557,7 @@ func handleSettingsPostAdvanced(ctx *context.Context) { } if form.EnableIssues && form.EnableExternalTracker && !unit_model.TypeExternalTracker.UnitGlobalDisabled() { - if !validation.IsValidExternalURL(form.ExternalTrackerURL) { + if !validation.IsValidURL(form.ExternalTrackerURL) { ctx.Flash.Error(ctx.Tr("repo.settings.external_tracker_url_error")) ctx.Redirect(repo.Link() + "/settings") return