Header values containing comma won't work
boggyhole opened this issue · 5 comments
First of all: Thanks heaps for this wonderful tool. I am absolutely loving it. Great work, thanks!
I just noticed headers are declared as StringSliceVarP:
flagSet.StringSliceVarP(&c.headers, "header", "H", []string{}, "A request header to be sent. Can be used multiple times to send multiple headers.")
Why allow comma separated values within the --header flag? I think for header values it's a common use case to have them contain a comma separated list of values (i.e. user groups).
Solution: Switch to StringArrayVarP and force users to supply multiple --header flags to define all header fields.
So instead of:
ali --header "key1:value1,key2:value2"
Use this:
ali --header "groups:group1,group2,group3" --header "user:username"
What do you think?
@boggyhole Hey, thank you for reporting the issue and such a kind word!
You're quite right. According to RFC 2616, we have to be possible to combine the multiple header fields into one pair. Using StringArrayVarP looks cool. Can you address it? I'm always open to any kinds of PRs!
@nakabonne Yeah sure, I can add it and create PR. Will see when I get time to do it.
Thank you!
PR ready, see #102
Please note this is a breaking change and is not compatible with the following example call:
ali --header 'key1:value1,key2:value2
This would indeed set header key1 to value1,key2:value2.
@boggyhole Hey, thank you for addressing! It is. I will announce that breaking change.
Closing since this is resolved by #102