Repository navigation
fix(import-url): validate artifact URL scheme - #560
dipak0000812 wants to merge 2 commits into
Conversation
Signed-off-by: Dipak Dhangar <dhangardip09@gmail.com>
|
Welcome to the Microcks community! 💖 Thanks and congrats 🎉 for opening your first pull request here! Be sure to follow the pull request template or please update it accordingly. Hope you have a great time there! |
|
LGTM, this matches what I proposed in #534. Thanks for picking it up, @dipak0000812 |
| f, | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
I think this way it gives to many responsibilities to this function.
Cobra offer pre-hook Args which is the right place for this kind of validation
this is also valid for the if len(arg) == 0 check
There was a problem hiding this comment.
Yes, I totally agree here. We can use that functionality to shift our validation much closer to the actual user input.
Signed-off-by: Dipak Dhangar <dhangardip09@gmail.com>
|
Thanks for the feedback. I've updated the implementation:
The targeted tests, I've pushed the changes in |
Description
Fixes #534
When
import-urlreceives an artifact argument without anhttp://orhttps://prefix,parseImportURLArgpreviously returned the input unchanged and it was passed tomc.DownloadArtifact, resulting in a misleading server or connection error.This change:
parseImportURLArg.errors.KindUsageerror when the input does not start withhttp://orhttps://.import-url'sRunE.ftp://,file:///), empty input, and the reported issue syntax.Testing
go test -v ./cmd -run "TestImportURL|TestParseImportURLArg"go test ./...go vet ./...git diff --checkAI Assistance Disclosure
AI assistance was used minimally during development for codebase/policy lookup and review. The implementation, tests, and final verification were performed and reviewed by the author.