[pbs-devel] [PATCH proxmox 0/2] fix non-optional date header for S3 api response parsing

[pbs-devel] [PATCH proxmox 0/2] fix non-optional date header for S3 api response parsing

[Christian Ebner]

Fixes an implementation oversight with possibly missing http date
headers in the S3 API response parsing. The header is included in
the response by almost all providers, but it is documented as being
optional.

Also, includes regression tests for the http date header parsing.

Since the now optional date header in responses is currently not used
by any code path (it was only ever considered for time drift detection
between host and s3 provider), no further adaptions are required.

This will avoid issues as reported in:
https://forum.proxmox.com/threads/169395/

Christian Ebner (2):
  s3-client: fix non-optional date header for api response parsing
  s3-client: add regression tests for http date header parsing

 proxmox-s3-client/src/response_reader.rs | 57 +++++++++++++++++++-----
 proxmox-s3-client/src/timestamps.rs      |  2 +-
 2 files changed, 48 insertions(+), 11 deletions(-)

-- 
2.47.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox 1/2] s3-client: fix non-optional date header for api response parsing

[Christian Ebner]

Fixes an implementation oversight for the http date header response
parsing.

This header is most commonly present for all responses, but it is
documented as optional in the AWS reference documentation [0]. While
introduced during development to possibly detect time shifts between
the local s3 client and the api for e.g. last modified time checks,
it is currently not actively used by any codepath, and can therefore
be adapted without further modifications.

[0] https://docs.aws.amazon.com/AmazonS3/latest/API/RESTCommonResponseHeaders.html

Fixes: https://forum.proxmox.com/threads/169395/
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 proxmox-s3-client/src/response_reader.rs | 33 +++++++++++++++++-------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/proxmox-s3-client/src/response_reader.rs b/proxmox-s3-client/src/response_reader.rs
index bfd71205..f200e15a 100644
--- a/proxmox-s3-client/src/response_reader.rs
+++ b/proxmox-s3-client/src/response_reader.rs
@@ -9,8 +9,7 @@ use hyper::http::StatusCode;
 use hyper::{HeaderMap, Response};
 use serde::Deserialize;
 
-use crate::S3ObjectKey;
-use crate::{HttpDate, LastModifiedTimestamp};
+use crate::{HttpDate, LastModifiedTimestamp, S3ObjectKey};
 
 /// Response reader to check S3 api response status codes and parse response body, if any.
 pub(crate) struct ResponseReader {
@@ -21,7 +20,7 @@ pub(crate) struct ResponseReader {
 /// Response contents of list objects v2 api calls.
 pub struct ListObjectsV2Response {
     /// Parsed http date header from response.
-    pub date: HttpDate,
+    pub date: Option<HttpDate>,
     /// Bucket name.
     pub name: String,
     /// Requested key prefix.
@@ -64,7 +63,7 @@ struct ListObjectsV2ResponseBody {
 }
 
 impl ListObjectsV2ResponseBody {
-    fn with_date(self, date: HttpDate) -> ListObjectsV2Response {
+    fn with_optional_date(self, date: Option<HttpDate>) -> ListObjectsV2Response {
         ListObjectsV2Response {
             date,
             name: self.name,
@@ -105,7 +104,7 @@ pub struct HeadObjectResponse {
     /// Content type header.
     pub content_type: String,
     /// Http date header.
-    pub date: HttpDate,
+    pub date: Option<HttpDate>,
     /// Entity tag header.
     pub e_tag: String,
     /// Last modified http header.
@@ -120,7 +119,7 @@ pub struct GetObjectResponse {
     /// Content type header.
     pub content_type: String,
     /// Http date header.
-    pub date: HttpDate,
+    pub date: Option<HttpDate>,
     /// Entity tag header.
     pub e_tag: String,
     /// Last modified http header.
@@ -272,12 +271,12 @@ impl ResponseReader {
 
         let body = String::from_utf8(body.to_vec())?;
 
-        let date: HttpDate = Self::parse_header(header::DATE, &parts.headers)?;
+        let date = Self::parse_optional_date_header(&parts.headers)?;
 
         let response: ListObjectsV2ResponseBody =
             serde_xml_rs::from_str(&body).context("failed to parse response body")?;
 
-        Ok(response.with_date(date))
+        Ok(response.with_optional_date(date))
     }
 
     /// Read and parse the head object response.
@@ -303,7 +302,7 @@ impl ResponseReader {
         let content_length: u64 = Self::parse_header(header::CONTENT_LENGTH, &parts.headers)?;
         let content_type = Self::parse_header(header::CONTENT_TYPE, &parts.headers)?;
         let e_tag = Self::parse_header(header::ETAG, &parts.headers)?;
-        let date = Self::parse_header(header::DATE, &parts.headers)?;
+        let date = Self::parse_optional_date_header(&parts.headers)?;
         let last_modified = Self::parse_header(header::LAST_MODIFIED, &parts.headers)?;
 
         Ok(Some(HeadObjectResponse {
@@ -335,7 +334,7 @@ impl ResponseReader {
         let content_length: u64 = Self::parse_header(header::CONTENT_LENGTH, &parts.headers)?;
         let content_type = Self::parse_header(header::CONTENT_TYPE, &parts.headers)?;
         let e_tag = Self::parse_header(header::ETAG, &parts.headers)?;
-        let date = Self::parse_header(header::DATE, &parts.headers)?;
+        let date = Self::parse_optional_date_header(&parts.headers)?;
         let last_modified = Self::parse_header(header::LAST_MODIFIED, &parts.headers)?;
 
         Ok(Some(GetObjectResponse {
@@ -509,6 +508,20 @@ impl ResponseReader {
             .with_context(|| format!("failed to parse header '{name}'"))?;
         Ok(value)
     }
+
+    fn parse_optional_date_header(headers: &HeaderMap) -> Result<Option<HttpDate>, Error> {
+        let header_value = match headers.get(header::DATE) {
+            Some(value) => value,
+            None => return Ok(None),
+        };
+        let header_str = header_value
+            .to_str()
+            .with_context(|| format!("non UTF-8 header '{}'", header::DATE))?;
+        let date: HttpDate = header_str
+            .parse()
+            .with_context(|| format!("failed to parse header '{}'", header::DATE))?;
+        Ok(Some(date))
+    }
 }
 
 #[test]
-- 
2.47.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox 2/2] s3-client: add regression tests for http date header parsing

[Christian Ebner]

Basic tests to check that the header is parsed as expected if present
and ignored if not.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 proxmox-s3-client/src/response_reader.rs | 24 ++++++++++++++++++++++++
 proxmox-s3-client/src/timestamps.rs      |  2 +-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/proxmox-s3-client/src/response_reader.rs b/proxmox-s3-client/src/response_reader.rs
index f200e15a..c00c7bd4 100644
--- a/proxmox-s3-client/src/response_reader.rs
+++ b/proxmox-s3-client/src/response_reader.rs
@@ -632,3 +632,27 @@ fn parse_list_buckets_response_test() {
         ]
     );
 }
+
+#[test]
+fn test_optional_date_header_parsing() {
+    let mut header_map = HeaderMap::new();
+
+    let expected_date = "Wed, 12 Oct 2009 17:50:00 GMT";
+    header_map.insert(header::DATE, expected_date.parse().unwrap());
+    let parsed_date = ResponseReader::parse_optional_date_header(&header_map).unwrap();
+    assert!(parsed_date.is_some());
+    assert_eq!(
+        parsed_date.unwrap(),
+        HttpDate::from_str(expected_date).unwrap(),
+    );
+
+    header_map.clear();
+    let invalid_date_format = "2019-11-10";
+    header_map.insert(header::DATE, invalid_date_format.parse().unwrap());
+    assert!(ResponseReader::parse_optional_date_header(&header_map).is_err());
+
+    header_map.clear();
+    assert!(ResponseReader::parse_optional_date_header(&header_map)
+        .unwrap()
+        .is_none());
+}
diff --git a/proxmox-s3-client/src/timestamps.rs b/proxmox-s3-client/src/timestamps.rs
index 661e1fdf..74397593 100644
--- a/proxmox-s3-client/src/timestamps.rs
+++ b/proxmox-s3-client/src/timestamps.rs
@@ -28,7 +28,7 @@ serde_plain::derive_deserialize_from_fromstr!(LastModifiedTimestamp, "last modif
 /// https://datatracker.ietf.org/doc/html/rfc2616#section-3.3
 /// https://datatracker.ietf.org/doc/html/rfc1123#section-5.2.14
 /// https://datatracker.ietf.org/doc/html/rfc822#section-5
-#[derive(Debug)]
+#[derive(Debug, PartialEq)]
 pub struct HttpDate {
     _epoch: i64,
 }
-- 
2.47.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] applied-series: [PATCH proxmox 0/2] fix non-optional date header for S3 api response parsing

[Fabian Grünbichler]

On Fri, 08 Aug 2025 13:41:15 +0200, Christian Ebner wrote:
> Fixes an implementation oversight with possibly missing http date
> headers in the S3 API response parsing. The header is included in
> the response by almost all providers, but it is documented as being
> optional.
> 
> Also, includes regression tests for the http date header parsing.
> 
> [...]

Applied, thanks!

[1/2] s3-client: fix non-optional date header for api response parsing
      commit: edd10fd64c8fac228697d8d96ba3b5bd1a3332e7
[2/2] s3-client: add regression tests for http date header parsing
      commit: 25a9d35be7a69c4205fe22b2dc1444191ad8d8a5

Best regards,
-- 
Fabian Grünbichler <f.gruenbichler@proxmox.com>


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel