iilyak commented on a change in pull request #1176: Implement pluggable 
authentication and session support for replicator
URL: https://github.com/apache/couchdb/pull/1176#discussion_r169178743

 File path: src/couch_replicator/src/couch_replicator_utils.erl
 @@ -174,3 +175,93 @@ filter_state(State, States, Info) ->
         false ->
+remove_basic_auth_from_headers(Headers) ->
+    Headers1 = mochiweb_headers:make(Headers),
+    case mochiweb_headers:get_value("Authorization", Headers1) of
+        undefined ->
+            {{undefined, undefined}, Headers};
+        Auth when length(Auth) =< 6 ->
 Review comment:
   What is the rationale for this approach. It seems rather complex. Here are 
the possible downsides of the code as it is written:
   * use of magic number 6 (I got it is the lenght("Basic ") but I have to stop 
for a minute)
   * unnecessary use of length/1
   * subsequent use of lists:split/2
   * what if we decide to use other types (Bearer or Digest or something else)
   If I understand the structure should be always `Authorization: <type> 
   This means that the same could be written as:
   Auth ->
       case lists:splitwith(fun(X) -> X =/= $\s end, Auth) of
           {AType, " " ++ Base64} ->
               maybe_remove_basic_auth(string:to_lower(AType), Base64, Headers1)
   OR if we use binaries 
   case binary:split(?l2b(Auth), <<" ">>) of
      {AType, Base64} ->
          maybe_remove_basic_auth(string:to_lower(?b2l(AType)), Base64, 

This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:

With regards,
Apache Git Services

Reply via email to