Skip to content

Commit 2808636

Browse files
AkihiroSudaAkihiroSuda
committed
dockerfile2llb: check GitURLOpts error
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 5a20f2e commit 2808636

File tree

2 files changed

+105
-74
lines changed

2 files changed

+105
-74
lines changed

‎frontend/dockerfile/dockerfile2llb/convert.go‎

Lines changed: 93 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,7 +1464,15 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
14641464
if len(cfg.params.SourcePaths) != 1 {
14651465
return errors.New("checksum can't be specified for multiple sources")
14661466
}
1467-
if !isHTTPSource(cfg.params.SourcePaths[0]) && !isGitSource(cfg.params.SourcePaths[0]) {
1467+
isHTTPSourceV, err := isHTTPSource(cfg.params.SourcePaths[0])
1468+
if err != nil {
1469+
return err
1470+
}
1471+
isGitSourceV, err := isGitSource(cfg.params.SourcePaths[0])
1472+
if err != nil {
1473+
return err
1474+
}
1475+
if !isHTTPSourceV && !isGitSourceV {
14681476
return errors.New("checksum requires HTTP(S) or Git sources")
14691477
}
14701478
}
@@ -1529,85 +1537,91 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
15291537
} else {
15301538
a = a.Copy(st, "/", dest, opts...)
15311539
}
1532-
} else if isHTTPSource(src) {
1533-
if !cfg.isAddCommand {
1534-
return errors.New("source can't be a URL for COPY")
1540+
} else {
1541+
isHTTPSourceV, err := isHTTPSource(src)
1542+
if err != nil {
1543+
return err
15351544
}
1545+
if isHTTPSourceV {
1546+
if !cfg.isAddCommand {
1547+
return errors.New("source can't be a URL for COPY")
1548+
}
15361549

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
1550+
// Resources from remote URLs are not decompressed.
1551+
// https://docs.docker.com/engine/reference/builder/#add
1552+
//
1553+
// Note: mixing up remote archives and local archives in a single ADD instruction
1554+
// would result in undefined behavior: https://github.com/moby/buildkit/pull/387#discussion_r189494717
1555+
u, err := url.Parse(src)
1556+
f := "__unnamed__"
1557+
if err == nil {
1558+
if base := path.Base(u.Path); base != "." && base != "/" {
1559+
f = base
1560+
}
15471561
}
1548-
}
15491562

1550-
var checksum digest.Digest
1551-
if cfg.checksum != "" {
1552-
checksum, err = digest.Parse(cfg.checksum)
1553-
if err != nil {
1554-
return err
1563+
var checksum digest.Digest
1564+
if cfg.checksum != "" {
1565+
checksum, err = digest.Parse(cfg.checksum)
1566+
if err != nil {
1567+
return err
1568+
}
15551569
}
1556-
}
15571570

1558-
st := llb.HTTP(src, llb.Filename(f), llb.WithCustomName(pgName), llb.Checksum(checksum), dfCmd(cfg.params))
1571+
st := llb.HTTP(src, llb.Filename(f), llb.WithCustomName(pgName), llb.Checksum(checksum), dfCmd(cfg.params))
15591572

1560-
opts := append([]llb.CopyOption{&llb.CopyInfo{
1561-
Mode: chopt,
1562-
CreateDestPath: true,
1563-
}}, copyOpt...)
1573+
opts := append([]llb.CopyOption{&llb.CopyInfo{
1574+
Mode: chopt,
1575+
CreateDestPath: true,
1576+
}}, copyOpt...)
15641577

1565-
if a == nil {
1566-
a = llb.Copy(st, f, dest, opts...)
1567-
} else {
1568-
a = a.Copy(st, f, dest, opts...)
1569-
}
1570-
} else {
1571-
validateCopySourcePath(src, &cfg)
1572-
var patterns []string
1573-
if cfg.parents {
1574-
// detect optional pivot point
1575-
parent, pattern, ok := strings.Cut(src, "/./")
1576-
if !ok {
1577-
pattern = src
1578-
src = "/"
1578+
if a == nil {
1579+
a = llb.Copy(st, f, dest, opts...)
15791580
} else {
1580-
src = parent
1581+
a = a.Copy(st, f, dest, opts...)
1582+
}
1583+
} else {
1584+
validateCopySourcePath(src, &cfg)
1585+
var patterns []string
1586+
if cfg.parents {
1587+
// detect optional pivot point
1588+
parent, pattern, ok := strings.Cut(src, "/./")
1589+
if !ok {
1590+
pattern = src
1591+
src = "/"
1592+
} else {
1593+
src = parent
1594+
}
1595+
1596+
pattern, err = system.NormalizePath("/", pattern, d.platform.OS, false)
1597+
if err != nil {
1598+
return errors.Wrap(err, "removing drive letter")
1599+
}
1600+
1601+
patterns = []string{strings.TrimPrefix(pattern, "/")}
15811602
}
15821603

1583-
pattern, err = system.NormalizePath("/", pattern, d.platform.OS, false)
1604+
src, err = system.NormalizePath("/", src, d.platform.OS, false)
15841605
if err != nil {
15851606
return errors.Wrap(err, "removing drive letter")
15861607
}
15871608

1588-
patterns = []string{strings.TrimPrefix(pattern, "/")}
1589-
}
1590-
1591-
src, err = system.NormalizePath("/", src, d.platform.OS, false)
1592-
if err != nil {
1593-
return errors.Wrap(err, "removing drive letter")
1594-
}
1595-
1596-
opts := append([]llb.CopyOption{&llb.CopyInfo{
1597-
Mode: chopt,
1598-
FollowSymlinks: true,
1599-
CopyDirContentsOnly: true,
1600-
IncludePatterns: patterns,
1601-
AttemptUnpack: cfg.isAddCommand,
1602-
CreateDestPath: true,
1603-
AllowWildcard: true,
1604-
AllowEmptyWildcard: true,
1605-
}}, copyOpt...)
1606-
1607-
if a == nil {
1608-
a = llb.Copy(cfg.source, src, dest, opts...)
1609-
} else {
1610-
a = a.Copy(cfg.source, src, dest, opts...)
1609+
opts := append([]llb.CopyOption{&llb.CopyInfo{
1610+
Mode: chopt,
1611+
FollowSymlinks: true,
1612+
CopyDirContentsOnly: true,
1613+
IncludePatterns: patterns,
1614+
AttemptUnpack: cfg.isAddCommand,
1615+
CreateDestPath: true,
1616+
AllowWildcard: true,
1617+
AllowEmptyWildcard: true,
1618+
}}, copyOpt...)
1619+
1620+
if a == nil {
1621+
a = llb.Copy(cfg.source, src, dest, opts...)
1622+
} else {
1623+
a = a.Copy(cfg.source, src, dest, opts...)
1624+
}
16111625
}
16121626
}
16131627
}
@@ -2269,19 +2283,26 @@ func commonImageNames() []string {
22692283
return out
22702284
}
22712285

2272-
func isHTTPSource(src string) bool {
2286+
func isHTTPSource(src string) (bool, error) {
22732287
if !strings.HasPrefix(src, "http://") && !strings.HasPrefix(src, "https://") {
2274-
return false
2288+
return false, nil
22752289
}
2276-
return !isGitSource(src)
2290+
b, err := isGitSource(src)
2291+
return !b, err
22772292
}
22782293

2279-
func isGitSource(src string) bool {
2294+
func isGitSource(src string) (bool, error) {
22802295
// https://github.com/ORG/REPO.git is a git source, not an http source
2281-
if gitRef, gitErr := gitutil.ParseGitRef(src); gitRef != nil && gitErr == nil {
2282-
return true
2296+
gitRef, gitErr := gitutil.ParseGitRef(src)
2297+
var guoe *gitutil.GitURLOptsError
2298+
if errors.As(gitErr, &guoe) {
2299+
// this is a git source, and it has invalid URL opts
2300+
return true, gitErr
2301+
}
2302+
if gitRef != nil && gitErr == nil {
2303+
return true, nil
22832304
}
2284-
return false
2305+
return false, nil
22852306
}
22862307

22872308
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
@@ -66,6 +66,11 @@ type GitURLOpts struct {
6666
Subdir string
6767
}
6868

69+
// GitURLOptsError is returned for invalid GitURLOpts.
70+
type GitURLOptsError struct {
71+
error
72+
}
73+
6974
// parseOpts splits a git URL fragment into its respective git
7075
// reference and subdirectory components.
7176
func parseOpts(fragment string, query url.Values) (*GitURLOpts, error) {
@@ -169,7 +174,9 @@ func fromURL(url *url.URL) (*GitURL, error) {
169174
withoutOpts.RawQuery = ""
170175
opts, err := parseOpts(url.Fragment, url.Query())
171176
if err != nil {
172-
return nil, err
177+
return nil, &GitURLOptsError{
178+
error: errors.Wrapf(err, "failed to parse git URL opts %q", url.Redacted()),
179+
}
173180
}
174181
return &GitURL{
175182
Scheme: url.Scheme,
@@ -187,7 +194,10 @@ func fromSCPStyleURL(url *sshutil.SCPStyleURL) (*GitURL, error) {
187194
withoutOpts.Query = nil
188195
opts, err := parseOpts(url.Fragment, url.Query)
189196
if err != nil {
190-
return nil, err
197+
return nil, &GitURLOptsError{
198+
// *sshutil.SCPStyleURL.String() does not contain password
199+
error: errors.Wrapf(err, "failed to parse git URL opts %q", url.String()),
200+
}
191201
}
192202
return &GitURL{
193203
Scheme: SSHProtocol,

0 commit comments

Comments
 (0)