[ENHANCEMENT] Allow for whitespace-trailing passwords (#2873) #2954
Conversation
Signed-off-by: Leo Hardt <[email protected]>
| @@ -260,15 +260,13 @@ func ParseAKV(in []byte) *AKV { | |||
| continue | |||
| } | |||
|
|
|||
| line = strings.TrimSpace(line) | |||
There was a problem hiding this comment.
The components of the line were going to be trimmed anyway.
| key, val, found := strings.Cut(line, kvSep) | ||
| if !found { | ||
| continue | ||
| } | ||
|
|
||
| key = strings.TrimSpace(key) | ||
| val = strings.TrimSpace(val) |
There was a problem hiding this comment.
Should I add a more detailed comment of why val wasn't Trimmed? It would seem arbitrary otherwise.
Do you normally reference GitHub issues on comments?
There was a problem hiding this comment.
Remove the dead code and add a reference to the GH Issue, please.
| t.Parallel() | ||
| // Expected behaviour is KEY: VALUE, with one space. | ||
| // Fallback should exist for KEY:VALUE, with no spaces. | ||
| mlValue := `foobar |
There was a problem hiding this comment.
I think the issue initially was about passwords surrounded by spaces, no just key-values pairs, no?
How does that work for passwords now? A test would be nice.
There was a problem hiding this comment.
Correct, I think the issue was about whitespace in passwords.
Being able to handle them in KV pairs is very nice as well, but either we need to change the description of the PR to match the implementation or add support for whitespace in passwords as well.
There was a problem hiding this comment.
Sorry, I had a misunderstanding between KV pairs and the password itself. But it should also be simple to not Trim the password. But I also see no harm in leaving values trimmed as well. Do you mind if I do both on the same PR?
Signed-off-by: Leo Hardt <[email protected]>
Summary
AKVfromTrimming the first line.Technical Notes
TrimSpaceonParseAKVonly applies to Keys, not Values. So keys must be trimmed, but values are taken as-is.