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

GetObjectFluentBuilder.if_modified_since has no effect #818

Closed
cowlicks opened this issue Jun 2, 2023 · 7 comments
Closed

GetObjectFluentBuilder.if_modified_since has no effect #818

cowlicks opened this issue Jun 2, 2023 · 7 comments
Labels
bug This issue is a bug. p1 This is a high priority issue

Comments

@cowlicks
Copy link

cowlicks commented Jun 2, 2023

Describe the bug

When using GetObjectFluentBuilder.if_modified_since or set_if_modified_since to get an object, the object is always returned. I tried both methods, and tried setting the input to be both before and after the last_modified date of the object (from GetObjectOutput.last_modified). In every case, the object is returned.

Expected Behavior

The set_if_modified_since and if_modified_methods should only return an object if the object was modified after the provided input.

Current Behavior

the set_if_modified_since and if_modified_methods do nothing.

Reproduction Steps

#[tokio::test]
async fn bug_report() -> Result<()> {
    dotenvy::dotenv().ok();
    let config = ::aws_config::load_from_env().await;
    let client = aws_sdk_s3::Client::new(&config);
    let key = "my_key_that_exists.json";
    let bucket = "my-bucket";

    let if_modified_since_input: aws_sdk_s3::primitives::DateTime =
        std::time::SystemTime::now().into();
    let resp = client
        .get_object()
        .bucket(bucket)
        .key(key)
        .if_modified_since(if_modified_since_input)
        .send()
        .await?;
    assert!(resp.last_modified.unwrap() > if_modified_since_input);
    dbg!(resp);
    Ok(())
}

Possible Solution

No response

Additional Information/Context

No response

Version

├── aws-config v0.55.3
│   ├── aws-credential-types v0.55.3
│   │   ├── aws-smithy-async v0.55.3
│   │   ├── aws-smithy-types v0.55.3
│   ├── aws-http v0.55.3
│   │   ├── aws-credential-types v0.55.3 (*)
│   │   ├── aws-smithy-http v0.55.3
│   │   │   ├── aws-smithy-eventstream v0.55.3
│   │   │   │   ├── aws-smithy-types v0.55.3 (*)
│   │   │   ├── aws-smithy-types v0.55.3 (*)
│   │   ├── aws-smithy-types v0.55.3 (*)
│   │   ├── aws-types v0.55.3
│   │   │   ├── aws-credential-types v0.55.3 (*)
│   │   │   ├── aws-smithy-async v0.55.3 (*)
│   │   │   ├── aws-smithy-client v0.55.3
│   │   │   │   ├── aws-smithy-async v0.55.3 (*)
│   │   │   │   ├── aws-smithy-http v0.55.3 (*)
│   │   │   │   ├── aws-smithy-http-tower v0.55.3
│   │   │   │   │   ├── aws-smithy-http v0.55.3 (*)
│   │   │   │   │   ├── aws-smithy-types v0.55.3 (*)
│   │   │   │   ├── aws-smithy-types v0.55.3 (*)
│   │   │   ├── aws-smithy-http v0.55.3 (*)
│   │   │   ├── aws-smithy-types v0.55.3 (*)
│   ├── aws-sdk-sso v0.28.0
│   │   ├── aws-credential-types v0.55.3 (*)
│   │   ├── aws-endpoint v0.55.3
│   │   │   ├── aws-smithy-http v0.55.3 (*)
│   │   │   ├── aws-smithy-types v0.55.3 (*)
│   │   │   ├── aws-types v0.55.3 (*)
│   │   ├── aws-http v0.55.3 (*)
│   │   ├── aws-sig-auth v0.55.3
│   │   │   ├── aws-credential-types v0.55.3 (*)
│   │   │   ├── aws-sigv4 v0.55.3
│   │   │   │   ├── aws-smithy-eventstream v0.55.3 (*)
│   │   │   │   ├── aws-smithy-http v0.55.3 (*)
│   │   │   ├── aws-smithy-eventstream v0.55.3 (*)
│   │   │   ├── aws-smithy-http v0.55.3 (*)
│   │   │   ├── aws-types v0.55.3 (*)
│   │   ├── aws-smithy-async v0.55.3 (*)
│   │   ├── aws-smithy-client v0.55.3 (*)
│   │   ├── aws-smithy-http v0.55.3 (*)
│   │   ├── aws-smithy-http-tower v0.55.3 (*)
│   │   ├── aws-smithy-json v0.55.3
│   │   │   └── aws-smithy-types v0.55.3 (*)
│   │   ├── aws-smithy-types v0.55.3 (*)
│   │   ├── aws-types v0.55.3 (*)
│   ├── aws-sdk-sts v0.28.0
│   │   ├── aws-credential-types v0.55.3 (*)
│   │   ├── aws-endpoint v0.55.3 (*)
│   │   ├── aws-http v0.55.3 (*)
│   │   ├── aws-sig-auth v0.55.3 (*)
│   │   ├── aws-smithy-async v0.55.3 (*)
│   │   ├── aws-smithy-client v0.55.3 (*)
│   │   ├── aws-smithy-http v0.55.3 (*)
│   │   ├── aws-smithy-http-tower v0.55.3 (*)
│   │   ├── aws-smithy-json v0.55.3 (*)
│   │   ├── aws-smithy-query v0.55.3
│   │   │   ├── aws-smithy-types v0.55.3 (*)
│   │   ├── aws-smithy-types v0.55.3 (*)
│   │   ├── aws-smithy-xml v0.55.3
│   │   ├── aws-types v0.55.3 (*)
│   ├── aws-smithy-async v0.55.3 (*)
│   ├── aws-smithy-client v0.55.3 (*)
│   ├── aws-smithy-http v0.55.3 (*)
│   ├── aws-smithy-http-tower v0.55.3 (*)
│   ├── aws-smithy-json v0.55.3 (*)
│   ├── aws-smithy-types v0.55.3 (*)
│   ├── aws-types v0.55.3 (*)
├── aws-sdk-s3 v0.28.0
│   ├── aws-credential-types v0.55.3 (*)
│   ├── aws-endpoint v0.55.3 (*)
│   ├── aws-http v0.55.3 (*)
│   ├── aws-sig-auth v0.55.3 (*)
│   ├── aws-sigv4 v0.55.3 (*)
│   ├── aws-smithy-async v0.55.3 (*)
│   ├── aws-smithy-checksums v0.55.3
│   │   ├── aws-smithy-http v0.55.3 (*)
│   │   ├── aws-smithy-types v0.55.3 (*)
│   ├── aws-smithy-client v0.55.3 (*)
│   ├── aws-smithy-eventstream v0.55.3 (*)
│   ├── aws-smithy-http v0.55.3 (*)
│   ├── aws-smithy-http-tower v0.55.3 (*)
│   ├── aws-smithy-json v0.55.3 (*)
│   ├── aws-smithy-types v0.55.3 (*)
│   ├── aws-smithy-xml v0.55.3 (*)
│   ├── aws-types v0.55.3 (*)

Environment details (OS name and version, etc.)

arch linux

Logs

running 1 test
[2023-06-02T15:30:20Z INFO  aws_config::meta::region] load_region; provider=EnvironmentVariableRegionProvider { env: Env(Real) }
[2023-06-02T15:30:20Z DEBUG aws_config::fs_util] loaded home directory src="HOME"
[2023-06-02T15:30:20Z DEBUG aws_config::profile::parser::source] load_config_file; file=Default(Config)
[2023-06-02T15:30:20Z DEBUG aws_config::profile::parser::source] performing home directory substitution home="/home/<REDACTED>" path="~/.aws/config"
[2023-06-02T15:30:20Z DEBUG aws_config::profile::parser::source] home directory expanded before="~/.aws/config" after="/home/<REDACTED>/.aws/config"
[2023-06-02T15:30:20Z DEBUG aws_config::profile::parser::source] config file loaded path=Some("/home/<REDACTED>/.aws/config") size=47
[2023-06-02T15:30:20Z DEBUG aws_config::profile::parser::source] load_config_file; file=Default(Credentials)
[2023-06-02T15:30:20Z DEBUG aws_config::profile::parser::source] performing home directory substitution home="/home/<REDACTED>" path="~/.aws/credentials"
[2023-06-02T15:30:20Z DEBUG aws_config::profile::parser::source] home directory expanded before="~/.aws/credentials" after="/home/<REDACTED>/.aws/credentials"
[2023-06-02T15:30:20Z DEBUG aws_config::profile::parser::source] config file not found path=~/.aws/credentials
[2023-06-02T15:30:20Z DEBUG aws_config::profile::parser::source] config file loaded path=Some("/home/<REDACTED>/.aws/credentials") size=0
[2023-06-02T15:30:20Z DEBUG tracing::span] build_profile_provider;
[2023-06-02T15:30:20Z DEBUG hyper_rustls::config] with_native_roots processed 143 valid and 0 invalid certs
[2023-06-02T15:30:20Z DEBUG aws_smithy_client] send_operation;
[2023-06-02T15:30:20Z DEBUG aws_smithy_client] send_operation; operation="GetObject"
[2023-06-02T15:30:20Z DEBUG aws_smithy_client] send_operation; service="s3"
[2023-06-02T15:30:20Z DEBUG aws_smithy_http_tower::map_request] map_request; name="resolve_endpoint"
[2023-06-02T15:30:20Z DEBUG aws_smithy_http_tower::map_request] map_request; name="resolve_endpoint"
[2023-06-02T15:30:20Z DEBUG aws_smithy_http_tower::map_request] map_request; name="generate_user_agent"
[2023-06-02T15:30:20Z DEBUG aws_smithy_http_tower::map_request] async_map_request; name="retrieve_credentials"
[2023-06-02T15:30:20Z INFO  tracing::span] lazy_load_credentials;
[2023-06-02T15:30:20Z DEBUG aws_config::default_provider::credentials] provide_credentials; provider=default_chain
[2023-06-02T15:30:20Z DEBUG aws_config::meta::credentials::chain] load_credentials; provider=Environment
[2023-06-02T15:30:20Z DEBUG aws_config::meta::credentials::chain] loaded credentials provider=Environment
[2023-06-02T15:30:20Z INFO  aws_credential_types::cache::lazy_caching] credentials cache miss occurred; added new AWS credentials (took 70.706µs)
[2023-06-02T15:30:20Z DEBUG aws_credential_types::cache::lazy_caching] loaded credentials
[2023-06-02T15:30:20Z DEBUG aws_smithy_http_tower::map_request] map_request; name="sigv4_sign_request"
[2023-06-02T15:30:20Z DEBUG aws_smithy_http_tower::map_request] map_request; name="recursion_detection"
[2023-06-02T15:30:20Z DEBUG tracing::span] dispatch;
[2023-06-02T15:30:20Z DEBUG hyper::client::connect::dns] resolving host="<REDACTED>"
[2023-06-02T15:30:20Z DEBUG hyper::client::connect::http] connecting to <REDACTED>
[2023-06-02T15:30:20Z DEBUG hyper::client::connect::http] connected to <REDACTED>
[2023-06-02T15:30:20Z DEBUG rustls::client::hs] No cached session for DnsName(DnsName(DnsName("<REDACTED>")))
[2023-06-02T15:30:20Z DEBUG rustls::client::hs] Not resuming any session
[2023-06-02T15:30:20Z DEBUG rustls::client::hs] ALPN protocol is Some(b"http/1.1")
[2023-06-02T15:30:20Z DEBUG rustls::client::hs] Using ciphersuite TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
[2023-06-02T15:30:20Z DEBUG rustls::client::tls12] ECDHE curve is ECParameters { curve_type: NamedCurve, named_group: secp256r1 }
[2023-06-02T15:30:20Z DEBUG rustls::client::tls12] Server DNS name is DnsName(DnsName(DnsName("<REDACTED>")))
[2023-06-02T15:30:20Z DEBUG rustls::client::tls12] Session saved
[2023-06-02T15:30:20Z DEBUG hyper::proto::h1::io] flushed 686 bytes
[2023-06-02T15:30:20Z DEBUG hyper::proto::h1::io] parsed 10 headers
[2023-06-02T15:30:20Z DEBUG hyper::proto::h1::conn] incoming body is content-length (4746924 bytes)
[2023-06-02T15:30:20Z DEBUG tracing::span] load_response;
[2023-06-02T15:30:20Z DEBUG tracing::span] parse_unloaded;
[2023-06-02T15:30:20Z DEBUG aws_sdk_s3::operation::get_object] extended_request_id=Some(<REDACTED>)
[2023-06-02T15:30:20Z DEBUG aws_sdk_s3::operation::get_object] request_id=Some(<REDACTD>)
[2023-06-02T15:30:20Z DEBUG aws_smithy_client] send_operation; status="ok"
thread '<REDACTED>::bug_report' panicked at 'assertion failed: resp.last_modified.unwrap() > if_modified_since_input', <REDACTED>:389:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test <REDACTED>::bug_report ... FAILED

failures:

failures:
    <REDACTED>::bug_report

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 29 filtered out; finished in 0.27s

error: test failed, to rerun pass `--lib`
@cowlicks cowlicks added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 2, 2023
@Velfi Velfi removed the needs-triage This issue or PR still needs to be triaged. label Jun 2, 2023
@Velfi
Copy link
Contributor

Velfi commented Jun 2, 2023

Hey @cowlicks, thanks for submitting this issue. We'll look into it.

@Velfi
Copy link
Contributor

Velfi commented Jun 2, 2023

@cowlicks Ok, I figured it out. If you send dates with subsecond nanos, then S3 will silently ignore it. If you update your code above to this, then it should start working as expected:

#[tokio::test]
async fn bug_report() -> Result<()> {
    dotenvy::dotenv().ok();
    let config = ::aws_config::load_from_env().await;
    let client = aws_sdk_s3::Client::new(&config);
    let key = "my_key_that_exists.json";
    let bucket = "my-bucket";

    let if_modified_since_input: aws_sdk_s3::primitives::DateTime =
        std::time::SystemTime::now().into();
    if_modified_since_input.set_subsec_nanos(0);

    let resp = client
        .get_object()
        .bucket(bucket)
        .key(key)
        .if_modified_since(if_modified_since_input)
        .send()
        .await?;
    assert!(resp.last_modified.unwrap() > if_modified_since_input);
    dbg!(resp);
    Ok(())
}

@jmklix I would consider this a bug on S3's side. They should either accept datetimes with nanosecond precision or respond with a 400 explaining that they don't allow subsecond precision. Would you be able to check if other SDKs have this same issue and file a ticket with the S3 team?

@Velfi
Copy link
Contributor

Velfi commented Jun 7, 2023

@jmklix I filed an issue with the S3 maintainers, ticket number is V925584723.

@rcoh
Copy link
Contributor

rcoh commented Aug 8, 2023

It appears our date time formatting is incorrect:

  • ensure protocol tests exist in Smithy validating this behavior
  • fix our date time formatting
  • retest this API / Add protocol test specific to this API

@rcoh rcoh added the p1 This is a high priority issue label Aug 8, 2023
@rcoh rcoh added the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Oct 31, 2023
@rcoh
Copy link
Contributor

rcoh commented Nov 2, 2023

this is fixed in S3 0.35 (currently queued up to release)

@rcoh rcoh closed this as completed Nov 2, 2023
Copy link

github-actions bot commented Nov 2, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@rcoh
Copy link
Contributor

rcoh commented Nov 7, 2023

Fix verified

@rcoh rcoh removed the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p1 This is a high priority issue
Projects
None yet
Development

No branches or pull requests

3 participants