Skip to content

feat!(v2): csv - validate license URL and guess more if invalid #110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion csv.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func csvMain(_ *cobra.Command, args []string) error {
} else {
glog.Errorf("Error identifying license in %q: %v", lib.LicensePath, err)
}
url, err := lib.FileURL(context.Background(), lib.LicensePath)
url, err := lib.LicenseURL(context.Background())
if err == nil {
licenseURL = url
} else {
Expand Down
1 change: 1 addition & 0 deletions e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ func TestCsvCommandE2E(t *testing.T) {
"testdata/modules/cli02",
"testdata/modules/vendored03",
"testdata/modules/replace04",
"testdata/modules/modinsubdir05",
}
originalWorkDir, err := os.Getwd()
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions internal/third_party/pkgsite/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@ Local modifications:
- For pkgsite/internal/source, switched to use go log package, because glog conflicts with a test
dependency that also defines the "v" flag.
- Add a SetCommit method to type ModuleInfo in ./source/source_patch.go, more rationale explained in the method's comments.
- Added RepoFileURL and RepoRawURL methods to source.Info struct in file ./source/source_patch.go.
They are needed when accessing files outside of the module dir, but in the same repo.
38 changes: 38 additions & 0 deletions internal/third_party/pkgsite/source/source_patch.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@

package source

import (
"path"
"strings"
)

// This file includes all local additions to source package for google/go-licenses use-cases.

// SetCommit overrides commit to a specified commit. Usually, you should pass your version to
Expand All @@ -31,3 +36,36 @@ func (i *Info) SetCommit(commit string) {
}
i.commit = commit
}

// RepoFileURL returns a URL for a file whose pathname is relative to the repo's home directory instead of the module's.
func (i *Info) RepoFileURL(pathname string) string {
if i == nil {
return ""
}
dir, base := path.Split(pathname)
return expand(i.templates.File, map[string]string{
"repo": i.repoURL,
"importPath": path.Join(strings.TrimPrefix(i.repoURL, "https://"), dir),
"commit": i.commit,
"dir": dir,
"file": pathname,
"base": base,
})
}

// RepoRawURL returns a URL referring to the raw contents of a file relative to the
// repo's home directory instead of the module's.
func (i *Info) RepoRawURL(pathname string) string {
if i == nil {
return ""
}
// Some templates don't support raw content serving.
if i.templates.Raw == "" {
return ""
}
return expand(i.templates.Raw, map[string]string{
"repo": i.repoURL,
"commit": i.commit,
"file": pathname,
})
}
98 changes: 92 additions & 6 deletions licenses/library.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"context"
"fmt"
"go/build"
"io/ioutil"
"net/http"
"path/filepath"
"sort"
"strings"
Expand Down Expand Up @@ -225,12 +227,17 @@ func (l *Library) String() string {
return l.Name()
}

// FileURL attempts to determine the URL for a file in this library using
// go module name and version.
func (l *Library) FileURL(ctx context.Context, filePath string) (string, error) {
// testOnlySkipValidation is an internal flag to skip validation during testing,
// because we cannot easily set up actual license files on disk.
var testOnlySkipValidation = false

// LicenseURL attempts to determine the URL for the license file in this library
// using go module name and version.
func (l *Library) LicenseURL(ctx context.Context) (string, error) {
if l == nil {
return "", fmt.Errorf("library is nil")
}
filePath := l.LicensePath
wrap := func(err error) error {
return fmt.Errorf("getting file URL in library %s: %w", l.Name(), err)
}
Expand Down Expand Up @@ -272,9 +279,88 @@ func (l *Library) FileURL(ctx context.Context, filePath string) (string, error)
if err != nil {
return "", wrap(err)
}
// TODO: there are still rare cases this may result in an incorrect URL.
// https://github.com/google/go-licenses/issues/73#issuecomment-1005587408
return remote.FileURL(relativePath), nil
url := remote.FileURL(relativePath)
if testOnlySkipValidation {
return url, nil
}
// An error during validation, the URL may still be valid.
validationError := func(err error) error {
return fmt.Errorf("failed to validate %s: %w", url, err)
}
localContentBytes, err := ioutil.ReadFile(l.LicensePath)
if err != nil {
return "", validationError(err)
}
localContent := string(localContentBytes)
// Attempt 1
rawURL := remote.RawURL(relativePath)
if rawURL == "" {
glog.Warningf(
"Skipping license URL validation, because %s. Please verify whether %s matches content of %s manually!",
validationError(fmt.Errorf("remote repo %s does not support raw URL", remote)),
url,
l.LicensePath,
)
return url, nil
}
validationError1 := validate(rawURL, localContent)
if validationError1 == nil {
// The found URL is valid!
return url, nil
}
if relativePath != "LICENSE" {
return "", validationError1
}
// Attempt 2 when relativePath == "LICENSE"
// When module is at a subdir, the LICENSE file we find at root
// of the module may actually lie in the root of the repo, due
// to special go module behavior.
// Reference: https://github.com/google/go-licenses/issues/73#issuecomment-1005587408.
url2 := remote.RepoFileURL("LICENSE")
rawURL2 := remote.RepoRawURL("LICENSE")
if url2 == url {
// Return early, because the second attempt resolved to the same file.
return "", validationError1
}
// For the same remote, no need to check rawURL != "" again.
validationError2 := validate(rawURL2, localContent)
if validationError2 == nil {
return url2, nil
}
return "", fmt.Errorf("cannot infer remote URL for %s, failed attempts:\n\tattempt 1: %s\n\tattempt 2: %s", l.LicensePath, validationError1, validationError2)
}

// validate validates content of rawURL matches localContent.
func validate(rawURL string, localContent string) error {
remoteContent, err := download(rawURL)
if err != nil {
// Retry after 1 sec.
time.Sleep(time.Second)
remoteContent, err = download(rawURL)
if err != nil {
return err
}
}
if remoteContent != localContent {
return fmt.Errorf("local license file content does not match remote license URL %s", rawURL)
}
return nil
}

func download(url string) (string, error) {
resp, err := http.Get(url)
if err != nil {
return "", fmt.Errorf("download(%q): %w", url, err)
}
defer resp.Body.Close()
if resp.StatusCode >= 400 {
return "", fmt.Errorf("download(%q): response status code %v not OK", url, resp.StatusCode)
}
bodyBytes, err := ioutil.ReadAll(resp.Body)
if err != nil {
return "", fmt.Errorf("download(%q): failed to read from response body: %w", url, err)
}
return string(bodyBytes), nil
}

// isStdLib returns true if this package is part of the Go standard library.
Expand Down
20 changes: 8 additions & 12 deletions licenses/library_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,13 @@ func TestLibraryFileURL(t *testing.T) {
"github.com/google/trillian",
"github.com/google/trillian/crypto",
},
LicensePath: "/go/src/github.com/google/trillian/LICENSE",
LicensePath: "/go/src/github.com/google/trillian/foo/README.md",
module: &Module{
Path: "github.com/google/trillian",
Dir: "/go/src/github.com/google/trillian",
Version: "v1.2.3",
},
},
path: "/go/src/github.com/google/trillian/foo/README.md",
wantURL: "https://github.com/google/trillian/blob/v1.2.3/foo/README.md",
},
{
Expand All @@ -182,14 +181,13 @@ func TestLibraryFileURL(t *testing.T) {
"bitbucket.org/user/project/pkg",
"bitbucket.org/user/project/pkg2",
},
LicensePath: "/foo/bar/bitbucket.org/user/project/LICENSE",
LicensePath: "/foo/bar/bitbucket.org/user/project/foo/README.md",
module: &Module{
Path: "bitbucket.org/user/project",
Dir: "/foo/bar/bitbucket.org/user/project",
Version: "v1.2.3",
},
},
path: "/foo/bar/bitbucket.org/user/project/foo/README.md",
wantURL: "https://bitbucket.org/user/project/src/v1.2.3/foo/README.md",
},
{
Expand All @@ -199,14 +197,13 @@ func TestLibraryFileURL(t *testing.T) {
"example.com/user/project/pkg",
"example.com/user/project/pkg2",
},
LicensePath: "/foo/bar/example.com/user/project/LICENSE",
LicensePath: "/foo/bar/example.com/user/project/foo/README.md",
module: &Module{
Path: "example.com/user/project",
Dir: "/foo/bar/example.com/user/project",
Version: "v1.2.3",
},
},
path: "/foo/bar/example.com/user/project/foo/README.md",
wantURL: "https://example.com/user/project/blob/v1.2.3/foo/README.md",
},
{
Expand All @@ -216,13 +213,12 @@ func TestLibraryFileURL(t *testing.T) {
"github.com/google/trillian",
"github.com/google/trillian/crypto",
},
LicensePath: "/go/src/github.com/google/trillian/LICENSE",
LicensePath: "/go/src/github.com/google/trillian/foo/README.md",
module: &Module{
Path: "github.com/google/trillian",
Dir: "/go/src/github.com/google/trillian",
},
},
path: "/go/src/github.com/google/trillian/foo/README.md",
wantURL: "https://github.com/google/trillian/blob/HEAD/foo/README.md",
},
{
Expand All @@ -238,19 +234,19 @@ func TestLibraryFileURL(t *testing.T) {
Version: "v0.23.1",
},
},
path: "/go/modcache/k8s.io/api/LICENSE",
wantURL: "https://github.com/kubernetes/api/blob/v0.23.1/LICENSE",
},
} {
t.Run(test.desc, func(t *testing.T) {
fileURL, err := test.lib.FileURL(context.Background(), test.path)
testOnlySkipValidation = true
fileURL, err := test.lib.LicenseURL(context.Background())
if gotErr := err != nil; gotErr != test.wantErr {
t.Fatalf("FileURL(%q) = (_, %q), want err? %t", test.path, err, test.wantErr)
t.Fatalf("LicenseURL(%q) = (_, %q), want err? %t", test.path, err, test.wantErr)
} else if gotErr {
return
}
if got, want := fileURL, test.wantURL; got != want {
t.Fatalf("FileURL(%q) = %q, want %q", test.path, got, want)
t.Fatalf("LicenseURL(%q) = %q, want %q", test.path, got, want)
}
})
}
Expand Down
5 changes: 5 additions & 0 deletions testdata/modules/modinsubdir05/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module github.com/google/go-licenses/testdata/modules/modinsubdir05

go 1.15

require cloud.google.com/go/storage v1.19.0
Loading