Return HTTP 400 if content-md5 does not match #596
Conversation
Fixes #596 Respect content-md5 headers on object uploads and return HTTP 400 with appropriate error body when it does not match the calculated digest of the object body. Also refactor the handling of content-md5 headers for multipart part uploads so that is matches the handling for normal object uploads.
|
thoughts on making a separate TODO for writing a client-test for this? |
yeah good idea |
|
I'm reviewing locally merged or rebased with #622. |
|
|
client test passed, but int-test is failing. Will see if this is happening on develop too. |
Fixes #596 Respect content-md5 headers on object uploads and return HTTP 400 with appropriate error body when it does not match the calculated digest of the object body. Also refactor the handling of content-md5 headers for multipart part uploads so that is matches the handling for normal object uploads.
|
Rebased from latest |
| @@ -70,6 +71,22 @@ api_error({error, Reason}, RD, Ctx) -> | |||
| error_response(StatusCode, _Code, _Message, RD, Ctx) -> | |||
| {{halt, StatusCode}, RD, Ctx}. | |||
|
|
|||
| %% @TODO Figure out how this should actually be formatted | |||
There was a problem hiding this comment.
In what way are we not sure how this should be formatted? Does this just mean we haven't tested that this makes clients throw the correct exception? If so, that can be part of #624.
There was a problem hiding this comment.
Oh right, this is also just for OpenStack, right?
There was a problem hiding this comment.
Right, they don't have any documentation I have found about expected error responses or error formatting and I haven't had a chance to manually determine it yet so punting for now.
|
+1, great work |
Fixes #596 Respect content-md5 headers on object uploads and return HTTP 400 with appropriate error body when it does not match the calculated digest of the object body. Also refactor the handling of content-md5 headers for multipart part uploads so that is matches the handling for normal object uploads.
|
See #624 to track testing of this PR. |
| _ = maybe_update_manifest_with_confirmation(ManiPid, Manifest), | ||
| _ = riak_cs_gc:gc_active_manifests(Bucket, Key, RiakPid), | ||
| gen_fsm:reply(From, {error, invalid_digest}), | ||
| {stop, invalid_digest, State}; |
There was a problem hiding this comment.
This causes a crash report to be printed. I might argue that this is still a 'normal' exit.
On PUT, Riak currently will not return HTTP 400 if the Content-MD5 header and the MD5 of the body do not match. S3 does. It does return the correct MD5 as the ETAG, and some clients do recognize this and throw and exception. Returning HTTP 400 is easy. Making sure we properly garbage collect the 'bad' version is a bit trickier.