Skip to content

Commit

Permalink
Fix partial redirect bug
Browse files Browse the repository at this point in the history
  • Loading branch information
algesten committed Jan 27, 2025
1 parent 5a3ee94 commit 2d0991c
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Unreleased

* Fix bug parsing partial redirects (#958)
* Expose typestate variables (#956)

# 3.0.1
Expand Down
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ features = ["rustls", "platform-verifier", "native-tls", "socks-proxy", "cookies

[dependencies]
base64 = "0.22.1"
ureq-proto = "0.2.5"
ureq-proto = "0.2.6"
# ureq-proto = { path = "../ureq-proto" }
log = "0.4.22"
once_cell = "1.19.0"
Expand Down
31 changes: 27 additions & 4 deletions src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,15 @@ fn flow_run(
SendRequestResult::RecvResponse(flow) => flow,
};

let (mut response, response_result) = recv_response(flow, &mut connection, config, timings)?;
let is_following_redirects = redirect_count < config.max_redirects();

let (mut response, response_result) = recv_response(
flow,
&mut connection,
config,
timings,
is_following_redirects,
)?;

info!("{:?}", DebugResponse(&response));

Expand Down Expand Up @@ -190,7 +198,7 @@ fn flow_run(
};

if response.status().is_redirection() {
if redirect_count < config.max_redirects() {
if is_following_redirects {
let flow = handler.consume_redirect_body()?;

FlowResult::Redirect(flow, handler.timings)
Expand All @@ -206,7 +214,7 @@ fn flow_run(
RecvResponseResult::Redirect(flow) => {
cleanup(connection, flow.must_close_connection(), timings.now());

if redirect_count < config.max_redirects() {
if is_following_redirects {
FlowResult::Redirect(flow, mem::take(timings))
} else if config.max_redirects_do_error() {
return Err(Error::TooManyRedirects);
Expand Down Expand Up @@ -488,14 +496,29 @@ fn recv_response(
connection: &mut Connection,
config: &Config,
timings: &mut CallTimings,
is_following_redirects: bool,
) -> Result<(Response<()>, RecvResponseResult<()>), Error> {
let response = loop {
let timeout = timings.next_timeout(Timeout::RecvResponse);
let made_progress = connection.await_input(timeout)?;

let input = connection.buffers().input();

let (amount, maybe_response) = flow.try_response(input)?;
// If cookies are disabled, we can allow ourselves to follow
// the redirect as soon as we discover the `Location` header.
// There are plenty of broken servers out there that don't send
// the finishing \r\n on redirects. With cookies off, we can
// handle that situation.
//
// If cookies are enabled, we risk mising a `Set-Cookie` header
// with such a strategy.
let cookies_enabled = cfg!(feature = "cookies");

// If we are not following redirects, do not allow partial returned
// 302 responses.
let allow_partial_redirect = !cookies_enabled && is_following_redirects;

let (amount, maybe_response) = flow.try_response(input, allow_partial_redirect)?;

if input.len() > config.max_response_header_size() {
return Err(Error::LargeResponseHeader(
Expand Down

0 comments on commit 2d0991c

Please sign in to comment.