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.

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

AgentConfig override on request level #805

Merged
merged 1 commit into from
Sep 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion src/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ impl Agent {
run(self, request, body)
}

pub(crate) fn config(&self) -> &AgentConfig {
/// Get the config for this agent.
pub fn config(&self) -> &AgentConfig {
&self.config
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ pub enum Error {

/// The setting [`AgentConfig::https_only`](crate::AgentConfig::https_only) is true and
/// the URI is not https.
#[error("agent is configured for https only: {0}")]
AgentRequireHttpsOnly(String),
#[error("configured for https only: {0}")]
RequireHttpsOnly(String),

/// The response header, from status up until body, is too big.
///
Expand Down
51 changes: 46 additions & 5 deletions src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use http::{HeaderName, HeaderValue, Method, Request, Response, Uri, Version};
use crate::body::Body;
use crate::send_body::AsSendBody;
use crate::util::private::Private;
use crate::{Agent, Error, SendBody, Timeouts};
use crate::{Agent, AgentConfig, Error, SendBody, Timeouts};

/// Transparent wrapper around [`http::request::Builder`].
///
Expand Down Expand Up @@ -89,9 +89,43 @@ impl<Any> RequestBuilder<Any> {
self
}

/// Override agent level config on the request level.
///
/// The agent config is copied and modified on request level.
///
/// # Example
///
/// ```
/// use ureq::{Agent, AgentConfig, Timeouts};
/// use std::time::Duration;
///
/// let agent: Agent = AgentConfig {
/// https_only: false,
/// ..Default::default()
/// }.into();
///
/// let mut builder = agent.get("http://httpbin.org/get");
///
/// let config = builder.config();
/// config.https_only = true;
///
/// // Make the request
/// let result = builder.call();
///
/// // The https_only was set on request level
/// assert!(matches!(result.unwrap_err(), ureq::Error::RequireHttpsOnly(_)));
///
/// // The request level did not affect the agent level
/// assert!(!agent.config().https_only);
/// # Ok::<_, ureq::Error>(())
/// ```
pub fn config(&mut self) -> &mut AgentConfig {
self.request_level_config()
}

/// Override agent timeouts on the request level.
///
/// The agent setting is copied and modified on request level.
/// The agent config is copied and modified on request level.
///
/// # Example
///
Expand Down Expand Up @@ -122,16 +156,23 @@ impl<Any> RequestBuilder<Any> {
/// # Ok::<_, ureq::Error>(())
/// ```
pub fn timeouts(&mut self) -> &mut Timeouts {
&mut self.request_level_config().timeouts
}

fn request_level_config(&mut self) -> &mut AgentConfig {
let exts = self
.builder
.extensions_mut()
.expect("builder without errors");

if exts.get::<Timeouts>().is_none() {
exts.insert(self.agent.config().timeouts);
if exts.get::<AgentConfig>().is_none() {
let config_ref = &*self.agent.config;
// This is the cost of setting request level config
let config: AgentConfig = config_ref.clone();
exts.insert(config);
}

// unwrap is ok because of above logic
// Unwrap is OK because of above check
exts.get_mut().unwrap()
}
}
Expand Down
124 changes: 75 additions & 49 deletions src/run.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::convert::TryFrom;
use std::sync::Arc;
use std::{io, mem};

use hoot::client::flow::state::{Await100, RecvBody, RecvResponse, Redirect, SendRequest};
Expand All @@ -14,22 +15,28 @@
use crate::transport::time::{Duration, Instant};
use crate::transport::ConnectionDetails;
use crate::util::{DebugRequest, DebugResponse, DebugUri, HeaderMapExt, UriExt};
use crate::{Agent, AgentConfig, Body, Error, SendBody, Timeout, Timeouts};
use crate::{Agent, AgentConfig, Body, Error, SendBody, Timeout};

type Flow<T> = hoot::client::flow::Flow<(), T>;

/// Run a request.
///
/// This is the "main loop" of entire ureq.
pub(crate) fn run(
agent: &Agent,
request: Request<()>,
mut request: Request<()>,
mut body: SendBody,
) -> Result<Response<Body>, Error> {
let mut redirect_count = 0;

// Timeouts on the request level overrides the agent level.
let timeouts = *request
.extensions()
.get::<Timeouts>()
.unwrap_or(&agent.config.timeouts);
// Configuration on the request level overrides the agent level.
let config = request
.extensions_mut()
.remove::<AgentConfig>()
.map(Arc::new)
.unwrap_or_else(|| agent.config.clone());

let timeouts = config.timeouts;

let mut timings = CallTimings::new(timeouts, CurrentTime::default());

Expand All @@ -45,12 +52,19 @@
return Err(Error::Timeout(Timeout::Global));
}

match flow_run(agent, flow, &mut body, redirect_count, &mut timings)? {
match flow_run(
agent,
&config,
flow,
&mut body,
redirect_count,
&mut timings,
)? {
// Follow redirect
FlowResult::Redirect(rflow, rtimings) => {
redirect_count += 1;

flow = handle_redirect(rflow, &agent.config)?;
flow = handle_redirect(rflow, &config)?;
timings = rtimings.new_call();
}

Expand All @@ -76,34 +90,16 @@
let status = response.status();
let is_err = status.is_client_error() || status.is_server_error();

if agent.config.http_status_as_error && is_err {
if config.http_status_as_error && is_err {
return Err(Error::StatusCode(status.as_u16()));
}

Ok(response)
}
#[allow(clippy::large_enum_variant)]
enum FlowResult {
Redirect(Flow<Redirect>, CallTimings),
Response(Response<()>, BodyHandler),
}

#[derive(Default)]
pub(crate) struct BodyHandler {
flow: Option<Flow<RecvBody>>,
connection: Option<Connection>,
timings: CallTimings,
remote_closed: bool,
redirect: Option<Flow<Redirect>>,
}

pub(crate) enum BodyHandlerRef<'a> {
Shared(&'a mut BodyHandler),
Owned(BodyHandler),
}

fn flow_run(
agent: &Agent,
config: &AgentConfig,
mut flow: Flow<Prepare>,
body: &mut SendBody,
redirect_count: u32,
Expand All @@ -112,13 +108,13 @@
let uri = flow.uri().clone();
info!("{} {:?}", flow.method(), &DebugUri(flow.uri()));

if agent.config.https_only && uri.scheme() != Some(&Scheme::HTTPS) {
return Err(Error::AgentRequireHttpsOnly(uri.to_string()));
if config.https_only && uri.scheme() != Some(&Scheme::HTTPS) {
return Err(Error::RequireHttpsOnly(uri.to_string()));
}

add_headers(&mut flow, agent, body, &uri)?;
add_headers(&mut flow, agent, config, body, &uri)?;

let mut connection = connect(agent, &uri, timings)?;
let mut connection = connect(agent, config, &uri, timings)?;

let mut flow = flow.proceed();

Expand All @@ -143,7 +139,7 @@
SendRequestResult::RecvResponse(flow) => flow,
};

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

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

Expand All @@ -157,7 +153,7 @@
..Default::default()
};

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

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

if redirect_count >= agent.config.max_redirects {
if redirect_count >= config.max_redirects {
FlowResult::Response(response, BodyHandler::default())
} else {
FlowResult::Redirect(flow, mem::take(timings))
Expand All @@ -183,9 +179,20 @@
Ok(ret)
}

/// Return type of [`flow_run`].
#[allow(clippy::large_enum_variant)]
enum FlowResult {
/// Flow resulted in a redirect.
Redirect(Flow<Redirect>, CallTimings),

/// Flow resulted in a response.
Response(Response<()>, BodyHandler),
}

fn add_headers(
flow: &mut Flow<Prepare>,
agent: &Agent,

Check failure on line 194 in src/run.rs

View workflow job for this annotation

GitHub Actions / Test (socks-proxy)

unused variable: `agent`

Check failure on line 194 in src/run.rs

View workflow job for this annotation

GitHub Actions / Test

unused variable: `agent`

Check failure on line 194 in src/run.rs

View workflow job for this annotation

GitHub Actions / Test (charset)

unused variable: `agent`

Check failure on line 194 in src/run.rs

View workflow job for this annotation

GitHub Actions / Test (brotli)

unused variable: `agent`

Check failure on line 194 in src/run.rs

View workflow job for this annotation

GitHub Actions / Test

unused variable: `agent`

Check failure on line 194 in src/run.rs

View workflow job for this annotation

GitHub Actions / Test (brotli)

unused variable: `agent`

Check failure on line 194 in src/run.rs

View workflow job for this annotation

GitHub Actions / Test (json)

unused variable: `agent`

Check failure on line 194 in src/run.rs

View workflow job for this annotation

GitHub Actions / Test (charset)

unused variable: `agent`
config: &AgentConfig,
body: &SendBody,
uri: &Uri,
) -> Result<(), Error> {
Expand Down Expand Up @@ -247,10 +254,10 @@
}
}

if !has_header_ua && !agent.config.user_agent.is_empty() {
if !has_header_ua && !config.user_agent.is_empty() {
// unwrap is ok because a user might override the agent, and if they
// set bad values, it's not really a big problem.
let value = HeaderValue::try_from(&agent.config.user_agent).unwrap();
let value = HeaderValue::try_from(&config.user_agent).unwrap();
flow.header("user-agent", value)?;
}

Expand All @@ -262,9 +269,14 @@
Ok(())
}

fn connect(agent: &Agent, uri: &Uri, timings: &mut CallTimings) -> Result<Connection, Error> {
fn connect(
agent: &Agent,
config: &AgentConfig,
uri: &Uri,
timings: &mut CallTimings,
) -> Result<Connection, Error> {
// If we're using a CONNECT proxy, we need to resolve that hostname.
let maybe_connect_uri = agent.config.connect_proxy_uri();
let maybe_connect_uri = config.connect_proxy_uri();

let effective_uri = maybe_connect_uri.unwrap_or(uri);

Expand All @@ -274,7 +286,7 @@

let addrs = agent.resolver.resolve(
effective_uri,
&agent.config,
config,
timings.next_timeout(Timeout::Resolve),
)?;

Expand All @@ -284,7 +296,7 @@
uri,
addrs,
resolver: &*agent.resolver,
config: &agent.config,
config,
now: timings.now(),
timeout: timings.next_timeout(Timeout::Connect),
};
Expand Down Expand Up @@ -465,13 +477,13 @@
}
}

impl<'a> io::Read for BodyHandlerRef<'a> {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
match self {
BodyHandlerRef::Shared(v) => v.read(buf),
BodyHandlerRef::Owned(v) => v.read(buf),
}
}
#[derive(Default)]
pub(crate) struct BodyHandler {
flow: Option<Flow<RecvBody>>,
connection: Option<Connection>,
timings: CallTimings,
remote_closed: bool,
redirect: Option<Flow<Redirect>>,
}

impl BodyHandler {
Expand Down Expand Up @@ -605,3 +617,17 @@
self.do_read(buf).map_err(|e| e.into_io())
}
}

pub(crate) enum BodyHandlerRef<'a> {
Shared(&'a mut BodyHandler),
Owned(BodyHandler),
}

impl<'a> io::Read for BodyHandlerRef<'a> {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
match self {
BodyHandlerRef::Shared(v) => v.read(buf),
BodyHandlerRef::Owned(v) => v.read(buf),
}
}
}
Loading