Skip to content
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
22 changes: 12 additions & 10 deletions .github/workflows/http-cache.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,11 @@ jobs:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
- run: |
cargo test --all-targets --no-default-features --features manager-cacache,cacache-smol,with-http-types,manager-moka,streaming-smol,http-headers-compat
cargo test --all-targets --no-default-features --features manager-cacache,cacache-tokio,with-http-types,manager-moka,streaming-tokio,http-headers-compat
cargo test --all-targets --no-default-features --features manager-cacache,cacache-smol,with-http-types,manager-moka,streaming-smol,rate-limiting,http-headers-compat
cargo test --all-targets --no-default-features --features manager-cacache,cacache-tokio,with-http-types,manager-moka,streaming-tokio,rate-limiting,http-headers-compat
cargo test --all-targets --no-default-features --features manager-cacache,cacache-smol,with-http-types,manager-moka,streaming-smol
cargo test --all-targets --no-default-features --features manager-cacache,cacache-tokio,with-http-types,manager-moka,streaming-tokio
cargo test --all-targets --no-default-features --features manager-cacache,cacache-smol,with-http-types,manager-moka,streaming-smol,rate-limiting
cargo test --all-targets --no-default-features --features manager-cacache,cacache-tokio,with-http-types,manager-moka,streaming-tokio,rate-limiting

clippy:
name: Check clippy
Expand All @@ -57,10 +58,11 @@ jobs:
with:
components: "clippy"
- run: |
cargo clippy --lib --tests --all-targets --no-default-features --features manager-cacache,cacache-smol,with-http-types,manager-moka,streaming-smol,http-headers-compat -- -D warnings
cargo clippy --lib --tests --all-targets --no-default-features --features manager-cacache,cacache-tokio,with-http-types,manager-moka,streaming-tokio,http-headers-compat -- -D warnings
cargo clippy --lib --tests --all-targets --no-default-features --features manager-cacache,cacache-smol,with-http-types,manager-moka,streaming-smol,rate-limiting,http-headers-compat -- -D warnings
cargo clippy --lib --tests --all-targets --no-default-features --features manager-cacache,cacache-tokio,with-http-types,manager-moka,streaming-tokio,rate-limiting,http-headers-compat -- -D warnings
cargo clippy --lib --tests --all-targets --no-default-features --features manager-cacache,cacache-smol,with-http-types,manager-moka,streaming-smol -- -D warnings
cargo clippy --lib --tests --all-targets --no-default-features --features manager-cacache,cacache-tokio,with-http-types,manager-moka,streaming-tokio -- -D warnings
cargo clippy --lib --tests --all-targets --no-default-features --features manager-cacache,cacache-smol,with-http-types,manager-moka,streaming-smol,rate-limiting -- -D warnings
cargo clippy --lib --tests --all-targets --no-default-features --features manager-cacache,cacache-tokio,with-http-types,manager-moka,streaming-tokio,rate-limiting -- -D warnings

docs:
name: Build docs
Expand All @@ -74,7 +76,7 @@ jobs:
RUSTDOCFLAGS: --cfg docsrs -Dwarnings
- run: |
cargo doc --no-deps --document-private-items
cargo test --doc --no-default-features --features manager-cacache,cacache-smol,with-http-types,manager-moka,streaming-smol
cargo test --doc --no-default-features --features manager-cacache,cacache-tokio,with-http-types,manager-moka,streaming-tokio
cargo test --doc --no-default-features --features manager-cacache,cacache-smol,with-http-types,manager-moka,streaming-smol,rate-limiting
cargo test --doc --no-default-features --features manager-cacache,cacache-tokio,with-http-types,manager-moka,streaming-tokio,rate-limiting
cargo test --doc --no-default-features --features manager-cacache,cacache-smol,with-http-types,manager-moka,streaming-smol,http-headers-compat
cargo test --doc --no-default-features --features manager-cacache,cacache-tokio,with-http-types,manager-moka,streaming-tokio,http-headers-compat
cargo test --doc --no-default-features --features manager-cacache,cacache-smol,with-http-types,manager-moka,streaming-smol,rate-limiting,http-headers-compat
cargo test --doc --no-default-features --features manager-cacache,cacache-tokio,with-http-types,manager-moka,streaming-tokio,rate-limiting,http-headers-compat
12 changes: 2 additions & 10 deletions http-cache-reqwest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,7 @@ pub type ReqwestStreamingError = http_cache::ClientStreamingError;
#[cfg(feature = "streaming")]
use http_cache::StreamingCacheManager;

use std::{
collections::HashMap, convert::TryInto, str::FromStr, time::SystemTime,
};
use std::{convert::TryInto, str::FromStr, time::SystemTime};

pub use http::request::Parts;
use http::{
Expand Down Expand Up @@ -443,13 +441,7 @@ impl Middleware for ReqwestMiddleware<'_> {
.run(copied_req, self.extensions)
.await
.map_err(BoxError::from)?;
let mut headers = HashMap::new();
for header in res.headers() {
headers.insert(
header.0.as_str().to_owned(),
header.1.to_str()?.to_owned(),
);
}
let headers = res.headers().into();
let url = res.url().clone();
let status = res.status().into();
let version = res.version();
Expand Down
83 changes: 83 additions & 0 deletions http-cache-reqwest/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1750,6 +1750,89 @@ mod streaming_tests {
Ok(())
}

#[tokio::test]
async fn streaming_cache_with_multiple_vary_headers() -> Result<()> {
let manager = create_streaming_cache_manager();
let cache = HttpStreamingCache {
mode: CacheMode::Default,
manager,
options: Default::default(),
};

// Create a request
let request = Request::builder()
.uri("https://example.com/multiple-vary-test")
.header("user-agent", "test-agent")
.header("accept-encoding", "gzip")
.header("accept-language", "en-US")
.body(())
.unwrap();

let (parts, _) = request.into_parts();
let analysis = cache.analyze_request(&parts, None)?;

// Create a response with MULTIPLE Vary headers (the issue from #119)
let mut response = Response::builder()
.status(200)
.header("content-type", "application/json")
.header("cache-control", "max-age=3600")
.body(Full::new(Bytes::from("multiple vary test data")))
.unwrap();

// Add multiple Vary headers using headers_mut()
let headers = response.headers_mut();
headers.append("vary", "Prefer".parse().unwrap());
headers.append("vary", "Accept".parse().unwrap());
headers.append("vary", "Range".parse().unwrap());
headers.append("vary", "Accept-Encoding".parse().unwrap());
headers.append("vary", "Accept-Language".parse().unwrap());
headers.append("vary", "Accept-Datetime".parse().unwrap());

// Process the response
let cached_response =
cache.process_response(analysis.clone(), response).await?;
assert_eq!(cached_response.status(), 200);

// Verify the body
let body_bytes =
cached_response.into_body().collect().await?.to_bytes();
assert_eq!(body_bytes, "multiple vary test data");

// Test cache lookup - this is where the bug would show up
let cached_result =
cache.lookup_cached_response(&analysis.cache_key).await?;
assert!(cached_result.is_some());

if let Some((response, _policy)) = cached_result {
// Get ALL Vary header values
let vary_values: Vec<_> = response
.headers()
.get_all("vary")
.iter()
.map(|v| v.to_str().unwrap())
.collect();

// Verify we got all 6 Vary headers, not just one
assert_eq!(
vary_values.len(),
6,
"Expected 6 Vary headers, got {}: {:?}",
vary_values.len(),
vary_values
);

// Verify all expected values are present
assert!(vary_values.contains(&"Prefer"));
assert!(vary_values.contains(&"Accept"));
assert!(vary_values.contains(&"Range"));
assert!(vary_values.contains(&"Accept-Encoding"));
assert!(vary_values.contains(&"Accept-Language"));
assert!(vary_values.contains(&"Accept-Datetime"));
}

Ok(())
}

#[cfg(feature = "rate-limiting")]
#[tokio::test]
async fn test_streaming_with_rate_limiting() -> Result<()> {
Expand Down
6 changes: 3 additions & 3 deletions http-cache-surf/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@
//! ```

use std::convert::TryInto;
use std::str::FromStr;
use std::time::SystemTime;
use std::{collections::HashMap, str::FromStr};

use http::{
header::CACHE_CONTROL,
Expand All @@ -155,7 +155,7 @@ use http_cache::{
BadHeader, BoxError, CacheManager, CacheOptions, HitOrMiss, HttpResponse,
Middleware, Result, XCACHE, XCACHELOOKUP,
};
pub use http_cache::{CacheMode, HttpCache};
pub use http_cache::{CacheMode, HttpCache, HttpHeaders};
use http_cache_semantics::CachePolicy;
use http_types::{
headers::HeaderValue as HttpTypesHeaderValue,
Expand Down Expand Up @@ -258,7 +258,7 @@ impl Middleware for SurfMiddleware<'_> {
let url = self.req.url().clone();
let mut res =
self.next.run(self.req.clone().into(), self.client.clone()).await?;
let mut headers = HashMap::new();
let mut headers = HttpHeaders::new();
for header in res.iter() {
headers.insert(
header.0.as_str().to_owned(),
Expand Down
37 changes: 18 additions & 19 deletions http-cache-ureq/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,11 +584,14 @@ impl<'a, T: CacheManager> CachedRequestBuilder<'a, T> {
if cache_status_headers {
cached_response
.headers
.insert(XCACHE.to_string(), HitOrMiss::MISS.to_string());
cached_response.headers.insert(
XCACHELOOKUP.to_string(),
HitOrMiss::MISS.to_string(),
);
.entry(XCACHE.to_string())
.or_insert_with(Vec::new)
.push(HitOrMiss::MISS.to_string());
cached_response
.headers
.entry(XCACHELOOKUP.to_string())
.or_insert_with(Vec::new)
.push(HitOrMiss::MISS.to_string());
}

Ok(cached_response)
Expand Down Expand Up @@ -664,18 +667,9 @@ fn convert_ureq_response_to_http_response(
url: &str,
) -> Result<HttpResponse, HttpCacheError> {
let status = response.status();
let mut headers = HashMap::new();

// Copy headers
for (name, value) in response.headers() {
let value_str = value.to_str().map_err(|e| {
HttpCacheError::http(Box::new(std::io::Error::other(format!(
"Invalid header value: {}",
e
))))
})?;
headers.insert(name.as_str().to_string(), value_str.to_string());
}
let headers = response.headers().into();

// Read body using read_to_string
let body_string = response.body_mut().read_to_string().map_err(|e| {
Expand Down Expand Up @@ -708,7 +702,7 @@ fn convert_ureq_response_to_http_response(
#[derive(Debug)]
pub struct CachedResponse {
status: u16,
headers: HashMap<String, String>,
headers: HashMap<String, Vec<String>>,
body: Vec<u8>,
url: String,
cached: bool,
Expand All @@ -727,7 +721,9 @@ impl CachedResponse {

/// Get a header value
pub fn header(&self, name: &str) -> Option<&str> {
self.headers.get(name).map(|s| s.as_str())
self.headers
.get(name)
.and_then(|values| values.first().map(|s| s.as_str()))
}

/// Get all header names
Expand Down Expand Up @@ -786,7 +782,10 @@ impl CachedResponse {
let mut headers = HashMap::new();
for (name, value) in response.headers() {
let value_str = value.to_str().unwrap_or("");
headers.insert(name.as_str().to_string(), value_str.to_string());
headers
.entry(name.as_str().to_string())
.or_insert_with(Vec::new)
.push(value_str.to_string());
}

// Note: Cache headers will be added by the cache system based on cache_status_headers option
Expand All @@ -810,7 +809,7 @@ impl From<HttpResponse> for CachedResponse {
// based on the cache_status_headers option, so don't add them here
Self {
status: response.status,
headers: response.headers,
headers: response.headers.into(),
body: response.body,
url: response.url.to_string(),
cached: true,
Expand Down
1 change: 1 addition & 0 deletions http-cache/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ smol-macros = "0.1.1"

[features]
default = ["manager-cacache", "cacache-smol"]
http-headers-compat = []
manager-cacache = ["cacache", "bincode"]
cacache-tokio = ["cacache/tokio-runtime", "tokio", "bincode"]
cacache-smol = ["cacache/async-std", "smol"]
Expand Down
Loading
Loading