-
-
Notifications
You must be signed in to change notification settings - Fork 91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make following redirects opt in #118
Conversation
awscurl/awscurl.py
Outdated
@@ -65,7 +66,8 @@ def make_request(method, | |||
secret_key, | |||
security_token, | |||
data_binary, | |||
verify=True): | |||
verify=True, | |||
redirects=False): |
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.
allow_redirects seems clearer name for the parameter
awscurl/awscurl.py
Outdated
@@ -82,6 +84,7 @@ def make_request(method, | |||
:param security_token: str | |||
:param data_binary: bool | |||
:param verify: bool | |||
:param redirects: false |
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.
allow_redirects seems clearer name for the parameter
awscurl/awscurl.py
Outdated
@@ -320,14 +323,14 @@ def __now(): | |||
return datetime.datetime.utcnow() | |||
|
|||
|
|||
def __send_request(uri, data, headers, method, verify): | |||
def __send_request(uri, data, headers, method, verify, redirects): |
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.
allow_redirects seems clearer name for the parameter
awscurl/awscurl.py
Outdated
@@ -433,6 +436,8 @@ def inner_main(argv): | |||
# https://github.com/boto/botocore/blob/c76553d3158b083d818f88c898d8f6d7918478fd/botocore/credentials.py#L260-262 | |||
parser.add_argument('--security_token', env_var='AWS_SECURITY_TOKEN') | |||
parser.add_argument('--session_token', env_var='AWS_SESSION_TOKEN') | |||
parser.add_argument('-f', '--follow-redirects', action='store_true', default=False, |
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.
in spirit of following curl, please see curl --help | grep Follow
-- i think awscurl should follow curl's convention
awscurl/awscurl.py
Outdated
|
||
if args.include or IS_VERBOSE: | ||
print(response.headers, end='\n\n') | ||
print(json.dumps(dict(response.headers)), end='\n\n') |
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.
please dont change default output format
Following the example from `curl`, requests will now only follow redirects if the `-L` or `--location` flag is provided. Signed requests will error on most redirects to other AWS services because the redirect uri no longer matches the canonical uri in the signed request. Closes okigan#117
7011c9a
to
78fb1a6
Compare
@okigan OK, addressed your comments and concerns and cleaned up the commit message a bit. Thanks! |
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.
Nice update!, please continue to update the unit tests to get the build passing (you can run ci.sh or ci-in-docker.sh to test build before push)
I tried to run
|
No description provided.