plusplusjiajia commented on code in PR #3082:
URL: https://github.com/apache/iceberg-rust/pull/3082#discussion_r4214071594


##########
crates/catalog/rest/src/auth/sigv4.rs:
##########
@@ -0,0 +1,1296 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! AWS SigV4 request signing for the REST catalog.
+
+use chrono::{DateTime, Utc};
+use iceberg::{Error, ErrorKind, Result};
+use sha2::{Digest, Sha256};
+
+/// Hex SHA-256 of the empty string.
+const EMPTY_BODY_HEX_SHA256: &str =
+    "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855";
+
+/// How the payload hash is encoded in the `x-amz-content-sha256` header.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+#[non_exhaustive]
+pub enum PayloadHashMode {
+    /// Iceberg Java's style: a base64 header when there is a body, hex when
+    /// there is none, and hex in the canonical request. The base64 comes from
+    /// how Java configures the AWS SDK, not from SigV4, so a verifier that
+    /// trusts the header instead of hashing the body rejects it.
+    IcebergRest,
+    /// Standard SigV4: hex everywhere.
+    StandardAws,
+}
+
+fn hex_sha256(data: &[u8]) -> String {
+    encode_hex(&Sha256::digest(data))
+}
+
+fn encode_hex(bytes: &[u8]) -> String {
+    bytes.iter().map(|byte| format!("{byte:02x}")).collect()
+}
+
+fn base64_encode(bytes: &[u8]) -> String {
+    base64::engine::Engine::encode(&base64::engine::general_purpose::STANDARD, 
bytes)
+}
+
+/// The `x-amz-content-sha256` value; `None` is no body at all.
+fn content_sha256_header(body: Option<&[u8]>, mode: PayloadHashMode) -> String 
{
+    match mode {
+        PayloadHashMode::StandardAws => hex_sha256(body.unwrap_or_default()),
+        PayloadHashMode::IcebergRest => match body {
+            None => EMPTY_BODY_HEX_SHA256.to_string(),
+            Some(body) => base64_encode(&Sha256::digest(body)),
+        },
+    }
+}
+
+/// Signs REST catalog requests the way Iceberg Java's `RESTSigV4AuthSession`
+/// does. Carries no credentials, so one signer serves every session.
+#[derive(Clone)]
+pub struct SigV4Signer {
+    region: String,
+    service: String,
+    mode: PayloadHashMode,
+}
+
+impl SigV4Signer {
+    /// Creates a new SigV4 signer.
+    pub fn new(

Review Comment:
   Done, `new` is replaced by a `typed-builder` builder.
   



##########
crates/catalog/rest/src/auth/sigv4.rs:
##########
@@ -0,0 +1,1296 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! AWS SigV4 request signing for the REST catalog.
+
+use chrono::{DateTime, Utc};
+use iceberg::{Error, ErrorKind, Result};
+use sha2::{Digest, Sha256};
+
+/// Hex SHA-256 of the empty string.
+const EMPTY_BODY_HEX_SHA256: &str =
+    "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855";
+
+/// How the payload hash is encoded in the `x-amz-content-sha256` header.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+#[non_exhaustive]
+pub enum PayloadHashMode {
+    /// Iceberg Java's style: a base64 header when there is a body, hex when
+    /// there is none, and hex in the canonical request. The base64 comes from
+    /// how Java configures the AWS SDK, not from SigV4, so a verifier that
+    /// trusts the header instead of hashing the body rejects it.
+    IcebergRest,
+    /// Standard SigV4: hex everywhere.
+    StandardAws,
+}
+
+fn hex_sha256(data: &[u8]) -> String {
+    encode_hex(&Sha256::digest(data))
+}
+
+fn encode_hex(bytes: &[u8]) -> String {
+    bytes.iter().map(|byte| format!("{byte:02x}")).collect()
+}
+
+fn base64_encode(bytes: &[u8]) -> String {
+    base64::engine::Engine::encode(&base64::engine::general_purpose::STANDARD, 
bytes)
+}
+
+/// The `x-amz-content-sha256` value; `None` is no body at all.
+fn content_sha256_header(body: Option<&[u8]>, mode: PayloadHashMode) -> String 
{
+    match mode {
+        PayloadHashMode::StandardAws => hex_sha256(body.unwrap_or_default()),
+        PayloadHashMode::IcebergRest => match body {
+            None => EMPTY_BODY_HEX_SHA256.to_string(),
+            Some(body) => base64_encode(&Sha256::digest(body)),
+        },
+    }
+}
+
+/// Signs REST catalog requests the way Iceberg Java's `RESTSigV4AuthSession`
+/// does. Carries no credentials, so one signer serves every session.
+#[derive(Clone)]
+pub struct SigV4Signer {
+    region: String,
+    service: String,
+    mode: PayloadHashMode,
+}
+
+impl SigV4Signer {
+    /// Creates a new SigV4 signer.
+    pub fn new(
+        region: impl Into<String>,
+        service: impl Into<String>,
+        mode: PayloadHashMode,
+    ) -> Self {
+        Self {
+            region: region.into(),
+            service: service.into(),
+            mode,
+        }
+    }
+
+    /// Signs `request` in place. An existing `Authorization` moves to
+    /// `Original-Authorization`, and a caller's `x-amz-date`,
+    /// `x-amz-content-sha256` or `x-amz-security-token` that the signer
+    /// overwrites moves to `Original-<name>` (Java keeps a caller's content
+    /// hash when there is a body). Userinfo leaves the URL, and a `+` in the
+    /// query becomes `%20`, so write a literal plus as `%2B`.
+    ///
+    /// Uses `credentials` as given and never refreshes them: resolve temporary
+    /// ones from their provider before each call.
+    ///
+    /// Fails on a streaming body or a non-UTF-8 header, which cannot be
+    /// canonicalized faithfully.
+    ///
+    /// Send the result through a client that does not follow redirects: a
+    /// redirect replays the signature, and across hosts reqwest drops
+    /// `Authorization` but keeps `Original-Authorization`.
+    ///
+    /// `aws_sigv4` traces requests without redacting `Original-Authorization`.
+    /// A `tracing` subscriber is muted for the call, but the `log` bridge (no
+    /// subscriber, or `log-always`) still forwards those events, so keep
+    /// `aws_sigv4` below trace level there.
+    pub fn sign(
+        &self,
+        request: &mut crate::HttpRequest,
+        credentials: &aws_credential_types::Credentials,
+    ) -> Result<()> {
+        self.sign_at(request, credentials, Utc::now())
+    }
+
+    fn sign_at(
+        &self,
+        request: &mut crate::HttpRequest,
+        credentials: &aws_credential_types::Credentials,
+        now: DateTime<Utc>,
+    ) -> Result<()> {
+        use aws_sigv4::http_request::{SignableBody, SignableRequest, sign};
+        use aws_sigv4::sign::v4;
+        use tracing::level_filters::LevelFilter;
+        use tracing::subscriber::NoSubscriber;
+
+        let body = signable_body(request)?;
+        let content_header = content_sha256_header(body.as_deref(), self.mode);
+
+        convert_headers(request);
+
+        // Relocated after signing, so the `Original-` copy is not signed.
+        let displaced_content_hash: Vec<_> = request
+            .headers()
+            .get_all(CONTENT_SHA256)
+            .iter()
+            .filter(|v| v.as_bytes() != content_header.as_bytes())
+            .cloned()
+            .collect();
+        let content_value = content_header.parse().map_err(|e| {
+            Error::new(ErrorKind::Unexpected, "invalid x-amz-content-sha256 
value").with_source(e)
+        })?;
+        request.headers_mut().insert(CONTENT_SHA256, content_value);
+
+        rewrite_url_for_signing(request);
+
+        let identity = credentials.clone().into();
+        let params = v4::SigningParams::builder()
+            .identity(&identity)
+            .region(&self.region)
+            .name(&self.service)
+            .time(now.into())
+            .settings(signing_settings())
+            .build()
+            .map_err(|e| {
+                Error::new(ErrorKind::Unexpected, "failed to build SigV4 
params").with_source(e)
+            })?
+            .into();
+
+        let headers = signable_headers(request)?;
+        let signable = SignableRequest::new(
+            request.method().as_str(),
+            request.url_str(),
+            headers.into_iter(),
+            SignableBody::Bytes(body.as_deref().unwrap_or_default()),
+        )
+        .map_err(|e| {
+            Error::new(ErrorKind::DataInvalid, "request is not 
signable").with_source(e)
+        })?;
+
+        // `aws_sigv4` traces the request, `Original-Authorization` included.
+        // Mute it only when a subscriber could record that (the max level is
+        // `OFF` until one is registered): `with_default` marks tracing as in
+        // use for good, which turns off its `log` fallback process-wide.
+        let signed = if LevelFilter::current() == LevelFilter::TRACE {
+            tracing::subscriber::with_default(NoSubscriber::default(), || 
sign(signable, &params))
+        } else {
+            sign(signable, &params)
+        };
+        let (instructions, _signature) = signed
+            .map_err(|e| Error::new(ErrorKind::Unexpected, "SigV4 signing 
failed").with_source(e))?
+            .into_parts();
+
+        update_request_headers(request, instructions, displaced_content_hash)
+    }
+}
+
+/// The body to sign; as in Java, an absent body and an empty one differ.
+fn signable_body(request: &crate::HttpRequest) -> Result<Option<Vec<u8>>> {
+    match request.body() {
+        crate::HttpRequestBody::Empty => Ok(None),
+        crate::HttpRequestBody::Buffered(bytes) => Ok(Some(bytes.to_vec())),
+        crate::HttpRequestBody::Streaming => Err(Error::new(
+            ErrorKind::FeatureUnsupported,
+            "cannot sign a streaming request body",
+        )),
+    }
+}
+
+/// The headers to sign. A non-UTF-8 one is an error: skipping it would send
+/// it unsigned.
+fn signable_headers(request: &crate::HttpRequest) -> Result<Vec<(&str, &str)>> 
{
+    request
+        .headers()
+        .iter()
+        .map(|(n, v)| {
+            let v = v.to_str().map_err(|e| {
+                Error::new(
+                    ErrorKind::DataInvalid,
+                    format!("cannot sign non-UTF-8 header value for `{n}`"),
+                )
+                .with_source(e)
+            })?;
+            Ok((n.as_str(), v))
+        })
+        .collect()
+}
+
+/// Drops userinfo, which the wire `Host` never carries, and rewrites `+` in 
the
+/// query as `%20`: a space to AWS and Java either way, but unambiguous to any
+/// verifier.
+fn rewrite_url_for_signing(request: &mut crate::HttpRequest) {
+    if !request.url().username().is_empty() || 
request.url().password().is_some() {
+        let url = request.url_mut();
+        let _ = url.set_username("");

Review Comment:
   Propagated. (It can't actually fail: only host-less URLs and `file:` reject 
the setters, and neither can carry userinfo.)
   



##########
crates/catalog/rest/src/auth/sigv4.rs:
##########
@@ -0,0 +1,1296 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! AWS SigV4 request signing for the REST catalog.
+
+use chrono::{DateTime, Utc};
+use iceberg::{Error, ErrorKind, Result};
+use sha2::{Digest, Sha256};
+
+/// Hex SHA-256 of the empty string.
+const EMPTY_BODY_HEX_SHA256: &str =
+    "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855";
+
+/// How the payload hash is encoded in the `x-amz-content-sha256` header.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+#[non_exhaustive]
+pub enum PayloadHashMode {
+    /// Iceberg Java's style: a base64 header when there is a body, hex when
+    /// there is none, and hex in the canonical request. The base64 comes from
+    /// how Java configures the AWS SDK, not from SigV4, so a verifier that
+    /// trusts the header instead of hashing the body rejects it.
+    IcebergRest,
+    /// Standard SigV4: hex everywhere.
+    StandardAws,
+}
+
+fn hex_sha256(data: &[u8]) -> String {
+    encode_hex(&Sha256::digest(data))
+}
+
+fn encode_hex(bytes: &[u8]) -> String {
+    bytes.iter().map(|byte| format!("{byte:02x}")).collect()
+}
+
+fn base64_encode(bytes: &[u8]) -> String {
+    base64::engine::Engine::encode(&base64::engine::general_purpose::STANDARD, 
bytes)
+}
+
+/// The `x-amz-content-sha256` value; `None` is no body at all.
+fn content_sha256_header(body: Option<&[u8]>, mode: PayloadHashMode) -> String 
{
+    match mode {
+        PayloadHashMode::StandardAws => hex_sha256(body.unwrap_or_default()),
+        PayloadHashMode::IcebergRest => match body {
+            None => EMPTY_BODY_HEX_SHA256.to_string(),
+            Some(body) => base64_encode(&Sha256::digest(body)),
+        },
+    }
+}
+
+/// Signs REST catalog requests the way Iceberg Java's `RESTSigV4AuthSession`
+/// does. Carries no credentials, so one signer serves every session.
+#[derive(Clone)]
+pub struct SigV4Signer {
+    region: String,
+    service: String,
+    mode: PayloadHashMode,
+}
+
+impl SigV4Signer {
+    /// Creates a new SigV4 signer.
+    pub fn new(
+        region: impl Into<String>,
+        service: impl Into<String>,
+        mode: PayloadHashMode,
+    ) -> Self {
+        Self {
+            region: region.into(),
+            service: service.into(),
+            mode,
+        }
+    }
+
+    /// Signs `request` in place. An existing `Authorization` moves to
+    /// `Original-Authorization`, and a caller's `x-amz-date`,
+    /// `x-amz-content-sha256` or `x-amz-security-token` that the signer
+    /// overwrites moves to `Original-<name>` (Java keeps a caller's content
+    /// hash when there is a body). Userinfo leaves the URL, and a `+` in the
+    /// query becomes `%20`, so write a literal plus as `%2B`.
+    ///
+    /// Uses `credentials` as given and never refreshes them: resolve temporary
+    /// ones from their provider before each call.
+    ///
+    /// Fails on a streaming body or a non-UTF-8 header, which cannot be
+    /// canonicalized faithfully.
+    ///
+    /// Send the result through a client that does not follow redirects: a
+    /// redirect replays the signature, and across hosts reqwest drops
+    /// `Authorization` but keeps `Original-Authorization`.
+    ///
+    /// `aws_sigv4` traces requests without redacting `Original-Authorization`.
+    /// A `tracing` subscriber is muted for the call, but the `log` bridge (no
+    /// subscriber, or `log-always`) still forwards those events, so keep
+    /// `aws_sigv4` below trace level there.
+    pub fn sign(
+        &self,
+        request: &mut crate::HttpRequest,
+        credentials: &aws_credential_types::Credentials,
+    ) -> Result<()> {
+        self.sign_at(request, credentials, Utc::now())
+    }
+
+    fn sign_at(
+        &self,
+        request: &mut crate::HttpRequest,
+        credentials: &aws_credential_types::Credentials,
+        now: DateTime<Utc>,
+    ) -> Result<()> {
+        use aws_sigv4::http_request::{SignableBody, SignableRequest, sign};
+        use aws_sigv4::sign::v4;
+        use tracing::level_filters::LevelFilter;
+        use tracing::subscriber::NoSubscriber;
+
+        let body = signable_body(request)?;
+        let content_header = content_sha256_header(body.as_deref(), self.mode);
+
+        convert_headers(request);
+
+        // Relocated after signing, so the `Original-` copy is not signed.
+        let displaced_content_hash: Vec<_> = request
+            .headers()
+            .get_all(CONTENT_SHA256)
+            .iter()
+            .filter(|v| v.as_bytes() != content_header.as_bytes())
+            .cloned()
+            .collect();
+        let content_value = content_header.parse().map_err(|e| {
+            Error::new(ErrorKind::Unexpected, "invalid x-amz-content-sha256 
value").with_source(e)
+        })?;
+        request.headers_mut().insert(CONTENT_SHA256, content_value);
+
+        rewrite_url_for_signing(request);
+
+        let identity = credentials.clone().into();
+        let params = v4::SigningParams::builder()
+            .identity(&identity)
+            .region(&self.region)
+            .name(&self.service)
+            .time(now.into())
+            .settings(signing_settings())
+            .build()
+            .map_err(|e| {
+                Error::new(ErrorKind::Unexpected, "failed to build SigV4 
params").with_source(e)
+            })?
+            .into();
+
+        let headers = signable_headers(request)?;
+        let signable = SignableRequest::new(
+            request.method().as_str(),
+            request.url_str(),
+            headers.into_iter(),
+            SignableBody::Bytes(body.as_deref().unwrap_or_default()),
+        )
+        .map_err(|e| {
+            Error::new(ErrorKind::DataInvalid, "request is not 
signable").with_source(e)
+        })?;
+
+        // `aws_sigv4` traces the request, `Original-Authorization` included.
+        // Mute it only when a subscriber could record that (the max level is
+        // `OFF` until one is registered): `with_default` marks tracing as in

Review Comment:
   I ran it to be sure. tracing 0.1.44 with the `log` feature and a counting 
logger: `tracing::info!` reaches `log` before `with_default(NoSubscriber, || 
{})`, and never again after the scope ends. `set_default` stores tracing-core's 
`EXISTS` flag, nothing clears it, and the `log` bridge checks it. The comment 
now names the flag.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to