From 98c33f01a38ac5cd6036318a977fe274f49d2a8d Mon Sep 17 00:00:00 2001 From: Saquib Mian Date: Wed, 21 Feb 2024 10:45:37 -0500 Subject: [PATCH 1/7] Prevent pushing with empty Label/Tag names --- private/buf/cmd/buf/command/push/push.go | 53 +++++++++++++++++++----- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/private/buf/cmd/buf/command/push/push.go b/private/buf/cmd/buf/command/push/push.go index 8164b69c26..dbb3afa0eb 100644 --- a/private/buf/cmd/buf/command/push/push.go +++ b/private/buf/cmd/buf/command/push/push.go @@ -140,7 +140,7 @@ func run( container appext.Container, flags *flags, ) (retErr error) { - if err := validateCreateFlags(flags); err != nil { + if err := validateFlags(flags); err != nil { return err } @@ -346,6 +346,37 @@ func createUploadModulesIfNotExist( return nil } +func validateFlags(flags *flags) error { + if err := validateCreateFlags(flags); err != nil { + return err + } + if err := validateLabelFlags(flags); err != nil { + return err + } + return nil +} + +func validateLabelFlags(flags *flags) error { + for _, label := range flags.Labels { + if label == "" { + return appcmd.NewInvalidArgumentErrorf( + "--%s requires a non-empty string.", + labelFlagName, + ) + } + } + for _, tag := range flags.Tags { + if tag == "" { + return appcmd.NewInvalidArgumentErrorf( + "%s|--%s requires a non-empty string.", + tagFlagShortName, + tagFlagName, + ) + } + } + return nil +} + func validateCreateFlags(flags *flags) error { if flags.Create { if flags.CreateVisibility == "" { @@ -371,16 +402,16 @@ func validateCreateFlags(flags *flags) error { } func combineLabelLikeFlags(flags *flags) []string { - return slicesext.ToUniqueSorted( - append( - flags.Labels, - append( - flags.Tags, - flags.Draft, - flags.Branch, - )..., - ), - ) + labels := make([]string, 0, len(flags.Labels)+len(flags.Tags)) + labels = append(labels, flags.Labels...) + labels = append(labels, flags.Tags...) + if flags.Draft != "" { + labels = append(labels, flags.Draft) + } + if flags.Branch != "" { + labels = append(labels, flags.Branch) + } + return slicesext.ToUniqueSorted(labels) } func newRequireModuleFullNameOnUploadError(module bufmodule.Module) error { From a277c4ed3ed828b6526e6b362e4213ce9f052d9b Mon Sep 17 00:00:00 2001 From: Saquib Mian Date: Wed, 21 Feb 2024 11:37:29 -0500 Subject: [PATCH 2/7] Fix short flag name --- private/buf/cmd/buf/command/push/push.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/private/buf/cmd/buf/command/push/push.go b/private/buf/cmd/buf/command/push/push.go index dbb3afa0eb..55d9d3aae5 100644 --- a/private/buf/cmd/buf/command/push/push.go +++ b/private/buf/cmd/buf/command/push/push.go @@ -368,7 +368,7 @@ func validateLabelFlags(flags *flags) error { for _, tag := range flags.Tags { if tag == "" { return appcmd.NewInvalidArgumentErrorf( - "%s|--%s requires a non-empty string.", + "-%s|--%s requires a non-empty string.", tagFlagShortName, tagFlagName, ) From d847d5df10b68b25e225406757d071f1a40e9d12 Mon Sep 17 00:00:00 2001 From: Saquib Mian Date: Wed, 21 Feb 2024 16:12:05 -0500 Subject: [PATCH 3/7] Don't print short flag names on invalid argument error --- private/buf/cmd/buf/command/push/push.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/private/buf/cmd/buf/command/push/push.go b/private/buf/cmd/buf/command/push/push.go index 55d9d3aae5..3ad3f32da8 100644 --- a/private/buf/cmd/buf/command/push/push.go +++ b/private/buf/cmd/buf/command/push/push.go @@ -368,8 +368,7 @@ func validateLabelFlags(flags *flags) error { for _, tag := range flags.Tags { if tag == "" { return appcmd.NewInvalidArgumentErrorf( - "-%s|--%s requires a non-empty string.", - tagFlagShortName, + "--%s requires a non-empty string.", tagFlagName, ) } From 6e23adf14cd718f6ed08b4d183205056be0d7a54 Mon Sep 17 00:00:00 2001 From: Saquib Mian Date: Wed, 21 Feb 2024 16:12:58 -0500 Subject: [PATCH 4/7] Move validateLabelFlags down --- private/buf/cmd/buf/command/push/push.go | 40 ++++++++++++------------ 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/private/buf/cmd/buf/command/push/push.go b/private/buf/cmd/buf/command/push/push.go index 3ad3f32da8..6c00d883fb 100644 --- a/private/buf/cmd/buf/command/push/push.go +++ b/private/buf/cmd/buf/command/push/push.go @@ -356,26 +356,6 @@ func validateFlags(flags *flags) error { return nil } -func validateLabelFlags(flags *flags) error { - for _, label := range flags.Labels { - if label == "" { - return appcmd.NewInvalidArgumentErrorf( - "--%s requires a non-empty string.", - labelFlagName, - ) - } - } - for _, tag := range flags.Tags { - if tag == "" { - return appcmd.NewInvalidArgumentErrorf( - "--%s requires a non-empty string.", - tagFlagName, - ) - } - } - return nil -} - func validateCreateFlags(flags *flags) error { if flags.Create { if flags.CreateVisibility == "" { @@ -400,6 +380,26 @@ func validateCreateFlags(flags *flags) error { return nil } +func validateLabelFlags(flags *flags) error { + for _, label := range flags.Labels { + if label == "" { + return appcmd.NewInvalidArgumentErrorf( + "--%s requires a non-empty string.", + labelFlagName, + ) + } + } + for _, tag := range flags.Tags { + if tag == "" { + return appcmd.NewInvalidArgumentErrorf( + "--%s requires a non-empty string.", + tagFlagName, + ) + } + } + return nil +} + func combineLabelLikeFlags(flags *flags) []string { labels := make([]string, 0, len(flags.Labels)+len(flags.Tags)) labels = append(labels, flags.Labels...) From 83e55d782f0b94d0dba5893c009be57d2a1a2356 Mon Sep 17 00:00:00 2001 From: Saquib Mian Date: Wed, 21 Feb 2024 16:13:52 -0500 Subject: [PATCH 5/7] Remove indenting --- private/buf/cmd/buf/command/push/push.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/private/buf/cmd/buf/command/push/push.go b/private/buf/cmd/buf/command/push/push.go index 6c00d883fb..33206becf5 100644 --- a/private/buf/cmd/buf/command/push/push.go +++ b/private/buf/cmd/buf/command/push/push.go @@ -383,18 +383,12 @@ func validateCreateFlags(flags *flags) error { func validateLabelFlags(flags *flags) error { for _, label := range flags.Labels { if label == "" { - return appcmd.NewInvalidArgumentErrorf( - "--%s requires a non-empty string.", - labelFlagName, - ) + return appcmd.NewInvalidArgumentErrorf("--%s requires a non-empty string.", labelFlagName) } } for _, tag := range flags.Tags { if tag == "" { - return appcmd.NewInvalidArgumentErrorf( - "--%s requires a non-empty string.", - tagFlagName, - ) + return appcmd.NewInvalidArgumentErrorf("--%s requires a non-empty string.", tagFlagName) } } return nil From cae081c13ddf795f3bae94943e8c450d0333f989 Mon Sep 17 00:00:00 2001 From: Saquib Mian Date: Wed, 21 Feb 2024 16:15:21 -0500 Subject: [PATCH 6/7] Remove punctuation on NewInvalidArgumentErrorf --- private/buf/cmd/buf/command/push/push.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/private/buf/cmd/buf/command/push/push.go b/private/buf/cmd/buf/command/push/push.go index 33206becf5..85af297b58 100644 --- a/private/buf/cmd/buf/command/push/push.go +++ b/private/buf/cmd/buf/command/push/push.go @@ -383,12 +383,12 @@ func validateCreateFlags(flags *flags) error { func validateLabelFlags(flags *flags) error { for _, label := range flags.Labels { if label == "" { - return appcmd.NewInvalidArgumentErrorf("--%s requires a non-empty string.", labelFlagName) + return appcmd.NewInvalidArgumentErrorf("--%s requires a non-empty string", labelFlagName) } } for _, tag := range flags.Tags { if tag == "" { - return appcmd.NewInvalidArgumentErrorf("--%s requires a non-empty string.", tagFlagName) + return appcmd.NewInvalidArgumentErrorf("--%s requires a non-empty string", tagFlagName) } } return nil From 23c861163a70e47ba5fa5d2f08ae1d4fdfd6cd78 Mon Sep 17 00:00:00 2001 From: Saquib Mian Date: Wed, 21 Feb 2024 16:41:23 -0500 Subject: [PATCH 7/7] pr comments --- private/buf/cmd/buf/command/push/push.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/private/buf/cmd/buf/command/push/push.go b/private/buf/cmd/buf/command/push/push.go index 85af297b58..3e42567613 100644 --- a/private/buf/cmd/buf/command/push/push.go +++ b/private/buf/cmd/buf/command/push/push.go @@ -360,7 +360,7 @@ func validateCreateFlags(flags *flags) error { if flags.Create { if flags.CreateVisibility == "" { return appcmd.NewInvalidArgumentErrorf( - "--%s is required if --%s is set.", + "--%s is required if --%s is set", createVisibilityFlagName, createFlagName, ) @@ -371,7 +371,7 @@ func validateCreateFlags(flags *flags) error { } else { if flags.CreateVisibility != "" { return appcmd.NewInvalidArgumentErrorf( - "Cannot set --%s without --%s.", + "Cannot set --%s without --%s", createVisibilityFlagName, createFlagName, ) @@ -395,9 +395,7 @@ func validateLabelFlags(flags *flags) error { } func combineLabelLikeFlags(flags *flags) []string { - labels := make([]string, 0, len(flags.Labels)+len(flags.Tags)) - labels = append(labels, flags.Labels...) - labels = append(labels, flags.Tags...) + labels := append(slicesext.Copy(flags.Labels), flags.Tags...) if flags.Draft != "" { labels = append(labels, flags.Draft) }