Skip to content

devpkg: better handling of escapes in flakeref paths #1598

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 1, 2023
Merged

devpkg: better handling of escapes in flakeref paths #1598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
deea2f7
devpkg: use struct field values in FlakeRef.String
gcurtis Oct 30, 2023
2658832
devpkg: better handling of escapes in flakeref paths
gcurtis Oct 30, 2023
2cbf32b
Merge branch 'main' of https://github.com/jetpack-io/devbox into gcur…
gcurtis Nov 1, 2023
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
79 changes: 50 additions & 29 deletions internal/devpkg/flakeref.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,10 @@ func parseFlakeURLRef(ref string) (parsed FlakeRef, fragment string, err error)
// [flake:]<flake-id>(/<rev-or-ref>(/rev)?)?

parsed.Type = "indirect"

// "indirect" is parsed as a path, "flake:indirect" is parsed as
// opaque because it has a scheme.
path := refURL.Path
if path == "" {
path, err = url.PathUnescape(refURL.Opaque)
if err != nil {
path = refURL.Opaque
}
split, err := splitPathOrOpaque(refURL)
if err != nil {
return FlakeRef{}, "", redact.Errorf("parse flake reference URL path: %v", err)
}
split := strings.SplitN(path, "/", 3)
parsed.ID = split[0]
if len(split) > 1 {
if isGitHash(split[1]) {
Expand All @@ -136,7 +129,7 @@ func parseFlakeURLRef(ref string) (parsed FlakeRef, fragment string, err error)
if refURL.Path == "" {
parsed.Path, err = url.PathUnescape(refURL.Opaque)
if err != nil {
parsed.Path = refURL.Opaque
return FlakeRef{}, "", err
}
} else {
parsed.Path = refURL.Path
Expand All @@ -147,16 +140,20 @@ func parseFlakeURLRef(ref string) (parsed FlakeRef, fragment string, err error)
} else {
parsed.Type = "file"
}
parsed.URL = ref
parsed.Dir = refURL.Query().Get("dir")
parsed.URL = refURL.String()
case "tarball+http", "tarball+https", "tarball+file":
parsed.Type = "tarball"
parsed.Dir = refURL.Query().Get("dir")
parsed.URL = ref[8:] // remove tarball+

refURL.Scheme = refURL.Scheme[8:] // remove tarball+
parsed.URL = refURL.String()
case "file+http", "file+https", "file+file":
parsed.Type = "file"
parsed.Dir = refURL.Query().Get("dir")
parsed.URL = ref[5:] // remove file+

refURL.Scheme = refURL.Scheme[5:] // remove file+
parsed.URL = refURL.String()
case "git", "git+http", "git+https", "git+ssh", "git+git", "git+file":
parsed.Type = "git"
q := refURL.Query()
Expand Down Expand Up @@ -185,17 +182,10 @@ func parseGitHubFlakeRef(refURL *url.URL, parsed *FlakeRef) error {
// github:<owner>/<repo>(/<rev-or-ref>)?(\?<params>)?

parsed.Type = "github"
path := refURL.Path
if path == "" {
var err error
path, err = url.PathUnescape(refURL.Opaque)
if err != nil {
path = refURL.Opaque
}
split, err := splitPathOrOpaque(refURL)
if err != nil {
return err
}
path = strings.TrimPrefix(path, "/")

split := strings.SplitN(path, "/", 3)
parsed.Owner = split[0]
parsed.Repo = split[1]
if len(split) > 2 {
Expand Down Expand Up @@ -280,7 +270,7 @@ func (f FlakeRef) String() string {
}
url := &url.URL{
Scheme: "github",
Opaque: buildOpaquePath(f.Owner, f.Repo, f.Rev, f.Ref),
Opaque: buildEscapedPath(f.Owner, f.Repo, f.Rev, f.Ref),
RawQuery: buildQueryString("host", f.Host, "dir", f.Dir),
}
return url.String()
Expand All @@ -290,7 +280,7 @@ func (f FlakeRef) String() string {
}
url := &url.URL{
Scheme: "flake",
Opaque: buildOpaquePath(f.ID, f.Ref, f.Rev),
Opaque: buildEscapedPath(f.ID, f.Ref, f.Rev),
RawQuery: buildQueryString("dir", f.Dir),
}
return url.String()
Expand All @@ -301,7 +291,7 @@ func (f FlakeRef) String() string {
f.Path = path.Clean(f.Path)
url := &url.URL{
Scheme: "path",
Opaque: buildOpaquePath(strings.Split(f.Path, "/")...),
Opaque: buildEscapedPath(strings.Split(f.Path, "/")...),
}

// Add the / prefix back if strings.Split removed it.
Expand Down Expand Up @@ -359,9 +349,40 @@ func isArchive(path string) bool {
strings.HasSuffix(path, ".zip")
}

// buildOpaquePath escapes and joins path elements for a URL flakeref. The
// splitPathOrOpaque splits a URL path by '/'. If the path is empty, it splits
// the opaque instead. Splitting happens before unescaping the path or opaque,
// ensuring that path elements with an encoded '/' (%2F) are not split.
// For example, "/dir/file%2Fname" becomes the elements "dir" and "file/name".
func splitPathOrOpaque(u *url.URL) ([]string, error) {
upath := u.EscapedPath()
if upath == "" {
upath = u.Opaque
}
upath = strings.TrimSpace(upath)
if upath == "" {
return nil, nil
}

// We don't want an empty element if the path is rooted.
if upath[0] == '/' {
upath = upath[1:]
}
upath = path.Clean(upath)

var err error
split := strings.Split(upath, "/")
for i := range split {
split[i], err = url.PathUnescape(split[i])
if err != nil {
return nil, err
}
}
return split, nil
}

// buildEscapedPath escapes and joins path elements for a URL flakeref. The
// resulting path is cleaned according to url.JoinPath.
func buildOpaquePath(elem ...string) string {
func buildEscapedPath(elem ...string) string {
for i := range elem {
elem[i] = url.PathEscape(elem[i])
}
Expand Down
10 changes: 4 additions & 6 deletions internal/devpkg/flakeref_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ func TestParseFlakeRef(t *testing.T) {
// Not a path and not a valid URL.
"://bad/url": {},

// Invalid escape.
"path:./relative/my%flake": {},

// Path-like references start with a '.' or '/'.
// This distinguishes them from indirect references
// (./nixpkgs is a directory; nixpkgs is an indirect).
Expand Down Expand Up @@ -60,7 +63,7 @@ func TestParseFlakeRef(t *testing.T) {
// Indirect references.
"flake:indirect": {Type: "indirect", ID: "indirect"},
"flake:indirect/ref": {Type: "indirect", ID: "indirect", Ref: "ref"},
"flake:indirect/my%20ref": {Type: "indirect", ID: "indirect", Ref: "my ref"},
"flake:indirect/my%2Fref": {Type: "indirect", ID: "indirect", Ref: "my/ref"},
"flake:indirect/5233fd2ba76a3accb5aaa999c00509a11fd0793c": {Type: "indirect", ID: "indirect", Rev: "5233fd2ba76a3accb5aaa999c00509a11fd0793c"},
"flake:indirect/ref/5233fd2ba76a3accb5aaa999c00509a11fd0793c": {Type: "indirect", ID: "indirect", Ref: "ref", Rev: "5233fd2ba76a3accb5aaa999c00509a11fd0793c"},

Expand Down Expand Up @@ -139,11 +142,6 @@ func TestParseFlakeRef(t *testing.T) {
"http://example.com/flake": {Type: "file", URL: "http://example.com/flake"},
"http://example.com/flake.git": {Type: "file", URL: "http://example.com/flake.git"},
"http://example.com/flake?dir=subdir": {Type: "file", URL: "http://example.com/flake?dir=subdir", Dir: "subdir"},

// Interpret opaques with invalid escapes literally.
"path:./relative/my%flake": {Type: "path", Path: "./relative/my%flake"},
"flake:indirect/my%ref": {Type: "indirect", ID: "indirect", Ref: "my%ref"},
"github:NixOS/nix/my%ref": {Type: "github", Owner: "NixOS", Repo: "nix", Ref: "my%ref"},
}

for ref, want := range cases {
Expand Down