[couchdb] branch prevent-multiple-responses-rebased created (now 2e32860)

[va...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

vatamane pushed a change to branch prevent-multiple-responses-rebased
in repository https://gitbox.apache.org/repos/asf/couchdb.git.


      at 2e32860  Ensure no more than one response per request

This branch includes the following new commits:

     new 2e32860  Ensure no more than one response per request

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

[couchdb] 01/01: Ensure no more than one response per request

[va...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

vatamane pushed a commit to branch prevent-multiple-responses-rebased
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 2e3286033a5457c5db7f854a68806d274d1f3822
Author: Robert Newson <rn...@apache.org>
AuthorDate: Tue May 11 18:20:20 2021 -0400

    Ensure no more than one response per request
    
    Due to a retry loop in erlfb:transactional couchdb might try to send
    multiple http responses for a single request which is clearly an
    error.
    
    This PR ensures the second attempt is prevented, closing the TCP
    socket instead.
---
 src/chttpd/src/chttpd.erl                       |  3 +++
 src/chttpd/test/eunit/chttpd_handlers_tests.erl |  9 +++++++++
 src/couch/src/couch_httpd.erl                   | 17 +++++++++++++++++
 3 files changed, 29 insertions(+)

diff --git a/src/chttpd/src/chttpd.erl b/src/chttpd/src/chttpd.erl
index 53fdf98..ba26b2b 100644
--- a/src/chttpd/src/chttpd.erl
+++ b/src/chttpd/src/chttpd.erl
@@ -160,6 +160,7 @@ stop() ->
     mochiweb_http:stop(?MODULE).
 
 handle_request(MochiReq0) ->
+    couch_httpd:response_not_started(),
     MochiReq = couch_httpd_vhost:dispatch_host(MochiReq0),
     handle_request_int(MochiReq).
 
@@ -1330,8 +1331,10 @@ handle_response(Req0, Code0, Headers0, Args0, Type) ->
     respond_(Req1, Code1, Headers1, Args1, Type).
 
 respond_(#httpd{mochi_req = MochiReq}, Code, Headers, _Args, start_response) ->
+    couch_httpd:abort_if_response_already_started(),
     MochiReq:start_response({Code, Headers});
 respond_(#httpd{mochi_req = MochiReq}, Code, Headers, Args, Type) ->
+    couch_httpd:abort_if_response_already_started(),
     MochiReq:Type({Code, Headers, Args}).
 
 get_user(#httpd{user_ctx = #user_ctx{name = null}}) ->
diff --git a/src/chttpd/test/eunit/chttpd_handlers_tests.erl b/src/chttpd/test/eunit/chttpd_handlers_tests.erl
index 649d82e..489ff67 100644
--- a/src/chttpd/test/eunit/chttpd_handlers_tests.erl
+++ b/src/chttpd/test/eunit/chttpd_handlers_tests.erl
@@ -63,6 +63,15 @@ should_escape_dbname_on_replicate(Url) ->
         end).
 
 
+prevent_multiple_http_response_test() ->
+    ?assertEqual(undefined, couch_httpd:abort_if_response_already_started()),
+    ?assertEqual(true, couch_httpd:response_not_started()),
+    ?assertEqual(undefined, couch_httpd:abort_if_response_already_started()),
+    Err = multiple_responses_attempted,
+    ?assertThrow({http_abort, {mochiweb_response, _}, Err},
+        couch_httpd:abort_if_response_already_started()).
+
+
 json_value(JSON, Keys) ->
     couch_util:get_nested_json_value(JSON, Keys).
 
diff --git a/src/couch/src/couch_httpd.erl b/src/couch/src/couch_httpd.erl
index fd83c25..b90d6da 100644
--- a/src/couch/src/couch_httpd.erl
+++ b/src/couch/src/couch_httpd.erl
@@ -36,6 +36,7 @@
 -export([validate_bind_address/1]).
 -export([check_max_request_length/1]).
 -export([maybe_decompress/2]).
+-export([response_not_started/0, abort_if_response_already_started/0]).
 
 -define(HANDLER_NAME_IN_MODULE_POS, 6).
 -define(MAX_DRAIN_BYTES, 1048576).
@@ -871,6 +872,7 @@ respond_(#httpd{mochi_req = MochiReq} = Req, Code, Headers, Args, Type) ->
     end.
 
 http_respond_(#httpd{mochi_req = MochiReq}, Code, Headers, _Args, start_response) ->
+    abort_if_response_already_started(),
     MochiReq:start_response({Code, Headers});
 http_respond_(#httpd{mochi_req = MochiReq}, 413, Headers, Args, Type) ->
     % Special handling for the 413 response. Make sure the socket is closed as
@@ -885,9 +887,24 @@ http_respond_(#httpd{mochi_req = MochiReq}, 413, Headers, Args, Type) ->
     mochiweb_socket:recv(Socket, ?MAX_DRAIN_BYTES, ?MAX_DRAIN_TIME_MSEC),
     Result;
 http_respond_(#httpd{mochi_req = MochiReq}, Code, Headers, Args, Type) ->
+    abort_if_response_already_started(),
     MochiReq:Type({Code, Headers, Args}).
 
 
+response_not_started() ->
+    erase(http_response_started).
+
+
+abort_if_response_already_started() ->
+    case get(http_response_started) of
+        undefined ->
+            put(http_response_started, true);
+        true ->
+            ErrResp = mochiweb:new_response({nil, 500, []}),
+            throw({http_abort, ErrResp, multiple_responses_attempted})
+    end.
+
+
 %%%%%%%% module tests below %%%%%%%%
 
 -ifdef(TEST).