Comments (17)
Thanks for the report. We'll have a fix out this week.
from aws-api.
- Client issues a request without any cache-related headers
- Client receives a response with a body, status 200, and cache-related headers
- Client issues a request for the same resource but now with cache-related headers
- The S3 server sees those headers and figures that the resource is not changed - it answers with status 304 and no body
from aws-api.
There is one other thing we added: :cognitect.aws.http/status
is now part of the body of all anomalies, so in the 304 case you'll get (at least):
{:cognitect.anomalies/category :cognitect.anomalies/conflict
:cognitect.aws.http/status 304}
So now application code can look for either the anomaly category or the status to make branching decisions.
from aws-api.
Would you please post the response you're getting in this case, along with (meta response)
?
from aws-api.
Sure, although I can easily do that only on my development setup where I use MinIO (with some private fields elided). But the behavior can be seen on proper AWS S3 as well.
The response:
{:cognitect.anomalies/category :cognitect.anomalies/incorrect}
Its meta:
{:http-request {:request-method :get
:scheme :http
:server-port 9000
:uri [...]
:headers {"x-amz-date" "20230531T101402Z"
"if-modified-since" "Tue, 09 May 2023 10:28:58 GMT"
"if-none-match" "\"2db6f39c4ad8bfe09eb1306ecacb64b2\""
"host" "localhost"
"x-amz-content-sha256" [...]
"authorization" [...]}
:body nil
:server-name "localhost"}
:http-response {:status 304
:headers {"server" "MinIO"
"x-content-type-options" "nosniff"
"strict-transport-security" "max-age=31536000; includeSubDomains"
"accept-ranges" "bytes"
"etag" "\"2db6f39c4ad8bfe09eb1306ecacb64b2\""
"x-amz-request-id" "176432D2F6B6204B"
"date" "Wed, 31 May 2023 10:14:02 GMT"
"vary" "Origin"
"last-modified" "Tue, 09 May 2023 10:28:58 GMT"
"x-xss-protection" "1; mode=block"
"content-security-policy" "block-all-mixed-content"}
:body nil}}
from aws-api.
And, just so I'm clear, the response you got before was whatever the content was and it just came with a 304 instead of a 2xx?
from aws-api.
Got it! Very helpful. Thank you.
from aws-api.
I'd not used these headers myself so I didn't know how to. Now I do ;)
For anybody else reading, the request map for e.g. :GetObject
accepts these cache related key/val pairs:
:IfNoneMatch string,
:IfUnmodifiedSince timestamp,
:IfModifiedSince timestamp,
:IfMatch string,
... which are coerced to http request headers e.g. "if-none-match", etc
from aws-api.
Wouldn't just allowing 304 though suffice, without having to peer into the headers?
from aws-api.
Wouldn't just allowing 304 though suffice, without having to peer into the headers?
100%! My comment about the headers was to provide context for readers who come across this issue down the road, not to hint at any implementation choice. Make sense?
from aws-api.
Ah, yeah. :)
from aws-api.
FYI, this opened up a design discussion that is taking a bit of time. I'm not sure where it's going to land yet, but in the mean time, you can test for (= 304 (-> response meta :http-response :status))
to make whatever programmatic decision you need to make.
from aws-api.
Is this discussion public? Curious to see the what the argument is about. Personally, I don't see any reason not to just check for 304 in the most direct way given that HTTP status codes are pretty much set in stone.
I have simply rolled back to an older version for now, so I hopefully don't have to change the app's code at all when this is resolved.
from aws-api.
Right now it's a private discussion among maintainers, but I'm happy to try to answer your questions.
Some context: whatever changes we make have to work consistently for all operations on all AWS services, which have different rules about how they deliver error information.
The AWS Java SDK (and I assume the others, still need to validate that) returns exceptions for everything >= 300, including 304. Before the recent change to aws-api, we were using 400 as a threshold, which meant that when AWS sent us an error payload with a 3xx (as they do for a 301), we were treating that as a success and delivering unspecified information to the user. This is the primary focus of the design discussion: what should we be doing for 3xx?
In terms of checking for a 304, (= 304 (-> response meta :http-response :status))
is the most direct way for client code to check for a 304 today, and is unlikely to change (though I'm not making any promises until we're ready to make them). This is quietly documented in the Usage section of the README, which shows that the http-response is in the metadata.
How is your code checking for the 304?
from aws-api.
This is quietly documented
We're going to make that less quiet once we define what we're doing here.
from aws-api.
Ah, makes sense in the context of all AWS services, thanks. I could be quite myopic here because I only use S3 currently.
(= 304 (-> response meta :http-response :status))
is the most direct way for client code to check for a 304 today
Right, I meant checking that in the library. To exclude the 304 status code from the ">= 300" range and treat it as a success. I can see how a redirect might not be considered a success, but I can't stretch that logic in my head enough to also cover the 304 status. After all, for the response to be 304, the request has to be made deliberately with the right headers, so the lib's client should expect that and in principle in that case 304 is not an anomaly but rather an expected behavior, unless, perhaps, if it's received in the absence of all the required headers.
How is your code checking for the 304?
That's the thing - it doesn't. As I mentioned, all my app's code is doing here is basically proxying S3 resources - it signs the URL for a requested resource, makes a request to AWS, and forwards the response to the client in full and mostly preserved. The only reason I'm checking for anomalies at all is to log truly anomalous things that shouldn't happen.
from aws-api.
Hey @p-himik. We've been studying this all this time, and we've decided that we're going to keep the new behavior, treating 304 as an error.
The rationale (also documented now in the README), is that AWS documents 304 as an error, and all of the AWS SDKs actually throw exceptions in this case.
We've also documented this as a breaking change in the CHANGELOG.
I understand and agree with your rationale for not treating 304 as an error. So does everybody else involved in the discussion. In the end, however, we all agreed that our guiding principle needs to be alignment with AWS semantics, regardless of whether AWS's choices align with expectations based on HTTP semantics.
I'm going to close this issue, but please feel free to keep commenting here if you wish, or we can discuss it further in the #aws channel in Clojurians Slack.
- https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObject.html
- https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorResponses.html#ErrorCodeList
from aws-api.
Related Issues (20)
- Missing `GeneratePresignedUrlRequest` S3 operation HOT 2
- 301 gets :cognitect.anomlies/fault HOT 3
- aws-api assumes ByteBuffer’s backing array represents complete content HOT 1
- case has int tests, but tested expression is not primitive HOT 1
- Verified Permissions always return 400 incorrect HOT 2
- Failing test? HOT 3
- IMDSv2 not supported HOT 6
- Failures faithfully parsing ISO8601 timestamps with microsecond precision
- Vulnerability in org.eclipse.jetty:[email protected] HOT 1
- Support Bedrock HOT 2
- AWS Location Service Endpoints Missing
- AWS S3 Express One Zone (AWS S3 Directory) HOT 1
- cognitect aws-api dependency security issues HOT 2
- http client fails with ring/ring-jetty-adapter 1.11.0 HOT 4
- Roadmap / plan for moving to AWS SDK v2 HOT 2
- "Property null is not supported" on valid responses
- Add support for Neptune Data API HOT 1
- Lambda invoke response is parsed to JSON automatically
- InvalidSignatureException when invoking :PostToConnection
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from aws-api.