Skip to content

Commit

Permalink
internal/code: reject all non-v0.0.0 pseudo-versions
Browse files Browse the repository at this point in the history 
The intention of the current ModuleHandler is to serve only v0.0.0
pseudo-versions. This requires checking the version component of the
incoming module request, and verifying that it is exactly v0.0.0 and
not another value.

This wasn't done previously, which means an explicit request to a
non-v0.0.0 pseudo-version would be served by ModuleHandler as if such
a version existed. This would be problematic if someone started to
depend on such an unintentional version, or if the Go module mirror
indexed it and made a permanent record of it existing.

Non-v0.0.0 pseudo-versions were never advertised by the list endpoint
or elsewhere, so the chance of it happening was low. Fortunately, no
such explicit requests were made, and unintentional versions were not
indexed by the Go module mirror.

This serves as an example of the risk one undertakes when implementing
a custom module proxy implementation. It's very important to serve the
precise versions that one intends to serve, and no other versions.

Simplify code to satisfy the current narrow needs. When the scope of
ModuleHandler expands to support non-v0.0.0 pseudo-versions, then we
can go back to using more general pseudo-version parsing code.

Updates golang/go#24031
  • Loading branch information
@dmitshur
dmitshur committed Dec 14, 2019
1 parent ebebcaa commit a776116
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 45 deletions.
6 changes: 3 additions & 3 deletions internal/code/module.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ func (h ModuleHandler) ServeModule(w http.ResponseWriter, req *http.Request) err
return h.serveList(req.Context(), w, gitDir)
}

// Parse the time and revision from the pseudo-version.
versionTime, versionRevision, err := mod.ParsePseudoVersion(version)
if err != nil || len(versionRevision) != 12 || !mod.AllHex(versionRevision) {
// Parse the time and revision from the v0.0.0 pseudo-version.
versionTime, versionRevision, err := mod.ParseV000PseudoVersion(version)
if err != nil {
return os.ErrNotExist
}

Expand Down
81 changes: 75 additions & 6 deletions internal/code/module_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,28 +33,33 @@ func TestModuleHandler(t *testing.T) {
})

for _, tt := range []struct {
url string
wantType string
wantBody string
wantSum string // Expected checksum for module .zip (as in go.sum).
wantModSum string // Expected checksum for go.mod file (as in go.sum).
name string
url string
wantNotExist bool // If true, expect 404 status code.
wantType string
wantBody string
wantSum string // Expected checksum for module .zip (as in go.sum).
wantModSum string // Expected checksum for go.mod file (as in go.sum).
}{
// Module emptyrepo tests.
{
name: "emptyrepo version list",
url: "/api/module/dmitri.shuralyov.com/emptyrepo/@v/list",
wantType: "text/plain; charset=utf-8",
wantBody: "",
},

// Module kebabcase tests.
{
name: "kebabcase version list",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/list",
wantType: "text/plain; charset=utf-8",
wantBody: `v0.0.0-20170912031248-a1d95f8919b5
v0.0.0-20170914162131-bf160e40a791
`,
},
{
name: "kebabcase version 1 info",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v0.0.0-20170912031248-a1d95f8919b5.info",
wantType: "application/json",
wantBody: `{
Expand All @@ -64,6 +69,7 @@ v0.0.0-20170914162131-bf160e40a791
`,
},
{
name: "kebabcase version 2 info",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v0.0.0-20170914162131-bf160e40a791.info",
wantType: "application/json",
wantBody: `{
Expand All @@ -73,30 +79,35 @@ v0.0.0-20170914162131-bf160e40a791
`,
},
{
name: "kebabcase version 1 mod",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v0.0.0-20170912031248-a1d95f8919b5.mod",
wantType: "text/plain; charset=utf-8",
wantBody: "module dmitri.shuralyov.com/kebabcase\n",
wantModSum: "h1:zlZLgG71KSMQ+9XWuKJgSRws1h0iMspYv2y69MUzNFo=",
},
{
name: "kebabcase version 2 mod",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v0.0.0-20170914162131-bf160e40a791.mod",
wantType: "text/plain; charset=utf-8",
wantBody: "module dmitri.shuralyov.com/kebabcase\n",
wantModSum: "h1:zlZLgG71KSMQ+9XWuKJgSRws1h0iMspYv2y69MUzNFo=",
},
{
name: "kebabcase version 1 zip",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v0.0.0-20170912031248-a1d95f8919b5.zip",
wantType: "application/zip",
wantSum: "h1:xUU8cZj0tfJxDjfyJ6xLLh6G615T10e16A1mxCoygiI=",
},
{
name: "kebabcase version 2 zip",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v0.0.0-20170914162131-bf160e40a791.zip",
wantType: "application/zip",
wantSum: "h1:Lz+BA1qBebmQ4Ev2oGecFqNFK4jq5orgAPanU0rsL98=",
},

// Module scratch tests.
{
name: "scratch version list",
url: "/api/module/dmitri.shuralyov.com/scratch/@v/list",
wantType: "text/plain; charset=utf-8",
wantBody: `v0.0.0-20171129001319-b205cb69d5d7
Expand All @@ -105,12 +116,70 @@ v0.0.0-20180125023930-cdbe493822d6
v0.0.0-20180326031431-f628922a6885
`,
},

// Versions that do not exist must serve 404.
{
name: "wrong timestamp",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v0.0.0-11111111111111-a1d95f8919b5.info",
wantNotExist: true,
},
{
name: "wrong revision",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v0.0.0-20170912031248-aaaaaaaaaaaa.info",
wantNotExist: true,
},
{
name: "revision length is not 12",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v0.0.0-20170912031248-a1d95f8919b.info",
wantNotExist: true,
},
{
name: "wrong separator",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v0.0.0-20170912031248.a1d95f8919b5.info",
wantNotExist: true,
},
{
name: "revision is not all lower-case",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v0.0.0-20170912031248-!a1d95f8919b5.info",
wantNotExist: true,
},
{
name: "incompatible version",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v0.0.0-20170912031248-a1d95f8919b5+incompatible.info",
wantNotExist: true,
},
{
name: "version v1.0.0",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v1.0.0-20170912031248-a1d95f8919b5.info",
wantNotExist: true,
},
{
name: "version v2.0.0",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v2.0.0-20170912031248-a1d95f8919b5.info",
wantNotExist: true,
},
{
name: "pseudo-version after v1.2.3-pre",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v1.2.3-pre.0.20170912031248-a1d95f8919b5.info",
wantNotExist: true,
},
{
name: "pseudo-version after v1.2.3",
url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v1.2.4-0.20170912031248-a1d95f8919b5.info",
wantNotExist: true,
},
} {
t.Run(tt.url[1:], func(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
req := httptest.NewRequest(http.MethodGet, tt.url, nil)
rr := httptest.NewRecorder()
mux.ServeHTTP(rr, req)
resp := rr.Result()
if tt.wantNotExist {
if got, want := resp.StatusCode, http.StatusNotFound; got != want {
t.Errorf("got status code %d %s, want %d %s", got, http.StatusText(got), want, http.StatusText(want))
}
return
}
if got, want := resp.StatusCode, http.StatusOK; got != want {
t.Errorf("got status code %d %s, want %d %s", got, http.StatusText(got), want, http.StatusText(want))
}
Expand Down
64 changes: 28 additions & 36 deletions internal/mod/mod.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"bytes"
"fmt"
"io"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -65,14 +64,20 @@ func PseudoVersion(major, older string, t time.Time, rev string) string {
return v + patch + "-0." + segment + build
}

// ParsePseudoVersion returns the time stamp and the revision identifier
// of the pseudo-version v.
// It returns an error if v is not a pseudo-version or if the time stamp
// embedded in the pseudo-version is not a valid time.
func ParsePseudoVersion(v string) (_ time.Time, rev string, err error) {
timestamp, rev, err := parsePseudoVersion(v)
if err != nil {
return time.Time{}, "", err
// ParseV000PseudoVersion returns the time stamp and the revision identifier
// of the v0.0.0 pseudo-version v.
// It returns an error if v is not a valid v0.0.0 pseudo-version
// of the form "v0.0.0-yyyymmddhhmmss-abcdef123456".
func ParseV000PseudoVersion(v string) (_ time.Time, rev string, err error) {
if len(v) != len("v0.0.0-yyyymmddhhmmss-abcdef123456") ||
!strings.HasPrefix(v, "v0.0.0-") ||
v[len("v0.0.0-yyyymmddhhmmss")] != '-' {
return time.Time{}, "", fmt.Errorf("not a v0.0.0 pseudo-version %q", v)
}
timestamp, rev := v[len("v0.0.0-"):len("v0.0.0-yyyymmddhhmmss")], v[len("v0.0.0-yyyymmddhhmmss-"):]
if !allDec(timestamp) ||
!allHex(rev) {
return time.Time{}, "", fmt.Errorf("not a v0.0.0 pseudo-version %q", v)
}
t, err := time.Parse("20060102150405", timestamp)
if err != nil {
Expand All @@ -81,37 +86,24 @@ func ParsePseudoVersion(v string) (_ time.Time, rev string, err error) {
return t, rev, nil
}

func parsePseudoVersion(v string) (timestamp, rev string, err error) {
if !isPseudoVersion(v) {
return "", "", fmt.Errorf("malformed pseudo-version %q", v)
}
v = strings.TrimSuffix(v, "+incompatible")
j := strings.LastIndex(v, "-")
v, rev = v[:j], v[j+1:]
i := strings.LastIndex(v, "-")
if j := strings.LastIndex(v, "."); j > i {
timestamp = v[j+1:]
} else {
timestamp = v[i+1:]
// allDec reports whether timestamp is entirely decimal digits.
func allDec(timestamp string) bool {
for _, b := range []byte(timestamp) {
ok := '0' <= b && b <= '9'
if !ok {
return false
}
}
return timestamp, rev, nil
}

// isPseudoVersion reports whether v is a pseudo-version.
func isPseudoVersion(v string) bool {
return strings.Count(v, "-") >= 2 && semver.IsValid(v) && pseudoVersionRE.MatchString(v)
return true
}

var pseudoVersionRE = regexp.MustCompile(`^v[0-9]+\.(0\.0-|\d+\.\d+-([^+]*\.)?0\.)\d{14}-[A-Za-z0-9]+(\+incompatible)?$`)

// AllHex reports whether the revision rev is entirely lower-case hexadecimal digits.
func AllHex(rev string) bool {
for i := 0; i < len(rev); i++ {
c := rev[i]
if '0' <= c && c <= '9' || 'a' <= c && c <= 'f' {
continue
// allHex reports whether the revision rev is entirely lower-case hexadecimal digits.
func allHex(rev string) bool {
for _, b := range []byte(rev) {
ok := '0' <= b && b <= '9' || 'a' <= b && b <= 'f'
if !ok {
return false
}
return false
}
return true
}
Expand Down

0 comments on commit a776116

Please sign in to comment.