Skip to content

Commit

Permalink
internal/proxy: clean versions returned with spaces
Browse files Browse the repository at this point in the history
jFrog Artifactory's go module proxy incorrectly responds to /@v/list with
versions that all have a space suffix ("v1.2.3 " instead of "v1.2.3").
Currently, this causes an issue when we take that version and we try to add
things like ".info", resulting in URLs like GET "foo.com/v1.2.3 .info" instead
of "foo.com/v1.2.3.info".

This change cleans up versions with such a suffix to fix this issue, and adds
a test (and the ability to test) for this situation.

Fixes golang/go#71141

Change-Id: I12d188f736258b5e263e007c6f757b15fd47545d
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/640655
Reviewed-by: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
kokoro-CI: kokoro <[email protected]>
  • Loading branch information
jeanbza authored and jba committed Jan 8, 2025
1 parent 9bf769c commit 840de57
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 6 deletions.
2 changes: 1 addition & 1 deletion internal/proxy/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func (c *Client) Versions(ctx context.Context, modulePath string) (_ []string, e
collect := func(body io.Reader) error {
scanner := bufio.NewScanner(body)
for scanner.Scan() {
versions = append(versions, scanner.Text())
versions = append(versions, strings.TrimSpace(scanner.Text()))
}
return scanner.Err()
}
Expand Down
7 changes: 6 additions & 1 deletion internal/proxy/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,16 @@ func TestListVersions(t *testing.T) {
Version: "v1.3.0",
Files: map[string]string{"bar.go": "package bar\nconst Version = 1.3"},
},
{
ModulePath: sample.ModulePath,
Version: "v1.4.0 ",
Files: map[string]string{"bar.go": "package bar\nconst Version = 1.4"},
},
}
client, teardownProxy := proxytest.SetupTestClient(t, testModules)
defer teardownProxy()

want := []string{"v1.1.0", "v1.2.0"}
want := []string{"v1.4.0", "v1.1.0", "v1.2.0"}
got, err := client.Versions(ctx, sample.ModulePath)
if err != nil {
t.Fatal(err)
Expand Down
12 changes: 11 additions & 1 deletion internal/proxy/proxytest/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
// Package proxytest supports testing with the proxy.
package proxytest

import "fmt"
import (
"fmt"
"strings"
)

// Module represents a module version used by the proxy server.
type Module struct {
Expand All @@ -16,6 +19,13 @@ type Module struct {
zip []byte
}

// Some module proxies incorrectly return a space after the version. Tests may
// simulate that that behavior by giving m.Version a space suffix. This function
// will always, however, return the correct form for the version.
func (m *Module) TidyVersion() string {
return strings.TrimSpace(m.Version)
}

// ChangePath returns a copy of m with a different module path.
func (m *Module) ChangePath(modulePath string) *Module {
m2 := *m
Expand Down
6 changes: 3 additions & 3 deletions internal/proxy/proxytest/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,15 @@ func (s *Server) handleMod(m *Module) {
if goMod == "" {
goMod = defaultGoMod(m.ModulePath)
}
s.mux.HandleFunc(fmt.Sprintf("/%s/@v/%s.mod", m.ModulePath, m.Version),
s.mux.HandleFunc(fmt.Sprintf("/%s/@v/%s.mod", m.ModulePath, m.TidyVersion()),
func(w http.ResponseWriter, r *http.Request) {
http.ServeContent(w, r, m.ModulePath, time.Now(), strings.NewReader(goMod))
})
}

// handleZip creates a zip endpoint for the specified module version.
func (s *Server) handleZip(m *Module) {
s.mux.HandleFunc(fmt.Sprintf("/%s/@v/%s.zip", m.ModulePath, m.Version),
s.mux.HandleFunc(fmt.Sprintf("/%s/@v/%s.zip", m.ModulePath, m.TidyVersion()),
func(w http.ResponseWriter, r *http.Request) {
s.mu.Lock()
s.zipRequests++
Expand Down Expand Up @@ -150,7 +150,7 @@ func (s *Server) addModule(m *Module, hasVersions bool) {
})
}
}
s.handleInfo(m.ModulePath, m.Version, m.NotCached)
s.handleInfo(m.ModulePath, m.TidyVersion(), m.NotCached)
s.handleMod(m)
s.handleZip(m)

Expand Down

0 comments on commit 840de57

Please sign in to comment.