Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix HTTP 307 redirects in verbose mode if request body is being streamed #1088

Merged
merged 1 commit into from
Jun 15, 2021
Merged

Fix HTTP 307 redirects in verbose mode if request body is being streamed #1088

merged 1 commit into from
Jun 15, 2021

Conversation

BoboTiG
Copy link
Contributor

@BoboTiG BoboTiG commented Jun 11, 2021 •
edited
Loading

Fixes #1079.

@BoboTiG BoboTiG changed the title Fix printing redirected prepared request in verbose mode Fix printing redirected prepared requests in verbose mode Jun 11, 2021
@BoboTiG BoboTiG closed this Jun 11, 2021
@BoboTiG BoboTiG reopened this Jun 11, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jun 11, 2021 •
edited
Loading

Codecov Report

Merging #1088 (0cfc7c1) into master (da03a06) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1088   +/-   ##
=======================================
  Coverage   95.84%   95.85%           
=======================================
  Files          61       61           
  Lines        4020     4029    +9     
=======================================
+ Hits         3853     3862    +9     
  Misses        167      167
Impacted Files Coverage Δ
httpie/core.py 92.19% <100.00%> (ø)
tests/test_redirects.py 97.05% <100.00%> (+1.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da03a06...0cfc7c1. Read the comment docs.

@BoboTiG
Copy link
Contributor Author

BoboTiG commented Jun 11, 2021 •
edited
Loading

It consistently fails on Windows 3.6 and 3.7.

@BoboTiG BoboTiG closed this Jun 11, 2021
@BoboTiG BoboTiG reopened this Jun 11, 2021
@BoboTiG BoboTiG changed the title Fix printing redirected prepared requests in verbose mode Fix HTTP 30x redirects in verbose mode if request body is being streamed Jun 11, 2021
@BoboTiG
Copy link
Contributor Author

BoboTiG commented Jun 15, 2021

The fix seems legit to me, WDYT @jakubroztocil ?

I would like to skip failing tests on Windows with Python 3.6 & 3.7 for now, and open an issue to keep track of that. The idea is to have the patch landing in the next release that will fix the issue for most of OSes.
A quick guess is that the issue may be somewhere in http.client but as Python 3.6 and 3.7 as in security-fixes only, an eventual fix may not be merged.

@BoboTiG BoboTiG marked this pull request as ready for review June 15, 2021 07:51
@BoboTiG
Copy link
Contributor Author

BoboTiG commented Jun 15, 2021

Well, now they both pass 🤔
Let's move forward then :)

@jkbrzt jkbrzt merged commit 2d55c01 into httpie:master Jun 15, 2021
@BoboTiG BoboTiG deleted the mickael/dev-86-fix-http-307-308-redirections-not-taken branch June 15, 2021 11:57
@jkbrzt jkbrzt changed the title Fix HTTP 30x redirects in verbose mode if request body is being streamed Fix HTTP 307 redirects in verbose mode if request body is being streamed Jun 15, 2021
@jkbrzt jkbrzt mentioned this pull request Jun 15, 2021
Merged
jkbrzt added a commit that referenced this pull request Jun 15, 2021
@jkbrzt
Final touches for #1088 (#1091)
07a0359 
* Make sure there’s no trailing \n in test files for easier output inspection

* Refactor output matching test utils

* More robust `test_http_307_allow_redirect_post_verbose()`

* Changelog

* Mention HTTP 307 Temporary Redirect re-post behaviour in README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot follow 307/308 HTTP redirects in verbose mode if request body is being streamed
3 participants
@BoboTiG @codecov-commenter @jkbrzt