Merge pull request #486 from cloudant/COUCHDB-3362-delete-att-on-nonexistdoc

Avoid creation of document if deleting attachment on non-existent doc

3 files changed, 91 insertions, 1 deletions

diff --git a/src/chttpd/src/chttpd_db.erl b/src/chttpd/src/chttpd_db.erl
index 37e466908..902b5b95b 100644
--- a/src/chttpd/src/chttpd_db.erl
+++ b/src/chttpd/src/chttpd_db.erl
@@ -1236,6 +1236,10 @@ db_attachment_req(#httpd{method=Method, user_ctx=Ctx}=Req, Db, DocId, FileNamePa
 
     Doc = case extract_header_rev(Req, chttpd:qs_value(Req, "rev")) of
         missing_rev -> % make the new doc
+            if Method =/= 'DELETE' -> ok; true ->
+                % check for the existence of the doc to handle the 404 case.
+                couch_doc_open(Db, DocId, nil, [])
+            end,
             couch_doc:validate_docid(DocId),
             #doc{id=DocId};
         Rev ->
diff --git a/src/chttpd/test/chttpd_db_test.erl b/src/chttpd/test/chttpd_db_test.erl
index be091f9c3..b7ea7f006 100644
--- a/src/chttpd/test/chttpd_db_test.erl
+++ b/src/chttpd/test/chttpd_db_test.erl
@@ -19,6 +19,7 @@
 -define(PASS, "pass").
 -define(AUTH, {basic_auth, {?USER, ?PASS}}).
 -define(CONTENT_JSON, {"Content-Type", "application/json"}).
+-define(FIXTURE_TXT, ?ABS_PATH(?FILE)).
 
 setup() ->
     ok = config:set("admins", ?USER, ?PASS, _Persist=false),
@@ -56,7 +57,10 @@ all_test_() ->
                 fun setup/0, fun teardown/1,
                 [
                     fun should_return_ok_true_on_bulk_update/1,
-                    fun should_accept_live_as_an_alias_for_continuous/1
+                    fun should_accept_live_as_an_alias_for_continuous/1,
+                    fun should_return_404_for_delete_att_on_notadoc/1,
+                    fun should_return_409_for_del_att_without_rev/1,
+                    fun should_return_200_for_del_att_with_rev/1
                 ]
             }
         }
@@ -97,3 +101,81 @@ should_accept_live_as_an_alias_for_continuous(Url) ->
 
         ?assertEqual(LastSeqNum + 1, SeqNum)
     end).
+
+
+should_return_404_for_delete_att_on_notadoc(Url) ->
+    ?_test(begin
+        {ok, RC, _, RespBody} = test_request:delete(
+            Url ++ "/notadoc/att.pdf",
+            [?CONTENT_JSON, ?AUTH],
+            []
+        ),
+        ?assertEqual(404, RC),
+        ?assertEqual(
+          {[{<<"error">>,<<"not_found">>},
+            {<<"reason">>,<<"missing">>}]},
+          jiffy:decode(RespBody)
+        ),
+        {ok, RC1, _, _} = test_request:get(
+            Url ++ "/notadoc",
+            [?CONTENT_JSON, ?AUTH],
+            []
+        ),
+        ?assertEqual(404, RC1)
+    end).
+
+
+should_return_409_for_del_att_without_rev(Url) ->
+    ?_test(begin
+        {ok, Data} = file:read_file(?FIXTURE_TXT),
+        Doc = {[
+            {<<"_attachments">>, {[
+                {<<"file.erl">>, {[
+                    {<<"content_type">>, <<"text/plain">>},
+                    {<<"data">>, base64:encode(Data)}
+                ]}
+            }]}}
+        ]},
+        {ok, RC, _, _} = test_request:put(
+            Url ++ "/testdoc3",
+            [?CONTENT_JSON, ?AUTH],
+            jiffy:encode(Doc)
+        ),
+        ?assertEqual(201, RC),
+
+        {ok, RC1, _, _} = test_request:delete(
+            Url ++ "/testdoc3/file.erl",
+            [?CONTENT_JSON, ?AUTH],
+            []
+        ),
+        ?assertEqual(409, RC1)
+    end).
+
+should_return_200_for_del_att_with_rev(Url) ->
+  ?_test(begin
+      {ok, Data} = file:read_file(?FIXTURE_TXT),
+      Doc = {[
+          {<<"_attachments">>, {[
+              {<<"file.erl">>, {[
+                  {<<"content_type">>, <<"text/plain">>},
+                  {<<"data">>, base64:encode(Data)}
+              ]}
+          }]}}
+      ]},
+      {ok, RC, _Headers, RespBody} = test_request:put(
+          Url ++ "/testdoc4",
+          [?CONTENT_JSON, ?AUTH],
+          jiffy:encode(Doc)
+      ),
+      ?assertEqual(201, RC),
+
+      {ResultJson} = ?JSON_DECODE(RespBody),
+      Rev = couch_util:get_value(<<"rev">>, ResultJson, undefined),
+
+      {ok, RC1, _, _} = test_request:delete(
+          Url ++ "/testdoc4/file.erl?rev=" ++ Rev,
+          [?CONTENT_JSON, ?AUTH],
+          []
+      ),
+      ?assertEqual(200, RC1)
+    end).
diff --git a/src/couch/src/couch_httpd_db.erl b/src/couch/src/couch_httpd_db.erl
index e1af1bfdc..a6d83d619 100644
--- a/src/couch/src/couch_httpd_db.erl
+++ b/src/couch/src/couch_httpd_db.erl
@@ -1013,6 +1013,10 @@ db_attachment_req(#httpd{method=Method,mochi_req=MochiReq}=Req, Db, DocId, FileN
 
     Doc = case extract_header_rev(Req, couch_httpd:qs_value(Req, "rev")) of
         missing_rev -> % make the new doc
+            if Method =/= 'DELETE' -> ok; true ->
+                % check for the existence of the doc to handle the 404 case.
+                couch_doc_open(Db, DocId, nil, [])
+            end,
             couch_doc:validate_docid(DocId),
             #doc{id=DocId};
         Rev ->