-
Notifications
You must be signed in to change notification settings - Fork 196
Enable override default headers from CLI #205
Conversation
headers[i.key] = i.value | ||
# :key:value case | ||
if i.key == '': | ||
k, v = i.value.split(':') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I don't think it's likely we could get a header with a colon in it (the only possible one is :path
, which might be able to have a colon in it), we should defend against that anyway. Let's swap this to i.value.split(':', 1)
.
Fantastic, everything is green! I have a few minor style notes that I'd like you to change, if possible, but when that's done I think this will be ready to merge. |
I changed this block as you suggested. |
else: | ||
# when overriding a HTTP/2 special header there will be a leading | ||
# colon, which tricks the command line parser into thinking | ||
# the header is empty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind indenting this one more level?
Sorry, I wasn't quite clear enough on the indentation. Fix that up and we're all good! =) |
Magnificent, thank you so much @pkrolikowski! ✨ 🍰 ✨ |
Enable override default headers from CLI
I am glad I could help :) So what do you think, what could I do next ? |
@Lukasa I had a problem with push to rebased branch, so I decided to make a new request.