Skip to content

Commit 78d708a

Browse files
ehl-jfehl-jf
authored
JGC-434 - Fix parsing of undeclared plugins args (jfrog#1513)
1 parent c8ec00c commit 78d708a

File tree

2 files changed

+12
-245
lines changed

2 files changed

+12
-245
lines changed

‎plugins/components/conversionlayer.go‎

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -456,14 +456,20 @@ func getValueForStringFlag(f StringFlag, baseContext *cli.Context) (string, erro
456456
return value, nil
457457
}
458458
// We try to find the flag value in the command arguments.
459-
flagIndex, flagValue := findFlag(f.Name, baseContext.Args())
459+
flagIndex, _, flagValue, _ := coreutils.FindFlag(f.Name, baseContext.Args())
460+
if flagIndex == -1 {
461+
flagIndex, _, flagValue, _ = coreutils.FindFlag("--"+f.Name, baseContext.Args())
462+
}
463+
460464
if flagIndex != -1 {
461465
return flagValue, nil
462466
}
467+
463468
if f.DefaultValue != "" {
464469
// Empty but has a default value defined.
465470
return f.DefaultValue, nil
466471
}
472+
467473
if f.Mandatory {
468474
// Empty but mandatory.
469475
return "", errors.New("Mandatory flag '" + f.Name + "' is missing")
@@ -477,32 +483,14 @@ func getValueForBoolFlag(f BoolFlag, baseContext *cli.Context) bool {
477483
}
478484

479485
// We try to find the flag value in the command arguments.
480-
flagIndex, flagValue := findFlag(f.Name, baseContext.Args())
486+
flagIndex, flagValue, _ := coreutils.FindBooleanFlag(f.Name, baseContext.Args())
487+
if flagIndex == -1 {
488+
flagIndex, flagValue, _ = coreutils.FindBooleanFlag("--"+f.Name, baseContext.Args())
489+
}
481490

482491
if flagIndex != -1 {
483-
switch strings.ToLower(flagValue) {
484-
case "true":
485-
return true
486-
case "false":
487-
return false
488-
case "":
489-
return true
490-
default:
491-
return false
492-
}
492+
return flagValue
493493
}
494494

495495
return f.DefaultValue
496496
}
497-
498-
func findFlag(flagName string, args []string) (flagIndex int, flagValue string) {
499-
var err error
500-
flagIndex, _, flagValue, err = coreutils.FindFlag(flagName, args)
501-
if err != nil {
502-
return
503-
}
504-
if flagIndex == -1 {
505-
flagIndex, _, flagValue, _ = coreutils.FindFlag("--"+flagName, args)
506-
}
507-
return flagIndex, flagValue
508-
}

‎plugins/components/conversionlayer_test.go‎

Lines changed: 0 additions & 221 deletions
Original file line numberDiff line numberDiff line change
@@ -308,125 +308,6 @@ func TestGetValueForStringFlag(t *testing.T) {
308308
assert.Equal(t, finalValue, expected)
309309
}
310310

311-
func TestGetValueForStringFlag_FindFlag(t *testing.T) {
312-
t.Run("FlagNotSetInContext_FoundInArgs_WithValue", func(t *testing.T) {
313-
f := NewStringFlag("string-flag", "Test description")
314-
// Test findFlag directly to verify it finds the flag in args
315-
testArgs := []string{"arg1", "--string-flag=test-value", "arg2"}
316-
flagIndex, flagValue := findFlag(f.Name, testArgs)
317-
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
318-
assert.Equal(t, "test-value", flagValue, "Flag value should be extracted correctly")
319-
})
320-
321-
t.Run("FlagNotSetInContext_FoundInArgs_WithSpaceSeparatedValue", func(t *testing.T) {
322-
f := NewStringFlag("string-flag", "Test description")
323-
// Test that findFlag can find flag with space-separated value
324-
testArgs := []string{"arg1", "--string-flag", "test-value", "arg2"}
325-
flagIndex, flagValue := findFlag(f.Name, testArgs)
326-
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
327-
assert.Equal(t, "test-value", flagValue, "Flag value should be extracted from next argument")
328-
})
329-
330-
t.Run("FlagNotSetInContext_FoundInArgs_OverridesDefault", func(t *testing.T) {
331-
f := NewStringFlag("string-flag", "Test description", WithStrDefaultValue("default-value"))
332-
// Test that when flag is found in args, it overrides default
333-
testArgs := []string{"--string-flag=arg-value"}
334-
flagIndex, flagValue := findFlag(f.Name, testArgs)
335-
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
336-
assert.Equal(t, "arg-value", flagValue, "Flag value from args should override default")
337-
assert.NotEqual(t, f.DefaultValue, flagValue, "Flag value should not be the default")
338-
})
339-
340-
t.Run("FlagNotSetInContext_NotFoundInArgs_UsesDefault", func(t *testing.T) {
341-
f := NewStringFlag("string-flag", "Test description", WithStrDefaultValue("default-value"))
342-
flagSet := flag.NewFlagSet("test", flag.ContinueOnError)
343-
flagSet.String(f.Name, "", "")
344-
assert.NoError(t, flagSet.Parse([]string{}))
345-
baseContext := cli.NewContext(nil, flagSet, nil)
346-
347-
// Flag not in args, should use default
348-
testArgs := []string{"arg1", "arg2"}
349-
flagIndex, _ := findFlag(f.Name, testArgs)
350-
assert.Equal(t, -1, flagIndex, "Flag should not be found in args")
351-
// When flagIndex == -1, function should return default value
352-
finalValue, err := getValueForStringFlag(f, baseContext)
353-
assert.NoError(t, err)
354-
assert.Equal(t, f.DefaultValue, finalValue, "Should return default value when flag not found")
355-
})
356-
357-
t.Run("FlagNotSetInContext_NotFoundInArgs_Mandatory_ReturnsError", func(t *testing.T) {
358-
f := NewStringFlag("string-flag", "Test description", SetMandatory())
359-
flagSet := flag.NewFlagSet("test", flag.ContinueOnError)
360-
flagSet.String(f.Name, "", "")
361-
assert.NoError(t, flagSet.Parse([]string{}))
362-
baseContext := cli.NewContext(nil, flagSet, nil)
363-
364-
// Flag not in args and mandatory, should return error
365-
testArgs := []string{"arg1", "arg2"}
366-
flagIndex, _ := findFlag(f.Name, testArgs)
367-
assert.Equal(t, -1, flagIndex, "Flag should not be found in args")
368-
// When flagIndex == -1 and mandatory, function should return error
369-
_, err := getValueForStringFlag(f, baseContext)
370-
assert.Error(t, err, "Should return error for mandatory flag when not found")
371-
assert.Contains(t, err.Error(), "Mandatory flag")
372-
assert.Contains(t, err.Error(), f.Name)
373-
})
374-
375-
t.Run("FlagNotSetInContext_NotFoundInArgs_Optional_ReturnsEmpty", func(t *testing.T) {
376-
f := NewStringFlag("string-flag", "Test description")
377-
flagSet := flag.NewFlagSet("test", flag.ContinueOnError)
378-
flagSet.String(f.Name, "", "")
379-
assert.NoError(t, flagSet.Parse([]string{}))
380-
baseContext := cli.NewContext(nil, flagSet, nil)
381-
382-
// Flag not in args and optional, should return empty
383-
testArgs := []string{"arg1", "arg2"}
384-
flagIndex, _ := findFlag(f.Name, testArgs)
385-
assert.Equal(t, -1, flagIndex, "Flag should not be found in args")
386-
// When flagIndex == -1 and optional, function should return empty string
387-
finalValue, err := getValueForStringFlag(f, baseContext)
388-
assert.NoError(t, err)
389-
assert.Empty(t, finalValue, "Should return empty string for optional flag when not found")
390-
})
391-
392-
t.Run("FlagSetInContext_TakesPrecedenceOverArgs", func(t *testing.T) {
393-
f := NewStringFlag("string-flag", "Test description")
394-
flagSet := flag.NewFlagSet("test", flag.ContinueOnError)
395-
flagSet.String(f.Name, "", "")
396-
contextValue := "context-value"
397-
assert.NoError(t, flagSet.Parse([]string{"--string-flag=" + contextValue}))
398-
baseContext := cli.NewContext(nil, flagSet, nil)
399-
400-
// Flag is set in context, should return context value even if args have different value
401-
finalValue, err := getValueForStringFlag(f, baseContext)
402-
assert.NoError(t, err)
403-
assert.Equal(t, contextValue, finalValue, "Flag set in context should take precedence")
404-
})
405-
406-
t.Run("FindFlag_WithEqualsSign", func(t *testing.T) {
407-
f := NewStringFlag("string-flag", "Test description")
408-
testArgs := []string{"--string-flag=value-with-equals"}
409-
flagIndex, flagValue := findFlag(f.Name, testArgs)
410-
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
411-
assert.Equal(t, "value-with-equals", flagValue, "Flag value with equals should be extracted correctly")
412-
})
413-
414-
t.Run("FindFlag_WithoutEqualsSign", func(t *testing.T) {
415-
f := NewStringFlag("string-flag", "Test description")
416-
testArgs := []string{"--string-flag", "value-without-equals"}
417-
flagIndex, flagValue := findFlag(f.Name, testArgs)
418-
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
419-
assert.Equal(t, "value-without-equals", flagValue, "Flag value without equals should be extracted correctly")
420-
})
421-
422-
t.Run("FindFlag_SimilarPrefix_DoesNotMatch", func(t *testing.T) {
423-
f := NewStringFlag("string-flag", "Test description")
424-
testArgs := []string{"--string-flag-other=value"}
425-
flagIndex, _ := findFlag(f.Name, testArgs)
426-
assert.Equal(t, -1, flagIndex, "Similar flag prefix should not match")
427-
})
428-
}
429-
430311
func TestGetValueForBoolFlag(t *testing.T) {
431312
t.Run("FlagSetInContext_True", func(t *testing.T) {
432313
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(false))
@@ -462,43 +343,6 @@ func TestGetValueForBoolFlag(t *testing.T) {
462343
assert.True(t, result, "BoolT flag set in context should return true")
463344
})
464345

465-
t.Run("FlagNotSetInContext_FoundInArgs_WithValueTrue", func(t *testing.T) {
466-
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(false))
467-
// Test the findFlag function directly to verify the logic
468-
testArgs := []string{"arg1", "--bool-flag=true", "arg2"}
469-
flagIndex, flagValue := findFlag(f.Name, testArgs)
470-
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
471-
assert.Equal(t, "true", flagValue, "Flag value should be 'true'")
472-
// When flagValue is "true", the function returns true
473-
})
474-
475-
t.Run("FlagNotSetInContext_FoundInArgs_WithValueFalse", func(t *testing.T) {
476-
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(true))
477-
testArgs := []string{"arg1", "--bool-flag=false", "arg2"}
478-
flagIndex, flagValue := findFlag(f.Name, testArgs)
479-
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
480-
assert.Equal(t, "false", flagValue, "Flag value should be 'false'")
481-
// When flagValue is "false", the function returns false
482-
})
483-
484-
t.Run("FlagNotSetInContext_FoundInArgs_EmptyValue", func(t *testing.T) {
485-
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(false))
486-
testArgs := []string{"arg1", "--bool-flag", "arg2"}
487-
flagIndex, flagValue := findFlag(f.Name, testArgs)
488-
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
489-
// When flagValue is empty (""), the function returns true
490-
assert.True(t, flagValue == "" || flagValue == "arg2", "Flag value should be empty or next arg")
491-
})
492-
493-
t.Run("FlagNotSetInContext_FoundInArgs_InvalidValue", func(t *testing.T) {
494-
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(true))
495-
testArgs := []string{"arg1", "--bool-flag=invalid", "arg2"}
496-
flagIndex, flagValue := findFlag(f.Name, testArgs)
497-
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
498-
assert.Equal(t, "invalid", flagValue, "Flag value should be 'invalid'")
499-
// When flagValue is not "true", "false", or "", the function returns false
500-
})
501-
502346
t.Run("FlagNotSetInContext_NotFoundInArgs_DefaultTrue", func(t *testing.T) {
503347
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(true))
504348
flagSet := flag.NewFlagSet("test", flag.ContinueOnError)
@@ -532,51 +376,6 @@ func TestGetValueForBoolFlag(t *testing.T) {
532376
assert.False(t, result, "Flag set in context should override default value")
533377
})
534378

535-
t.Run("FlagNotSetInContext_NotFoundInArgs_SimilarPrefix", func(t *testing.T) {
536-
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(false))
537-
// Test that similar prefix doesn't match
538-
testArgs := []string{"--bool-flag-other"}
539-
flagIndex, _ := findFlag(f.Name, testArgs)
540-
assert.Equal(t, -1, flagIndex, "Similar flag prefix should not match")
541-
// When flagIndex == -1, the function returns default value
542-
})
543-
544-
t.Run("FlagNotSetInContext_FoundInArgs_CaseInsensitiveTrue", func(t *testing.T) {
545-
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(false))
546-
testArgs := []string{"--bool-flag=TRUE"}
547-
flagIndex, flagValue := findFlag(f.Name, testArgs)
548-
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
549-
assert.Equal(t, "TRUE", flagValue, "Flag value should be 'TRUE'")
550-
// The function uses strings.ToLower, so "TRUE" becomes "true" and returns true
551-
})
552-
553-
t.Run("FlagNotSetInContext_FoundInArgs_CaseInsensitiveFalse", func(t *testing.T) {
554-
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(true))
555-
testArgs := []string{"--bool-flag=FALSE"}
556-
flagIndex, flagValue := findFlag(f.Name, testArgs)
557-
assert.NotEqual(t, -1, flagIndex, "Flag should be found in args")
558-
assert.Equal(t, "FALSE", flagValue, "Flag value should be 'FALSE'")
559-
// The function uses strings.ToLower, so "FALSE" becomes "false" and returns false
560-
})
561-
562-
t.Run("FlagValueParsing_True", func(t *testing.T) {
563-
// Test that flagValue "true" returns true
564-
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(false))
565-
testArgs := []string{"--bool-flag=true"}
566-
flagIndex, flagValue := findFlag(f.Name, testArgs)
567-
assert.NotEqual(t, -1, flagIndex)
568-
assert.Equal(t, "true", flagValue)
569-
})
570-
571-
t.Run("FlagValueParsing_False", func(t *testing.T) {
572-
// Test that flagValue "false" returns false
573-
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(true))
574-
testArgs := []string{"--bool-flag=false"}
575-
flagIndex, flagValue := findFlag(f.Name, testArgs)
576-
assert.NotEqual(t, -1, flagIndex)
577-
assert.Equal(t, "false", flagValue)
578-
})
579-
580379
t.Run("FlagValueParsing_Empty", func(t *testing.T) {
581380
// Test that empty flagValue returns true
582381
// When a flag is provided without a value like "--bool-flag",
@@ -599,26 +398,6 @@ func TestGetValueForBoolFlag(t *testing.T) {
599398
// Note: findFlag may return -1 for flags without values due to FindFlag error handling
600399
// but the important part is that the switch statement correctly handles "" -> true
601400
})
602-
603-
t.Run("FlagValueParsing_InvalidValue", func(t *testing.T) {
604-
// Test that invalid flagValue returns false
605-
f := NewBoolFlag("bool-flag", "Bool flag", WithBoolDefaultValue(true))
606-
testArgs := []string{"--bool-flag=invalid"}
607-
flagIndex, flagValue := findFlag(f.Name, testArgs)
608-
assert.NotEqual(t, -1, flagIndex)
609-
assert.Equal(t, "invalid", flagValue)
610-
// Verify the switch statement logic: "invalid" -> false (default case)
611-
switch strings.ToLower(flagValue) {
612-
case "true":
613-
assert.Fail(t, "Should not match 'true'")
614-
case "false":
615-
assert.Fail(t, "Should not match 'false'")
616-
case "":
617-
assert.Fail(t, "Should not match empty")
618-
default:
619-
assert.True(t, true, "Invalid flag value should result in false (default case)")
620-
}
621-
})
622401
}
623402

624403
type DummyFlagValue struct {

0 commit comments

Comments
 (0)