diff --git a/models/unittest/mock_http.go b/models/unittest/mock_http.go index 15b0d561e14..d6cb3e7b923 100644 --- a/models/unittest/mock_http.go +++ b/models/unittest/mock_http.go @@ -12,6 +12,7 @@ import ( "net/url" "os" "slices" + "strconv" "strings" "testing" @@ -103,6 +104,10 @@ func NewMockWebServer(t *testing.T, liveServerBaseURL, testDataDir string, liveM require.NoError(t, err, "creating fixture %s", fixturePath) defer out.Close() + if resp.StatusCode != http.StatusOK { + _, err = fmt.Fprintf(out, "HTTP/1.1 %d %s\n", resp.StatusCode, http.StatusText(resp.StatusCode)) + require.NoError(t, err, "writing fixture header for %s", liveURL) + } for _, name := range slices.Sorted(maps.Keys(resp.Header)) { if slices.Contains(ignoredHeaders, strings.ToLower(name)) { continue @@ -125,14 +130,21 @@ func NewMockWebServer(t *testing.T, liveServerBaseURL, testDataDir string, liveM replayed := strings.ReplaceAll(string(raw), liveServerBaseURL, mockURL) headers, body, _ := strings.Cut(replayed, "\n\n") + code := http.StatusOK for line := range strings.SplitSeq(headers, "\n") { + if strings.HasPrefix(line, "HTTP/") { + parts := strings.SplitN(line, " ", 3) + require.Len(t, parts, 3) + code, _ = strconv.Atoi(parts[1]) + continue + } name, value, ok := strings.Cut(line, ": ") if !ok || strings.EqualFold(name, "Content-Length") { continue } w.Header().Set(name, value) } - w.WriteHeader(http.StatusOK) + w.WriteHeader(code) _, err = w.Write([]byte(body)) require.NoError(t, err) }) diff --git a/services/migrations/_mock_data/TestOneDevDownloadRepo/GET_%2F~api%2Fserver%2Fversion b/services/migrations/_mock_data/TestOneDevDownloadRepo/GET_%2F~api%2Fserver%2Fversion new file mode 100644 index 00000000000..aed272fed7c --- /dev/null +++ b/services/migrations/_mock_data/TestOneDevDownloadRepo/GET_%2F~api%2Fserver%2Fversion @@ -0,0 +1 @@ +HTTP/1.1 404 Not Found diff --git a/services/migrations/onedev.go b/services/migrations/onedev.go index 596ec8c5373..cf6a80f73aa 100644 --- a/services/migrations/onedev.go +++ b/services/migrations/onedev.go @@ -5,6 +5,7 @@ package migrations import ( "context" + "errors" "fmt" "io" "net/http" @@ -77,6 +78,8 @@ type OneDevDownloader struct { maxIssueIndex int64 userMap map[int64]*onedevUser milestoneMap map[int64]string + + cachedServerVersion *version.Version } // NewOneDevDownloader creates a new downloader @@ -114,10 +117,10 @@ func (d *OneDevDownloader) LogString() string { return fmt.Sprintf("", d.baseURL, d.repoID, d.repoPath) } -func (d *OneDevDownloader) callAPI(ctx context.Context, endpoint string, parameter map[string]string, result any) error { +func (d *OneDevDownloader) makeRequest(ctx context.Context, endpoint string, parameter map[string]string) (*http.Response, error) { u, err := d.baseURL.Parse(endpoint) if err != nil { - return err + return nil, err } if parameter != nil { @@ -130,52 +133,75 @@ func (d *OneDevDownloader) callAPI(ctx context.Context, endpoint string, paramet req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil) if err != nil { - return err + return nil, err } + return d.client.Do(req) +} - resp, err := d.client.Do(req) +func (d *OneDevDownloader) callAPI(ctx context.Context, endpoint string, parameter map[string]string, result any) error { + resp, err := d.makeRequest(ctx, endpoint, parameter) if err != nil { return err } defer resp.Body.Close() - // special case to read OneDev server version, which is not valid JSON - if presult, ok := result.(**version.Version); ok { - bytes, err := io.ReadAll(io.LimitReader(resp.Body, maxOneDevVersionResponseSize+1)) - if err != nil { - return err - } - if len(bytes) > maxOneDevVersionResponseSize { - return fmt.Errorf("OneDev server version response exceeds %d bytes", maxOneDevVersionResponseSize) - } - vers, err := version.NewVersion(string(bytes)) - if err != nil { - return err - } - *presult = vers - return nil - } - decoder := json.NewDecoder(resp.Body) return decoder.Decode(&result) } +func (d *OneDevDownloader) getServerVersionByEndpoint(ctx context.Context, endpoint string) (*version.Version, error) { + resp, err := d.makeRequest(ctx, endpoint, nil) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("OneDev server version request failed: %s", resp.Status) + } + // This patch doesn't change the situation, "decoder.Decode(&result)" in callAPI still reads the entire body + bytes, err := io.ReadAll(io.LimitReader(resp.Body, maxOneDevVersionResponseSize+1)) + if err != nil { + return nil, err + } + if len(bytes) > maxOneDevVersionResponseSize { + return nil, fmt.Errorf("OneDev server version response exceeds %d bytes", maxOneDevVersionResponseSize) + } + ver, err := version.NewVersion(string(bytes)) + if err != nil { + return nil, fmt.Errorf("failed to parse OneDev server version %q: %w", string(bytes), err) + } + return ver, nil +} + +// getServerVersion reads the OneDev server version. +func (d *OneDevDownloader) getServerVersion(ctx context.Context) (_ *version.Version, retErr error) { + if d.cachedServerVersion != nil { + return d.cachedServerVersion, nil + } + // OneDev moved the endpoint from /~api/version/server to /~api/server/version in 16.8, so both are tried, the new first. + oneDevVersionEndpoints := []string{"/~api/server/version", "/~api/version/server"} + for _, endpoint := range oneDevVersionEndpoints { + ver, err := d.getServerVersionByEndpoint(ctx, endpoint) + if err == nil { + d.cachedServerVersion = ver + return ver, nil + } + retErr = errors.Join(err, fmt.Errorf("failed to get OneDev server version via %s: %w", endpoint, err)) + } + return nil, retErr +} + // GetRepoInfo returns repository information func (d *OneDevDownloader) GetRepoInfo(ctx context.Context) (*base.Repository, error) { // check OneDev server version - var serverVersion *version.Version - err := d.callAPI( - ctx, - "/~api/version/server", - nil, - &serverVersion, - ) + serverVersion, err := d.getServerVersion(ctx) if err != nil { - return nil, fmt.Errorf("failed to get OneDev server version; OneDev %s or newer required", OneDevRequiredVersion) + return nil, fmt.Errorf("OneDev %s or newer is required: %w", OneDevRequiredVersion, err) } requiredVersion, _ := version.NewVersion(OneDevRequiredVersion) if serverVersion.LessThan(requiredVersion) { - return nil, fmt.Errorf("OneDev %s or newer required; currently running OneDev %s", OneDevRequiredVersion, serverVersion) + return nil, fmt.Errorf("OneDev version %s is too old, %s or newer is required", serverVersion, OneDevRequiredVersion) } info := make([]struct { diff --git a/services/migrations/onedev_test.go b/services/migrations/onedev_test.go index eb2e47800d9..581dfe37cd3 100644 --- a/services/migrations/onedev_test.go +++ b/services/migrations/onedev_test.go @@ -4,35 +4,72 @@ package migrations import ( - "bytes" + "io" "net/http" "net/http/httptest" "net/url" "os" "path/filepath" "runtime" + "strings" + "sync/atomic" "testing" "time" "gitea.dev/models/unittest" base "gitea.dev/modules/migration" + "gitea.dev/modules/util" - "github.com/hashicorp/go-version" "github.com/stretchr/testify/assert" ) -func TestOneDevVersionResponseSizeLimit(t *testing.T) { +func TestOneDevServerVersion(t *testing.T) { + type respMapType map[string]struct { + code int + body string + } + var respMap atomic.Pointer[respMapType] server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write(bytes.Repeat([]byte("1"), maxOneDevVersionResponseSize+1)) + resp, ok := (*respMap.Load())[r.URL.Path] + w.WriteHeader(util.Iif(ok, resp.code, http.StatusNotFound)) + _, _ = io.WriteString(w, resp.body) })) defer server.Close() baseURL, err := url.Parse(server.URL) assert.NoError(t, err) - download := &OneDevDownloader{baseURL: baseURL, client: server.Client()} - var version *version.Version - err = download.callAPI(t.Context(), "/~api/version/server", nil, &version) - assert.Error(t, err) + downloader := func() *OneDevDownloader { return &OneDevDownloader{baseURL: baseURL, client: server.Client()} } + + t.Run("LargeResponse", func(t *testing.T) { + respMap.Store(&respMapType{"/~api/version/server": {code: http.StatusOK, body: strings.Repeat("x", maxOneDevVersionResponseSize+1)}}) + _, err := downloader().getServerVersion(t.Context()) + assert.ErrorContains(t, err, "server version response exceeds") + }) + + t.Run("LegacyEndpoint", func(t *testing.T) { + respMap.Store(&respMapType{"/~api/version/server": {code: http.StatusOK, body: "12.0.1"}}) + d := downloader() + v, err := d.getServerVersion(t.Context()) + assert.NoError(t, err) + assert.Equal(t, "12.0.1", v.String()) + respMap.Store(&respMapType{}) // remove the legacy endpoint, next call should still succeed by using cached version + v, err = d.getServerVersion(t.Context()) + assert.NoError(t, err) + assert.Equal(t, "12.0.1", v.String()) + }) + + t.Run("NewEndpoint", func(t *testing.T) { + respMap.Store(&respMapType{"/~api/server/version": {code: http.StatusOK, body: "16.9.7"}}) + v, err := downloader().getServerVersion(t.Context()) + assert.NoError(t, err) + assert.Equal(t, "16.9.7", v.String()) + }) + + t.Run("NoEndpoint", func(t *testing.T) { + respMap.Store(&respMapType{}) + _, err := downloader().getServerVersion(t.Context()) + assert.ErrorContains(t, err, "server version request failed") + }) } func TestOneDevDownloadRepo(t *testing.T) {