[Feature Request] HTTP 301 handling

[Choochmeque]

It would be great to have redirect handling support in the ejabberd_auth_http.

[RomanHargrave]

In what module?

[badlop]

@Choochmeque : can you provide more details about your feature request?

[Choochmeque]

@RomanHargrave , @badlop , sorry, somehow I missed notifications.

I meant, ejabberd_auth_http module. For example Django wants to have / at the end of url. For urls without /, Django returns http 301. So it would be cool if mod_auth_http would handle this http response.

[badlop]

I wrote a draft patch. It isn't tested in real-world conditions, that's your duty ;)

Please report your results

diff --git a/ejabberd_auth_http/src/ejabberd_auth_http.erl b/ejabberd_auth_http/src/ejabberd_auth_http.erl
index c3b1b64..8389b67 100644
--- a/ejabberd_auth_http/src/ejabberd_auth_http.erl
+++ b/ejabberd_auth_http/src/ejabberd_auth_http.erl
@@ -213,13 +213,22 @@ make_req(Method, Path, LUser, LServer, Password) ->
     Connection = cuesport:get_worker(existing_pool_name(LServer)),
 
     ?DEBUG("Making request '~s' for user ~s@~s...", [Path, LUser, LServer]),
-    {ok, {{Code, _Reason}, _RespHeaders, RespBody, _, _}} = case Method of
-        get -> fusco:request(Connection, <<PathPrefix/binary, Path/binary, "?", Query/binary>>,
-                             "GET", Header, "", 2, 5000);
-        post -> fusco:request(Connection, <<PathPrefix/binary, Path/binary>>,
-                              "POST", [ContentType|Header], Query, 2, 5000)
-    end,
+    {Url, MethodStr, Headers, Query2} =
+        case Method of
+            get -> {<<PathPrefix/binary, Path/binary, "?", Query/binary>>,
+                    "GET",
+                    Header,
+                    ""};
+            post -> {<<PathPrefix/binary, Path/binary>>,
+                     "POST",
+                     [ContentType|Header],
+                     Query}
+        end,
+    http_request(Connection, Url, MethodStr, Headers, Query2, 0).
 
+http_request(Connection, Url, MethodStr, Headers, Query, RedirectCounter) ->
+    {ok, {{Code, _Reason}, RespHeaders, RespBody, _, _}} =
+        fusco:request(Connection, Url, MethodStr, Headers, Query, 2, 5000),
     ?DEBUG("Request result: ~s: ~p", [Code, RespBody]),
     case Code of
         <<"409">> -> {error, conflict};
@@ -231,9 +240,18 @@ make_req(Method, Path, LUser, LServer, Password) ->
         <<"204">> -> {ok, <<"">>};
         <<"201">> -> {ok, <<"created">>};
         <<"200">> -> {ok, RespBody};
+        R when (R==<<"301">>) or (R==<<"307">>) or (R==<<"308">>) ->
+            handle_redirect(RespHeaders, Connection, MethodStr, Headers, Query, RedirectCounter+1);
         _ -> {error, RespBody}
     end.
 
+handle_redirect(RespHeaders, Connection, MethodStr, Headers, Query, RedirectCounter)
+      when RedirectCounter < 5 ->
+    {_, Location} = lists:keyfind(<<"location">>, 1, RespHeaders),
+    http_request(Connection, Location, MethodStr, Headers, Query, RedirectCounter);
+handle_redirect(_, _, _, _, _, _) ->
+    {error, redirect_loop}.
+
 %%%----------------------------------------------------------------------
 %%% Other internal functions
 %%%----------------------------------------------------------------------

[Choochmeque]

That's cool! Will try ASAP and let you know...

[badlop]

Hi, were you able to test it? Did it work correctly, with no drawbacks?

[Neustradamus]

@Choochmeque: Have you tried? What is the result?

[Neustradamus]

@Choochmeque: Have you tried the @badlop patch? What is the result?

[Neustradamus]

@Choochmeque: We are in 2024, have you tried the @badlop patch? What is the result?

[Neustradamus]

@badlop: Not possible to add your patch in upstream?