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>
This commit is contained in:
Copilot
2026-04-22 09:23:27 -07:00
committed by GitHub
parent f52b6f3315
commit df8aa2f804
4 changed files with 6 additions and 85 deletions

View File

@@ -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()

View File

@@ -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",

View File

@@ -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

View File

@@ -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