Repository navigation
fix(import-url): validate artifact URL scheme #560
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,12 +31,8 @@ func NewImportURLCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm | |
| Use: "import-url", | ||
| Short: "import API artifacts from URL on Microcks server", | ||
| Long: `import API artifacts from URL on Microcks server`, | ||
| Args: validateImportURLArgs, | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| // Parse subcommand args first. | ||
| if len(args) == 0 { | ||
| return errors.Wrapf(errors.KindUsage, "import-url requires a <specificationFileURL[:primary[:secret]]> argument") | ||
| } | ||
|
|
||
| specificationFiles := args[0] | ||
|
|
||
| mc, _, err := newCommandClient(globalClientOpts) | ||
|
|
@@ -64,24 +60,39 @@ func NewImportURLCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm | |
| return importURLCmd | ||
| } | ||
|
|
||
| func validateImportURLArgs(cmd *cobra.Command, args []string) error { | ||
| if len(args) == 0 { | ||
| return errors.Wrapf(errors.KindUsage, "import-url requires a <specificationFileURL[:primary[:secret]]> argument") | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this way it gives to many responsibilities to this function.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I totally agree here. We can use that functionality to shift our validation much closer to the actual user input. |
||
| for _, f := range strings.Split(args[0], ",") { | ||
| lower := strings.ToLower(f) | ||
| if !strings.HasPrefix(lower, "https://") && !strings.HasPrefix(lower, "http://") { | ||
| return errors.Wrapf( | ||
| errors.KindUsage, | ||
| "invalid artifact URL '%s': must start with http:// or https://", | ||
| f, | ||
| ) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func parseImportURLArg(f string) (string, bool, string) { | ||
| mainArtifact := true | ||
| secret := "" | ||
|
|
||
| // Check if URL starts with https or http | ||
| if strings.HasPrefix(f, "https://") || strings.HasPrefix(f, "http://") { | ||
| parts := strings.Split(f, ":") | ||
| n := len(parts) | ||
| parts := strings.Split(f, ":") | ||
| n := len(parts) | ||
|
|
||
| for i := 2; i < n; i++ { | ||
| if val, parseErr := strconv.ParseBool(parts[i]); parseErr == nil { | ||
| mainArtifact = val | ||
| if i+1 < n { | ||
| secret = strings.Join(parts[i+1:], ":") | ||
| } | ||
| f = strings.Join(parts[:i], ":") | ||
| break | ||
| for i := 2; i < n; i++ { | ||
| if val, parseErr := strconv.ParseBool(parts[i]); parseErr == nil { | ||
| mainArtifact = val | ||
| if i+1 < n { | ||
| secret = strings.Join(parts[i+1:], ":") | ||
| } | ||
| f = strings.Join(parts[:i], ":") | ||
| break | ||
| } | ||
| } | ||
| return f, mainArtifact, secret | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thing before merge:
validateImportURLArgsonly rejectslen(args) == 0. Runningimport-url url1 url2silently dropsurl2. Please return a usage error forlen(args) > 1that points to the comma-separated form.