This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/couchdb.git
The following commit(s) were added to refs/heads/main by this push:
new bf355b9 Port attachment deletion fix from 3.x
bf355b9 is described below
commit bf355b96ee3efc24cb6edfb3eeb5b6fe519fdbf8
Author: Nick Vatamaniuc <vatam...@gmail.com>
AuthorDate: Fri Jul 30 17:05:29 2021 -0400
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
---
src/chttpd/src/chttpd_db.erl | 271 +++++++++++++++++-----------------
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 41bb741..9edf243 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 1f7c27a..3c48e44 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"
