Skip to content
This repository was archived by the owner on Jan 13, 2021. It is now read-only.

Enable override default headers from CLI #205

Merged
merged 3 commits into from
Feb 22, 2016
Merged

Enable override default headers from CLI #205

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bb62f80
Enable override default headers from CLI
pkrolikowski Feb 22, 2016
095c50e
Enable override default headers from CLI
pkrolikowski Feb 22, 2016
e509c7a
Enable override default headers from CLI
pkrolikowski Feb 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion hyper/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,12 @@ def set_request_data(args):
body, headers, params = {}, {}, {}
for i in args.items:
if i.sep == SEP_HEADERS:
headers[i.key] = i.value
# :key:value case
if i.key == '':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally speaking, good Python style tests for the empty string by saying if not i.key, rather than explicitly comparing to the empty string. For that reason, let's flip this entire block on its head, changing it to:

if i.key:
    headers[i.key] = i.value
else:
    # :key:value case
    k, v = i.value.split(':', 1)
    headers[':' + k] = v

Let's also make this comment a little bit more expressive: something like "if the user wants to override a HTTP/2 special header there will be a leading colon, which tricks the command line parser into thinking the header is empty".

k, v = i.value.split(':')
Copy link
Member

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).

headers[':' + k] = v
else:
headers[i.key] = i.value
elif i.sep == SEP_QUERY:
params[i.key] = i.value
elif i.sep == SEP_DATA:
Expand Down
9 changes: 9 additions & 0 deletions test/test_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,20 @@ def test_cli_with_system_exit(argv):
{'method': 'GET', 'url.path': '/?param=test'}),
(['POST', 'example.com', 'data=test'],
{'method': 'POST', 'body': '{"data": "test"}'}),
(['GET', 'example.com', ':authority:example.org'],
{'method': 'GET', 'headers': {
':authority': 'example.org'}}),
(['GET', 'example.com', ':authority:example.org', 'x-test:header'],
{'method': 'GET', 'headers': {
':authority': 'example.org',
'x-test':'header'}}),
], ids=[
'specified "--debug" option',
'specified host and additional header',
'specified host and get parameter',
'specified host and post data',
'specified host and override default header',
'specified host and override default header and additional header',
])
def test_parse_argument(argv, expected):
args = parse_argument(argv)
Expand Down