processone / ejabberd-contrib

Growing and curated ejabberd contributions repository - PR or ask to join !
http://ejabberd.im
248 stars 137 forks source link

[Feature Request] HTTP 301 handling #312

Open Choochmeque opened 1 year ago

Choochmeque commented 1 year ago

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

RomanHargrave commented 1 year ago

In what module?

badlop commented 1 year ago

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

Choochmeque commented 1 year ago

@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 commented 1 year ago

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

Please report your results

```diff 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, <>, - "GET", Header, "", 2, 5000); - post -> fusco:request(Connection, <>, - "POST", [ContentType|Header], Query, 2, 5000) - end, + {Url, MethodStr, Headers, Query2} = + case Method of + get -> {<>, + "GET", + Header, + ""}; + post -> {<>, + "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 commented 1 year ago

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

badlop commented 1 year ago

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

Neustradamus commented 1 year ago

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

Neustradamus commented 1 year ago

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

Neustradamus commented 6 months ago

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