You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by rn...@apache.org on 2020/10/09 17:27:54 UTC

[couchdb-ibrowse] branch rebased-master created (now 826843a)

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

rnewson pushed a change to branch rebased-master
in repository https://gitbox.apache.org/repos/asf/couchdb-ibrowse.git.


      at 826843a  Strip sensitive data from state

This branch includes the following new commits:

     new 2813d18  Get a new lb pid if existing pid is not alive
     new 826843a  Strip sensitive data from state

The 2 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-ibrowse] 01/02: Get a new lb pid if existing pid is not alive

Posted by rn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

rnewson pushed a commit to branch rebased-master
in repository https://gitbox.apache.org/repos/asf/couchdb-ibrowse.git

commit 2813d1873947bdb73c3fc0dec372b5ea7e36baa6
Author: Jay Doane <ja...@apache.org>
AuthorDate: Mon Apr 8 23:32:52 2019 -0700

    Get a new lb pid if existing pid is not alive
    
    It's possible for the connection process associated with a pid in the
    ibrowse_lb ets table to die, yet remain in the table, in which case
    subsequent requests to the corresponding {Host, Port} will result in an
    error like the following:
    
    (node1@127.0.0.1)9> ibrowse:send_req("http://localhost:15984", [], get).
                    ** exception exit: {noproc,
                           {gen_server,call,
                               [<0.2451.0>,
                                {spawn_connection,
                                    {url,"http://localhost:15984","localhost",15984,
                                        undefined,undefined,"/",http,hostname},
                                    10,10,
                                    {[],false},
                                    []}]}}
         in function  gen_server:call/2 (gen_server.erl, line 215)
         in call from ibrowse:try_routing_request/14 (src/ibrowse.erl, line 377)
    
    This checks whether the pid about to be returned from the table is alive,
    and if not, the entry is deleted, and a new pid is obtained.
---
 src/ibrowse.erl       | 21 +++++++++++++++------
 test/ibrowse_test.erl | 22 ++++++++++++++++++++--
 2 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/src/ibrowse.erl b/src/ibrowse.erl
index 2066799..5adf047 100644
--- a/src/ibrowse.erl
+++ b/src/ibrowse.erl
@@ -342,12 +342,7 @@ send_req(Url, Headers, Method, Body, Options, Timeout) ->
         #url{host = Host,
              port = Port,
              protocol = Protocol} = Parsed_url ->
-            Lb_pid = case ets:lookup(ibrowse_lb, {Host, Port}) of
-                         [] ->
-                             get_lb_pid(Parsed_url);
-                         [#lb_pid{pid = Lb_pid_1}] ->
-                             Lb_pid_1
-                     end,
+            Lb_pid = lb_pid(Host, Port, Parsed_url),
             Max_sessions = get_max_sessions(Host, Port, Options),
             Max_pipeline_size = get_max_pipeline_size(Host, Port, Options),
             Max_attempts = get_max_attempts(Host, Port, Options),
@@ -367,6 +362,20 @@ send_req(Url, Headers, Method, Body, Options, Timeout) ->
             {error, {url_parsing_failed, Err}}
     end.
 
+lb_pid(Host, Port, Url) ->
+    case ets:lookup(ibrowse_lb, {Host, Port}) of
+        [] ->
+            get_lb_pid(Url);
+        [#lb_pid{pid = Pid}] ->
+            case is_process_alive(Pid) of
+                true ->
+                    Pid;
+                false ->
+                    ets:delete(ibrowse_lb, {Host, Port}),
+                    get_lb_pid(Url)
+            end
+    end.
+
 try_routing_request(Lb_pid, Parsed_url,
                     Max_sessions, 
                     Max_pipeline_size,
diff --git a/test/ibrowse_test.erl b/test/ibrowse_test.erl
index cd78049..fc2e4af 100644
--- a/test/ibrowse_test.erl
+++ b/test/ibrowse_test.erl
@@ -40,7 +40,8 @@
 	 test_save_to_file_no_content_length/0,
          socks5_noauth/0,
          socks5_auth_succ/0,
-         socks5_auth_fail/0
+         socks5_auth_fail/0,
+         test_dead_lb_pid/0
 	]).
 
 -include_lib("ibrowse/include/ibrowse.hrl").
@@ -64,7 +65,8 @@
                       {local_test_fun, test_retry_of_requests, []},
 		      {local_test_fun, verify_chunked_streaming, []},
 		      {local_test_fun, test_chunked_streaming_once, []},
-		      {local_test_fun, test_generate_body_0, []}
+		      {local_test_fun, test_generate_body_0, []},
+		      {local_test_fun, test_dead_lb_pid, []}
                      ]).
 
 -define(TEST_LIST, [{"http://intranet/messenger", get},
@@ -877,6 +879,22 @@ test_generate_body_0() ->
         ets:delete(Tid)
     end.
 
+%% Test that when an lb process dies, its entry is removed from the ibrowse_lb
+%% table by the next requestor and replaced with a new process
+%%------------------------------------------------------------------------------
+test_dead_lb_pid() ->
+    {Host, Port} = {"localhost", 8181},
+    Url = "http://" ++ Host ++ ":" ++ integer_to_list(Port),
+    {ok, "200", _, _} = ibrowse:send_req(Url, [], get),
+    [{lb_pid, {Host, Port}, Pid}] = ets:lookup(ibrowse_lb, {Host, Port}),
+    true = exit(Pid, kill),
+    false = is_process_alive(Pid),
+    {ok, "200", _, _} = ibrowse:send_req(Url, [], get),
+    [{lb_pid, {Host, Port}, NewPid}] = ets:lookup(ibrowse_lb, {Host, Port}),
+    true = NewPid /= Pid,
+    true = is_process_alive(NewPid),
+    success.
+
 do_trace(Fmt, Args) ->
     do_trace(get(my_trace_flag), Fmt, Args).
 


[couchdb-ibrowse] 02/02: Strip sensitive data from state

Posted by rn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

rnewson pushed a commit to branch rebased-master
in repository https://gitbox.apache.org/repos/asf/couchdb-ibrowse.git

commit 826843a7be8dcf59b1cb5ab1f11846855b863aa8
Author: ILYA Khlopotov <ii...@apache.org>
AuthorDate: Thu Jul 23 08:15:02 2020 -0700

    Strip sensitive data from state
---
 src/ibrowse_http_client.erl | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/src/ibrowse_http_client.erl b/src/ibrowse_http_client.erl
index e59228c..df8eb93 100644
--- a/src/ibrowse_http_client.erl
+++ b/src/ibrowse_http_client.erl
@@ -33,7 +33,8 @@
          handle_cast/2,
          handle_info/2,
          terminate/2,
-         code_change/3
+         code_change/3,
+         format_status/2
         ]).
 
 -include("ibrowse.hrl").
@@ -321,10 +322,49 @@ terminate(_Reason, #state{lb_ets_tid = Tid} = State) ->
 code_change(_OldVsn, State, _Extra) ->
     {ok, State}.
 
+
+%%--------------------------------------------------------------------
+%% Function: format_status/2
+%% Purpose: Clean process state before logging
+%% Returns: key value list
+%%--------------------------------------------------------------------
+format_status(_Opt, [_PDict, State]) ->
+    #state{
+        reqs=Reqs,
+        reply_buffer=ReplyBuf,
+        recvd_headers=RCVDHeaders,
+        raw_headers=RawHeaders,
+        chunk_size_buffer=ChunkSizeBuf,
+        cur_req=Request
+    } = State,
+    ScrubbedReq = Request#request{url=url_strip_password(Request#request.url)},
+    Scrubbed = State#state{
+        socks5_user=nil,
+        socks5_password=nil,
+        reqs={queue_length, queue:len(Reqs)},
+        reply_buffer={byte_size, byte_size(ReplyBuf)},
+        recvd_headers=lists:map(fun({K, _V}) -> K end, RCVDHeaders),
+        raw_headers={byte_size, byte_size(RawHeaders)},
+        chunk_size_buffer={byte_size, byte_size(ChunkSizeBuf)},
+        cur_req=ScrubbedReq
+    },
+    [{data, [{"State",
+        lists:zip(
+            record_info(fields, state),
+            tl(tuple_to_list(Scrubbed))
+        )
+    }]}].
+
 %%--------------------------------------------------------------------
 %%% Internal functions
 %%--------------------------------------------------------------------
 
+url_strip_password(Url) ->
+    re:replace(Url,
+        "(http|https|socks5)://([^:]+):[^@]+@(.*)$",
+        "\\1://\\2:*****@\\3",
+        [{return, list}]).
+
 %%--------------------------------------------------------------------
 %% Handles data recvd on the socket
 %%--------------------------------------------------------------------