bilal-fazlani / commanddotnet

A modern framework for building modern CLI apps
https://commanddotnet.bilal-fazlani.com
MIT License
570 stars 29 forks source link

TypoSuggestionsMiddleware throws an exception if no arguments are provided #321

Closed taori closed 4 years ago

taori commented 4 years ago

Version 3.6.4

21:28:51.778 ERROR Application terminated in an unexpected way. System.InvalidOperationException: Sequence contains no elements at System.Linq.ThrowHelper.ThrowNoElementsException() at System.Linq.Enumerable.Max[TSource](IEnumerable1 source, Func2 selector) at CommandDotNet.Parsing.TypoSuggestionsMiddleware.<>c__DisplayClass5_0.b__1(String name) in CommandDotNet/Parsing/TypoSuggestionsMiddleware.cs:line 106 at System.Linq.Enumerable.WhereSelectEnumerableIterator2.ToArray() at System.Linq.Buffer1..ctor(IEnumerable1 source) at System.Linq.OrderedEnumerable1.ToList() at System.Linq.Enumerable.ToList[TSource](IEnumerable1 source) at CommandDotNet.Parsing.TypoSuggestionsMiddleware.GetSuggestions(IEnumerable1 names, String typo, Int32 maxSuggestionCount) in CommandDotNet/Parsing/TypoSuggestionsMiddleware.cs:line 98 at CommandDotNet.Parsing.TypoSuggestionsMiddleware.TrySuggest(CommandContext ctx, CommandParsingException cpe, IReadOnlyCollection`1 argumentNodes, String argumentNodeType, String prefix) in CommandDotNe t/Parsing/TypoSuggestionsMiddleware.cs:line 73

drewburlingame commented 4 years ago

Hi @taori, I see where it's happening and I can fix the code to prevent it, but I can't reproduce the error.

The only way I can see that the enum would be empty is if the typo was an empty string.

This stack is triggered by an UnrecognizedArgument error which means the token had an empty string. I don't see how the token could have an empty string.

Do you have a token transformation that could create a token w/ an empty string? If so, filter them out. Or does the string[] args you pass to AppRunner contain an empty string? If so, what OS and terminal are you using? I haven't encountered this behavior yet.

drewburlingame commented 4 years ago

Ok, I found a repro which is to use quotes in the shell to pass an empty string. myapp.exe "". Is that how you got an empty string?

taori commented 4 years ago

@drewburlingame You're correct about your diagnostic. The following code makes this happen:

grafik

drewburlingame commented 4 years ago

Is empty string a valid argument for your application? If not, can you filter them out? It would give you the fastest resolution.

Also, what's the intent of what you're doing here? Is it to host a repl session? If so, you may be interested in this PR https://github.com/bilal-fazlani/commanddotnet/pull/306. It will become a part of v5 at some point. In the meantime, you could copy that middleware into your app and adapt it until V5 is released. It has richer functionality for editing the line being input and also support for history.

taori commented 4 years ago

Essentially it is similar yes. I figured i'd report it, since using the Typo middleware results in an exception, while disabling it does not.

drewburlingame commented 4 years ago

Thanks for submitting it. I can update TypoSuggestions to handle empty string. I'm wondering what else may be vulnerable :)

With the fix, you'll still have a bug though since the CommandParser wasn't able to parse the input.
If it's helpful, you could use the CommandLineStringSplitter to split the input.

Seeing your use of Environment.UserInteractive reminds me that I should check that before starting a repl session or auto-prompting for missing arguments.

drewburlingame commented 4 years ago

fixed in https://www.nuget.org/packages/CommandDotNet/4.1.3