fix import not ignoring url path #3622
Conversation
|
Can I find the URL of image tarball to add test for this? |
|
Can you do |
|
No runtime_img.go |
cmd/podman/shared/parse/parse.go
Outdated
| // ValidURL checks a string urlStr is a url or not | ||
| func ValidURL(urlStr string) error { | ||
| _, err := url.ParseRequestURI(urlStr) | ||
| return errors.Wrapf(err, "invalid url path: %q", urlStr) |
There was a problem hiding this comment.
it seems to me that doing this wrapping mean ValidURL always returns error, regardless of if the url is valid. is that the case, or does passing a nil err to wrapf return nil?
There was a problem hiding this comment.
yes, I forget to change when I wrap the error
There was a problem hiding this comment.
but yes passing a nil err to wrapf return nil
|
LGTM |
|
/approve |
|
Code LGTM |
cmd/podman/import.go
Outdated
| errURL := parse.ValidURL(source) | ||
|
|
||
| if errFileName != nil && errURL != nil { | ||
| return errors.Errorf("%q, %q invalid tarball path", errFileName, errURL) |
There was a problem hiding this comment.
We could use a multierror here, maybe?
There was a problem hiding this comment.
12f055b to
87e4423
Compare
|
@QiWang19 Not sure what these test failures are, but I don't think they're you - mind rebasing+repushing? |
fix containers#3609 Podman import used to check filename to only allow tarball path as a file. It should also allow an url as the doc mentioned. This PR allows the program to continue if the input is a valid URL Signed-off-by: Qi Wang <qiwan@redhat.com>
|
@mheon PTAL |
|
LGTM |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, mheon, QiWang19 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
fix #3609
This PR allows the program to continue if the input is a valid URL.
Podman import used to check filename to only allow tarball path as a file. It should also allow an URL as the doc mentioned.
Signed-off-by: Qi Wang qiwan@redhat.com