Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] HTTP 301 handling #312

Open
Choochmeque opened this issue Nov 11, 2022 · 9 comments
Open

[Feature Request] HTTP 301 handling #312

Choochmeque opened this issue Nov 11, 2022 · 9 comments

Comments

@Choochmeque
Copy link

Choochmeque commented Nov 11, 2022

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

@RomanHargrave
Copy link
Contributor

In what module?

@badlop
Copy link
Member

badlop commented Jan 24, 2023

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

@Choochmeque
Copy link
Author

@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
Copy link
Member

badlop commented Jan 30, 2023

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
Copy link
Author

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

@badlop
Copy link
Member

badlop commented Mar 28, 2023

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

@Neustradamus
Copy link

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

@Neustradamus
Copy link

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

@Neustradamus
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants