diff options
author | Nick Vatamaniuc <vatamane@gmail.com> | 2021-07-30 17:05:29 -0400 |
---|---|---|
committer | Nick Vatamaniuc <nickva@users.noreply.github.com> | 2021-07-30 22:00:27 -0400 |
commit | bf355b96ee3efc24cb6edfb3eeb5b6fe519fdbf8 (patch) | |
tree | 24b2fe90c68ec99b0eaebd554fedf0d552dc29a2 | |
parent | 2332fbca9f8c63907a590f3a6da325ba7c602042 (diff) | |
download | couchdb-bf355b96ee3efc24cb6edfb3eeb5b6fe519fdbf8.tar.gz |
Port attachment deletion fix from 3.x
Port ba638783b5d87b855939dae69fd63ffd41cb5ed7 from 3.x
This should fix the edge case where deletion with a bad rev was returning a
404. Now it should returna 409.
Issue: https://github.com/apache/couchdb/issues/2146
-rw-r--r-- | src/chttpd/src/chttpd_db.erl | 271 | ||||
-rw-r--r-- | test/elixir/test/attachments_test.exs | 6 |
2 files changed, 139 insertions, 138 deletions
diff --git a/src/chttpd/src/chttpd_db.erl b/src/chttpd/src/chttpd_db.erl index 41bb74135..9edf2434a 100644 --- a/src/chttpd/src/chttpd_db.erl +++ b/src/chttpd/src/chttpd_db.erl @@ -1664,20 +1664,11 @@ couch_doc_open(Db, DocId, Rev, Options) -> end end. -get_attachment(Req, Db, DocId, FileName) -> - % Check if attachment exists - #doc_query_args{ - rev=Rev, - options=Options - } = parse_doc_query(Req), - #doc{ - atts=Atts - } = Doc = couch_doc_open(Db, DocId, Rev, Options), +get_existing_attachment(Atts, FileName) -> + % Check if attachment exists, if not throw not_found case [A || A <- Atts, couch_att:fetch(name, A) == FileName] of - [] -> - {error, missing}; - [Att] -> - {ok, Doc, Att} + [] -> throw({not_found, "Document is missing attachment"}); + [Att] -> Att end. % Attachment request handlers @@ -1692,125 +1683,128 @@ db_attachment_req(#httpd{method = 'GET', mochi_req = MochiReq} = Req, Db, DocId, "/" ) ), - case get_attachment(Req, Db, DocId, FileName) of - {error, missing} -> - throw({not_found, "Document is missing attachment"}); - {ok, Doc, Att} -> - [Type, Enc, DiskLen, AttLen, Md5] = couch_att:fetch( - [type, encoding, disk_len, att_len, md5], Att - ), - Refs = monitor_attachments(Att), - try - Etag = - case Md5 of - <<>> -> chttpd:doc_etag(Doc); - _ -> "\"" ++ ?b2l(base64:encode(Md5)) ++ "\"" - end, - ReqAcceptsAttEnc = lists:member( - atom_to_list(Enc), - couch_httpd:accepted_encodings(Req) - ), - Headers = - [ - {"ETag", Etag}, - {"Cache-Control", "must-revalidate"}, - {"Content-Type", binary_to_list(Type)} - ] ++ - case ReqAcceptsAttEnc of - true when Enc =/= identity -> - % RFC 2616 says that the 'identify' encoding should not be used in - % the Content-Encoding header - [{"Content-Encoding", atom_to_list(Enc)}]; - _ -> - [] - end ++ - case Enc of - identity -> - [{"Accept-Ranges", "bytes"}]; - _ -> - [{"Accept-Ranges", "none"}] - end, - Len = - case {Enc, ReqAcceptsAttEnc} of - {identity, _} -> - % stored and served in identity form - DiskLen; - {_, false} when DiskLen =/= AttLen -> - % Stored encoded, but client doesn't accept the encoding we used, - % so we need to decode on the fly. DiskLen is the identity length - % of the attachment. - DiskLen; - {_, true} -> - % Stored and served encoded. AttLen is the encoded length. - AttLen; - _ -> - % We received an encoded attachment and stored it as such, so we - % don't know the identity length. The client doesn't accept the - % encoding, and since we cannot serve a correct Content-Length - % header we'll fall back to a chunked response. - undefined - end, - AttFun = - case ReqAcceptsAttEnc of - false -> - fun couch_att:foldl_decode/3; - true -> - fun couch_att:foldl/3 - end, - chttpd:etag_respond( - Req, - Etag, - fun() -> - case Len of - undefined -> - {ok, Resp} = start_chunked_response(Req, 200, Headers), - AttFun(Att, fun(Seg, _) -> send_chunk(Resp, Seg) end, {ok, Resp}), - couch_httpd:last_chunk(Resp); + #doc_query_args{ + rev=Rev, + options=Options + } = parse_doc_query(Req), + #doc{ + atts=Atts + } = Doc = couch_doc_open(Db, DocId, Rev, Options), + Att = get_existing_attachment(Atts, FileName), + [Type, Enc, DiskLen, AttLen, Md5] = couch_att:fetch( + [type, encoding, disk_len, att_len, md5], Att + ), + Refs = monitor_attachments(Att), + try + Etag = + case Md5 of + <<>> -> chttpd:doc_etag(Doc); + _ -> "\"" ++ ?b2l(base64:encode(Md5)) ++ "\"" + end, + ReqAcceptsAttEnc = lists:member( + atom_to_list(Enc), + couch_httpd:accepted_encodings(Req) + ), + Headers = + [ + {"ETag", Etag}, + {"Cache-Control", "must-revalidate"}, + {"Content-Type", binary_to_list(Type)} + ] ++ + case ReqAcceptsAttEnc of + true when Enc =/= identity -> + % RFC 2616 says that the 'identify' encoding should not be used in + % the Content-Encoding header + [{"Content-Encoding", atom_to_list(Enc)}]; + _ -> + [] + end ++ + case Enc of + identity -> + [{"Accept-Ranges", "bytes"}]; + _ -> + [{"Accept-Ranges", "none"}] + end, + Len = + case {Enc, ReqAcceptsAttEnc} of + {identity, _} -> + % stored and served in identity form + DiskLen; + {_, false} when DiskLen =/= AttLen -> + % Stored encoded, but client doesn't accept the encoding we used, + % so we need to decode on the fly. DiskLen is the identity length + % of the attachment. + DiskLen; + {_, true} -> + % Stored and served encoded. AttLen is the encoded length. + AttLen; + _ -> + % We received an encoded attachment and stored it as such, so we + % don't know the identity length. The client doesn't accept the + % encoding, and since we cannot serve a correct Content-Length + % header we'll fall back to a chunked response. + undefined + end, + AttFun = + case ReqAcceptsAttEnc of + false -> + fun couch_att:foldl_decode/3; + true -> + fun couch_att:foldl/3 + end, + chttpd:etag_respond( + Req, + Etag, + fun() -> + case Len of + undefined -> + {ok, Resp} = start_chunked_response(Req, 200, Headers), + AttFun(Att, fun(Seg, _) -> send_chunk(Resp, Seg) end, {ok, Resp}), + couch_httpd:last_chunk(Resp); + _ -> + Ranges = parse_ranges(MochiReq:get(range), Len), + case {Enc, Ranges} of + {identity, [{From, To}]} -> + Headers1 = + [{"Content-Range", make_content_range(From, To, Len)}] ++ + Headers, + {ok, Resp} = start_response_length( + Req, 206, Headers1, To - From + 1 + ), + couch_att:range_foldl( + Att, + From, + To + 1, + fun(Seg, _) -> send(Resp, Seg) end, + {ok, Resp} + ); + {identity, Ranges} when + is_list(Ranges) andalso length(Ranges) < 10 + -> + send_ranges_multipart(Req, Type, Len, Att, Ranges); _ -> - Ranges = parse_ranges(MochiReq:get(range), Len), - case {Enc, Ranges} of - {identity, [{From, To}]} -> - Headers1 = - [{"Content-Range", make_content_range(From, To, Len)}] ++ - Headers, - {ok, Resp} = start_response_length( - Req, 206, Headers1, To - From + 1 - ), - couch_att:range_foldl( - Att, - From, - To + 1, - fun(Seg, _) -> send(Resp, Seg) end, - {ok, Resp} - ); - {identity, Ranges} when - is_list(Ranges) andalso length(Ranges) < 10 - -> - send_ranges_multipart(Req, Type, Len, Att, Ranges); - _ -> - Headers1 = - Headers ++ - if - Enc =:= identity orelse - ReqAcceptsAttEnc =:= true -> - [ - {"Content-MD5", - base64:encode( - couch_att:fetch(md5, Att) - )} - ]; - true -> - [] - end, - {ok, Resp} = start_response_length(Req, 200, Headers1, Len), - AttFun(Att, fun(Seg, _) -> send(Resp, Seg) end, {ok, Resp}) - end + Headers1 = + Headers ++ + if + Enc =:= identity orelse + ReqAcceptsAttEnc =:= true -> + [ + {"Content-MD5", + base64:encode( + couch_att:fetch(md5, Att) + )} + ]; + true -> + [] + end, + {ok, Resp} = start_response_length(Req, 200, Headers1, Len), + AttFun(Att, fun(Seg, _) -> send(Resp, Seg) end, {ok, Resp}) end - end - ) - after - demonitor_refs(Refs) + end end + ) + after + demonitor_refs(Refs) end; db_attachment_req(#httpd{method = Method} = Req, Db, DocId, FileNameParts) when (Method == 'PUT') or (Method == 'DELETE') @@ -1831,12 +1825,7 @@ db_attachment_req(#httpd{method = Method} = Req, Db, DocId, FileNameParts) when NewAtt = case Method of 'DELETE' -> - case get_attachment(Req, Db, DocId, FileName) of - {error, missing} -> - throw({not_found, "Document is missing attachment"}); - {ok, _, _} -> - [] - end; + []; _ -> MimeType = case chttpd:header_value(Req, "Content-Type") of @@ -1919,8 +1908,9 @@ db_attachment_req(#httpd{method = Method} = Req, Db, DocId, FileNameParts) when Method =/= 'DELETE' -> ok; true -> - % check for the existence of the doc to handle the 404 case. - couch_doc_open(Db, DocId, nil, []) + % check for the existence of the doc and attachment + CurrDoc = #doc{} = couch_doc_open(Db, DocId, nil, []), + get_existing_attachment(CurrDoc#doc.atts, FileName) end, fabric2_db:validate_docid(DocId), #doc{id = DocId}; @@ -1928,6 +1918,13 @@ db_attachment_req(#httpd{method = Method} = Req, Db, DocId, FileNameParts) when case fabric2_db:open_doc_revs(Db, DocId, [Rev], []) of {ok, [{ok, Doc0}]} -> chttpd_stats:incr_reads(), + if + Method =/= 'DELETE' -> + ok; + true -> + % check if attachment exists + get_existing_attachment(Doc0#doc.atts, FileName) + end, Doc0; {ok, [Error]} -> throw(Error); diff --git a/test/elixir/test/attachments_test.exs b/test/elixir/test/attachments_test.exs index 1f7c27ada..3c48e4452 100644 --- a/test/elixir/test/attachments_test.exs +++ b/test/elixir/test/attachments_test.exs @@ -107,9 +107,13 @@ defmodule AttachmentsTest do rev = resp.body["rev"] resp = Couch.delete("/#{db_name}/bin_doc/foo.txt", query: %{w: 3}) + assert resp.status_code == 409 + resp = Couch.delete("/#{db_name}/bin_doc/foo.txt", query: %{w: 3, rev: "4-garbage"}) assert resp.status_code == 409 - + assert resp.body["error"] == "not_found" + assert resp.body["reason"] == "missing_rev" + resp = Couch.delete("/#{db_name}/bin_doc/notexisting.txt", query: %{w: 3, rev: rev}) assert resp.status_code == 404 assert resp.body["error"] == "not_found" |