Skip to content

Commit aa9bb87

Browse files
committed
client: handle file URI scheme on windows
A file URI takes the form of file://host/path https://en.wikipedia.org/wiki/File_URI_scheme On windows, for example, vulndb located in c:\dir\vulndb will be file:///c:/dir/vulndb Previously, the code took `file://` prefix and attempted to use the remaining as a directory of local vulndb. On windows, that caused to os.Stat on /c:/dir/vulndb when a correctly encoded URI was passed in. Turned out file-uri parsing is a known, complex issue. See golang/go#32456 for context. This CL includes the code copied from the Go project. https://github.com/golang/go/blob/2ebe77a2fda1ee9ff6fd9a3e08933ad1ebaea039/src/cmd/go/internal/web/url.go Updated the tests to exercise the windows-like file path correctly when testing on windows. Previously, tests depended on relative paths or assumed an incorrect form of windows file uri (e.g. file://C:\workdir\gopath\src\golang.org\x\vuln\cmd\govulncheck/testdata/vulndb) Change-Id: I5fc451e5ca44649b9623daee28ee3210a7b2b96c Reviewed-on: https://go-review.googlesource.com/c/vuln/+/438175 Run-TryBot: Hyang-Ah Hana Kim <[email protected]> Reviewed-by: Jonathan Amsterdam <[email protected]> Reviewed-by: Zvonimir Pavlinovic <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Bryan Mills <[email protected]>
1 parent cb35227 commit aa9bb87

File tree

5 files changed

+383
-15
lines changed

5 files changed

+383
-15
lines changed

client/client.go

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import (
4545
"golang.org/x/mod/module"
4646
"golang.org/x/vuln/internal"
4747
"golang.org/x/vuln/internal/derrors"
48+
"golang.org/x/vuln/internal/web"
4849
"golang.org/x/vuln/osv"
4950
)
5051

@@ -436,17 +437,16 @@ type Options struct {
436437
func NewClient(sources []string, opts Options) (_ Client, err error) {
437438
defer derrors.Wrap(&err, "NewClient(%v, opts)", sources)
438439
c := &client{}
439-
for _, uri := range sources {
440-
uri = strings.TrimRight(uri, "/")
441-
// should parse the URI out here instead of in there
442-
switch {
443-
case strings.HasPrefix(uri, "http://") || strings.HasPrefix(uri, "https://"):
444-
hs := &httpSource{url: uri}
445-
url, err := url.Parse(uri)
446-
if err != nil {
447-
return nil, err
448-
}
449-
hs.dbName = url.Hostname()
440+
for _, source := range sources {
441+
source = strings.TrimRight(source, "/") // TODO: why?
442+
uri, err := url.Parse(source)
443+
if err != nil {
444+
return nil, err
445+
}
446+
switch uri.Scheme {
447+
case "http", "https":
448+
hs := &httpSource{url: uri.String()}
449+
hs.dbName = uri.Hostname()
450450
if opts.HTTPCache != nil {
451451
hs.cache = opts.HTTPCache
452452
}
@@ -456,8 +456,11 @@ func NewClient(sources []string, opts Options) (_ Client, err error) {
456456
hs.c = new(http.Client)
457457
}
458458
c.sources = append(c.sources, hs)
459-
case strings.HasPrefix(uri, "file://"):
460-
dir := strings.TrimPrefix(uri, "file://")
459+
case "file":
460+
dir, err := web.URLToFilePath(uri)
461+
if err != nil {
462+
return nil, err
463+
}
461464
fi, err := os.Stat(dir)
462465
if err != nil {
463466
return nil, err

client/client_test.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package client
77
import (
88
"context"
99
"encoding/json"
10+
"fmt"
1011
"net/http"
1112
"net/http/httptest"
1213
"net/url"
@@ -20,6 +21,7 @@ import (
2021

2122
"github.com/google/go-cmp/cmp"
2223
"golang.org/x/vuln/internal"
24+
"golang.org/x/vuln/internal/web"
2325
"golang.org/x/vuln/osv"
2426
)
2527

@@ -85,7 +87,17 @@ func newTestServer() *httptest.Server {
8587
return httptest.NewServer(mux)
8688
}
8789

88-
const localURL = "file://testdata/vulndb"
90+
var localURL = func() string {
91+
absDir, err := filepath.Abs("testdata/vulndb")
92+
if err != nil {
93+
panic(fmt.Sprintf("failed to read testdata/vulndb: %v", err))
94+
}
95+
u, err := web.URLFromFilePath(absDir)
96+
if err != nil {
97+
panic(fmt.Sprintf("failed to read testdata/vulndb: %v", err))
98+
}
99+
return u.String()
100+
}()
89101

90102
func TestByModule(t *testing.T) {
91103
if runtime.GOOS == "js" {

cmd/govulncheck/main_command_118_test.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222

2323
"github.com/google/go-cmdtest"
2424
"golang.org/x/vuln/internal/buildtest"
25+
"golang.org/x/vuln/internal/web"
2526
)
2627

2728
var update = flag.Bool("update", false, "update test files with results")
@@ -50,8 +51,17 @@ func TestCommand(t *testing.T) {
5051
if inputFile != "" {
5152
return nil, errors.New("input redirection makes no sense")
5253
}
54+
// We set GOVERSION to always get the same results regardless of the underlying Go build system.
55+
vulndbDir, err := filepath.Abs(filepath.Join(testDir, "testdata", "vulndb"))
56+
if err != nil {
57+
return nil, err
58+
}
59+
govulndbURI, err := web.URLFromFilePath(vulndbDir)
60+
if err != nil {
61+
return nil, fmt.Errorf("failed to create GOVULNDB env var: %v", err)
62+
}
5363
// We set TEST_GOVERSION to always get the same results regardless of the underlying Go build system.
54-
cmd.Env = append(os.Environ(), "GOVULNDB=file://"+testDir+"/testdata/vulndb", "TEST_GOVERSION=go1.18")
64+
cmd.Env = append(os.Environ(), "GOVULNDB="+govulndbURI.String(), "TEST_GOVERSION=go1.18")
5565
out, err := cmd.CombinedOutput()
5666
out = filterGoFilePaths(out)
5767
out = filterStdlibVersions(out)

internal/web/url.go

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
// Copyright 2022 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// Code copied from
6+
// https://github.com/golang/go/blob/2ebe77a2fda1ee9ff6fd9a3e08933ad1ebaea039/src/cmd/go/internal/web/url.go
7+
// TODO(go.dev/issue/32456): if accepted, use the new API.
8+
9+
package web
10+
11+
import (
12+
"errors"
13+
"net/url"
14+
"path/filepath"
15+
"runtime"
16+
"strings"
17+
)
18+
19+
var errNotAbsolute = errors.New("path is not absolute")
20+
21+
// URLToFilePath converts a file-scheme url to a file path.
22+
func URLToFilePath(u *url.URL) (string, error) {
23+
if u.Scheme != "file" {
24+
return "", errors.New("non-file URL")
25+
}
26+
27+
checkAbs := func(path string) (string, error) {
28+
if !filepath.IsAbs(path) {
29+
return "", errNotAbsolute
30+
}
31+
return path, nil
32+
}
33+
34+
if u.Path == "" {
35+
if u.Host != "" || u.Opaque == "" {
36+
return "", errors.New("file URL missing path")
37+
}
38+
return checkAbs(filepath.FromSlash(u.Opaque))
39+
}
40+
41+
path, err := convertFileURLPath(u.Host, u.Path)
42+
if err != nil {
43+
return path, err
44+
}
45+
return checkAbs(path)
46+
}
47+
48+
// URLFromFilePath converts the given absolute path to a URL.
49+
func URLFromFilePath(path string) (*url.URL, error) {
50+
if !filepath.IsAbs(path) {
51+
return nil, errNotAbsolute
52+
}
53+
54+
// If path has a Windows volume name, convert the volume to a host and prefix
55+
// per https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/.
56+
if vol := filepath.VolumeName(path); vol != "" {
57+
if strings.HasPrefix(vol, `\\`) {
58+
path = filepath.ToSlash(path[2:])
59+
i := strings.IndexByte(path, '/')
60+
61+
if i < 0 {
62+
// A degenerate case.
63+
// \\host.example.com (without a share name)
64+
// becomes
65+
// file://host.example.com/
66+
return &url.URL{
67+
Scheme: "file",
68+
Host: path,
69+
Path: "/",
70+
}, nil
71+
}
72+
73+
// \\host.example.com\Share\path\to\file
74+
// becomes
75+
// file://host.example.com/Share/path/to/file
76+
return &url.URL{
77+
Scheme: "file",
78+
Host: path[:i],
79+
Path: filepath.ToSlash(path[i:]),
80+
}, nil
81+
}
82+
83+
// C:\path\to\file
84+
// becomes
85+
// file:///C:/path/to/file
86+
return &url.URL{
87+
Scheme: "file",
88+
Path: "/" + filepath.ToSlash(path),
89+
}, nil
90+
}
91+
92+
// /path/to/file
93+
// becomes
94+
// file:///path/to/file
95+
return &url.URL{
96+
Scheme: "file",
97+
Path: filepath.ToSlash(path),
98+
}, nil
99+
}
100+
101+
func convertFileURLPath(host, path string) (string, error) {
102+
if runtime.GOOS == "windows" {
103+
return convertFileURLPathWindows(host, path)
104+
}
105+
switch host {
106+
case "", "localhost":
107+
default:
108+
return "", errors.New("file URL specifies non-local host")
109+
}
110+
return filepath.FromSlash(path), nil
111+
}
112+
113+
func convertFileURLPathWindows(host, path string) (string, error) {
114+
if len(path) == 0 || path[0] != '/' {
115+
return "", errNotAbsolute
116+
}
117+
118+
path = filepath.FromSlash(path)
119+
120+
// We interpret Windows file URLs per the description in
121+
// https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/.
122+
123+
// The host part of a file URL (if any) is the UNC volume name,
124+
// but RFC 8089 reserves the authority "localhost" for the local machine.
125+
if host != "" && host != "localhost" {
126+
// A common "legacy" format omits the leading slash before a drive letter,
127+
// encoding the drive letter as the host instead of part of the path.
128+
// (See https://blogs.msdn.microsoft.com/freeassociations/2005/05/19/the-bizarre-and-unhappy-story-of-file-urls/.)
129+
// We do not support that format, but we should at least emit a more
130+
// helpful error message for it.
131+
if filepath.VolumeName(host) != "" {
132+
return "", errors.New("file URL encodes volume in host field: too few slashes?")
133+
}
134+
return `\\` + host + path, nil
135+
}
136+
137+
// If host is empty, path must contain an initial slash followed by a
138+
// drive letter and path. Remove the slash and verify that the path is valid.
139+
if vol := filepath.VolumeName(path[1:]); vol == "" || strings.HasPrefix(vol, `\\`) {
140+
return "", errors.New("file URL missing drive letter")
141+
}
142+
return path[1:], nil
143+
}

0 commit comments

Comments
 (0)