-
Notifications
You must be signed in to change notification settings - Fork 39
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
add --cookie
option
#224
add --cookie
option
#224
Conversation
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.
Thanks for your work, I really appreciate it!
Just make sure to fix the tests (cargo test
).
@@ -68,8 +70,12 @@ impl Downloader { | |||
auth_map.insert(host, (username, password)); | |||
} | |||
|
|||
let mut headers = HeaderMap::new(); | |||
headers.insert(COOKIE, HeaderValue::from_str(cookie).unwrap()); |
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.
I fear this will not work over multiple domains, as you would always give the same cookies without taking into consideration the domains. What if 2 domains want a cookie named test
?
If you have time (and want to go further with this), have a look there:
- https://docs.rs/reqwest/latest/reqwest/cookie/struct.Jar.html#method.add_cookie_str
- https://docs.rs/reqwest/latest/reqwest/struct.ClientBuilder.html#method.cookie_provider
In any case this is better than what we have now so I will take this, with the modifications or not. Let me know
Signed-off-by: Esteban Blanc <[email protected]>
Signed-off-by: Esteban Blanc <[email protected]>
Replaced by #229 |
A simple pull request that adds a
--cookie
flag that allows setting a cookie header in the formatkey1=value;key2=value
Solves #67