Skip to content

Commit 921c77c

Browse files
committed
dockerfile2llb: check ErrInvalidGitURLOpts
Prior to this commit, `ADD https://github.com/moby/example.git?invalid=42` was fetching a single HTML file without causing an error. Now it returns an error `unexpected query "invalid"`. Hint: use `git show --ignore-all-space` to review this commit. Signed-off-by: Akihiro Suda <[email protected]>
1 parent 9c50ca8 commit 921c77c

File tree

2 files changed

+92
-66
lines changed

2 files changed

+92
-66
lines changed

frontend/dockerfile/dockerfile2llb/convert.go

Lines changed: 80 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1470,7 +1470,11 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
14701470
if len(cfg.params.SourcePaths) != 1 {
14711471
return errors.New("checksum can't be specified for multiple sources")
14721472
}
1473-
if !isHTTPSource(cfg.params.SourcePaths[0]) {
1473+
ok, err := isHTTPSource(cfg.params.SourcePaths[0])
1474+
if err != nil {
1475+
return err
1476+
}
1477+
if !ok {
14741478
return errors.New("checksum can't be specified for non-HTTP(S) sources")
14751479
}
14761480
}
@@ -1529,77 +1533,83 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
15291533
} else {
15301534
a = a.Copy(st, "/", dest, opts...)
15311535
}
1532-
} else if isHTTPSource(src) {
1533-
if !cfg.isAddCommand {
1534-
return errors.New("source can't be a URL for COPY")
1536+
} else {
1537+
isHTTPSourceV, err := isHTTPSource(src)
1538+
if err != nil {
1539+
return err
15351540
}
1541+
if isHTTPSourceV {
1542+
if !cfg.isAddCommand {
1543+
return errors.New("source can't be a URL for COPY")
1544+
}
15361545

1537-
// Resources from remote URLs are not decompressed.
1538-
// https://docs.docker.com/engine/reference/builder/#add
1539-
//
1540-
// Note: mixing up remote archives and local archives in a single ADD instruction
1541-
// would result in undefined behavior: https://github.com/moby/buildkit/pull/387#discussion_r189494717
1542-
u, err := url.Parse(src)
1543-
f := "__unnamed__"
1544-
if err == nil {
1545-
if base := path.Base(u.Path); base != "." && base != "/" {
1546-
f = base
1546+
// Resources from remote URLs are not decompressed.
1547+
// https://docs.docker.com/engine/reference/builder/#add
1548+
//
1549+
// Note: mixing up remote archives and local archives in a single ADD instruction
1550+
// would result in undefined behavior: https://github.com/moby/buildkit/pull/387#discussion_r189494717
1551+
u, err := url.Parse(src)
1552+
f := "__unnamed__"
1553+
if err == nil {
1554+
if base := path.Base(u.Path); base != "." && base != "/" {
1555+
f = base
1556+
}
15471557
}
1548-
}
15491558

1550-
st := llb.HTTP(src, llb.Filename(f), llb.WithCustomName(pgName), llb.Checksum(cfg.checksum), dfCmd(cfg.params))
1559+
st := llb.HTTP(src, llb.Filename(f), llb.WithCustomName(pgName), llb.Checksum(cfg.checksum), dfCmd(cfg.params))
15511560

1552-
opts := append([]llb.CopyOption{&llb.CopyInfo{
1553-
Mode: chopt,
1554-
CreateDestPath: true,
1555-
}}, copyOpt...)
1561+
opts := append([]llb.CopyOption{&llb.CopyInfo{
1562+
Mode: chopt,
1563+
CreateDestPath: true,
1564+
}}, copyOpt...)
15561565

1557-
if a == nil {
1558-
a = llb.Copy(st, f, dest, opts...)
1559-
} else {
1560-
a = a.Copy(st, f, dest, opts...)
1561-
}
1562-
} else {
1563-
validateCopySourcePath(src, &cfg)
1564-
var patterns []string
1565-
if cfg.parents {
1566-
// detect optional pivot point
1567-
parent, pattern, ok := strings.Cut(src, "/./")
1568-
if !ok {
1569-
pattern = src
1570-
src = "/"
1566+
if a == nil {
1567+
a = llb.Copy(st, f, dest, opts...)
15711568
} else {
1572-
src = parent
1569+
a = a.Copy(st, f, dest, opts...)
15731570
}
1571+
} else {
1572+
validateCopySourcePath(src, &cfg)
1573+
var patterns []string
1574+
if cfg.parents {
1575+
// detect optional pivot point
1576+
parent, pattern, ok := strings.Cut(src, "/./")
1577+
if !ok {
1578+
pattern = src
1579+
src = "/"
1580+
} else {
1581+
src = parent
1582+
}
1583+
1584+
pattern, err = system.NormalizePath("/", pattern, d.platform.OS, false)
1585+
if err != nil {
1586+
return errors.Wrap(err, "removing drive letter")
1587+
}
15741588

1575-
pattern, err = system.NormalizePath("/", pattern, d.platform.OS, false)
1589+
patterns = []string{strings.TrimPrefix(pattern, "/")}
1590+
}
1591+
1592+
src, err = system.NormalizePath("/", src, d.platform.OS, false)
15761593
if err != nil {
15771594
return errors.Wrap(err, "removing drive letter")
15781595
}
15791596

1580-
patterns = []string{strings.TrimPrefix(pattern, "/")}
1581-
}
1582-
1583-
src, err = system.NormalizePath("/", src, d.platform.OS, false)
1584-
if err != nil {
1585-
return errors.Wrap(err, "removing drive letter")
1586-
}
1587-
1588-
opts := append([]llb.CopyOption{&llb.CopyInfo{
1589-
Mode: chopt,
1590-
FollowSymlinks: true,
1591-
CopyDirContentsOnly: true,
1592-
IncludePatterns: patterns,
1593-
AttemptUnpack: cfg.isAddCommand,
1594-
CreateDestPath: true,
1595-
AllowWildcard: true,
1596-
AllowEmptyWildcard: true,
1597-
}}, copyOpt...)
1598-
1599-
if a == nil {
1600-
a = llb.Copy(cfg.source, src, dest, opts...)
1601-
} else {
1602-
a = a.Copy(cfg.source, src, dest, opts...)
1597+
opts := append([]llb.CopyOption{&llb.CopyInfo{
1598+
Mode: chopt,
1599+
FollowSymlinks: true,
1600+
CopyDirContentsOnly: true,
1601+
IncludePatterns: patterns,
1602+
AttemptUnpack: cfg.isAddCommand,
1603+
CreateDestPath: true,
1604+
AllowWildcard: true,
1605+
AllowEmptyWildcard: true,
1606+
}}, copyOpt...)
1607+
1608+
if a == nil {
1609+
a = llb.Copy(cfg.source, src, dest, opts...)
1610+
} else {
1611+
a = a.Copy(cfg.source, src, dest, opts...)
1612+
}
16031613
}
16041614
}
16051615
}
@@ -2261,15 +2271,21 @@ func commonImageNames() []string {
22612271
return out
22622272
}
22632273

2264-
func isHTTPSource(src string) bool {
2274+
func isHTTPSource(src string) (bool, error) {
22652275
if !strings.HasPrefix(src, "http://") && !strings.HasPrefix(src, "https://") {
2266-
return false
2276+
return false, nil
22672277
}
22682278
// https://github.com/ORG/REPO.git is a git source, not an http source
2269-
if gitRef, gitErr := gitutil.ParseGitRef(src); gitRef != nil && gitErr == nil {
2270-
return false
2279+
gitRef, gitErr := gitutil.ParseGitRef(src)
2280+
var eiuf *gitutil.ErrInvalidGitURLOpts
2281+
if errors.As(gitErr, &eiuf) {
2282+
// this is a git source, and it has invalid URL opts
2283+
return false, gitErr
2284+
}
2285+
if gitRef != nil && gitErr == nil {
2286+
return false, nil
22712287
}
2272-
return true
2288+
return true, nil
22732289
}
22742290

22752291
func isEnabledForStage(stage string, value string) bool {

util/gitutil/git_url.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ type GitURLOpts struct {
6464
Subdir string
6565
}
6666

67+
// ErrInvalidGitURLOpts is returned for invalid GitURLOpts
68+
type ErrInvalidGitURLOpts struct {
69+
error
70+
}
71+
6772
// parseOpts splits a git URL fragment into its respective git
6873
// reference and subdirectory components.
6974
func parseOpts(fragment string, query url.Values) (*GitURLOpts, error) {
@@ -162,7 +167,9 @@ func fromURL(url *url.URL) (*GitURL, error) {
162167
withoutOpts.RawQuery = ""
163168
opts, err := parseOpts(url.Fragment, url.Query())
164169
if err != nil {
165-
return nil, err
170+
return nil, &ErrInvalidGitURLOpts{
171+
error: errors.Wrapf(err, "failed to parse git URL opts %q", url.Redacted()),
172+
}
166173
}
167174
return &GitURL{
168175
Scheme: url.Scheme,
@@ -180,7 +187,10 @@ func fromSCPStyleURL(url *sshutil.SCPStyleURL) (*GitURL, error) {
180187
withoutOpts.Query = nil
181188
opts, err := parseOpts(url.Fragment, url.Query)
182189
if err != nil {
183-
return nil, err
190+
return nil, &ErrInvalidGitURLOpts{
191+
// *sshutil.SCPStyleURL.String() does not contain password
192+
error: errors.Wrapf(err, "failed to parse git URL opts %q", url.String()),
193+
}
184194
}
185195
return &GitURL{
186196
Scheme: SSHProtocol,

0 commit comments

Comments
 (0)