From 9aaadfcc448b40a6cf3bc567b5949c4ee6ac04a2 Mon Sep 17 00:00:00 2001 From: Ahmed Shafeeq Bin Mohd Shariff Date: Sun, 18 Sep 2016 12:14:08 -0500 Subject: Update behavior of httpc:request to match RFC-7231 - The behavior of httpc:request when autoredirect = true is not correct according to the latest update in RFC-7231. This patch corrects the autoredirect behavior. --- lib/inets/src/http_client/httpc_response.erl | 35 +++++++++++++++------------- lib/inets/test/httpc_SUITE.erl | 6 ++--- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/lib/inets/src/http_client/httpc_response.erl b/lib/inets/src/http_client/httpc_response.erl index 91256fa6a2..d8bdac24e3 100644 --- a/lib/inets/src/http_client/httpc_response.erl +++ b/lib/inets/src/http_client/httpc_response.erl @@ -109,28 +109,31 @@ result(Response = {{_, 300, _}, _, _}, true}}) -> redirect(Response, Request); +result(Response = {{_, Code, _}, _, _}, + Request = #request{settings = + #http_options{autoredirect = true}, + method = post}) when (Code =:= 301) orelse + (Code =:= 302) orelse + (Code =:= 303) -> + redirect(Response, Request#request{method = get}); +result(Response = {{_, Code, _}, _, _}, + Request = #request{settings = + #http_options{autoredirect = true}, + method = post}) when (Code =:= 307) -> + redirect(Response, Request); result(Response = {{_, Code, _}, _, _}, Request = #request{settings = #http_options{autoredirect = true}, - method = head}) when (Code =:= 301) orelse + method = Method}) when (Code =:= 301) orelse (Code =:= 302) orelse (Code =:= 303) orelse (Code =:= 307) -> - redirect(Response, Request); -result(Response = {{_, Code, _}, _, _}, - Request = #request{settings = - #http_options{autoredirect = true}, - method = get}) when (Code =:= 301) orelse - (Code =:= 302) orelse - (Code =:= 303) orelse - (Code =:= 307) -> - redirect(Response, Request); -result(Response = {{_, 303, _}, _, _}, - Request = #request{settings = - #http_options{autoredirect = true}, - method = post}) -> - redirect(Response, Request#request{method = get}); - + case lists:member(Method, [get, head, options, trace]) of + true -> + redirect(Response, Request); + false -> + transparent(Response, Request) + end; result(Response = {{_,503,_}, _, _}, Request) -> status_service_unavailable(Response, Request); diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl index 932567ec55..dd09f2f8a4 100644 --- a/lib/inets/test/httpc_SUITE.erl +++ b/lib/inets/test/httpc_SUITE.erl @@ -514,7 +514,7 @@ redirect_moved_permanently(Config) when is_list(Config) -> {ok, {{_,200,_}, [_ | _], []}} = httpc:request(head, {URL301, []}, [], []), - {ok, {{_,301,_}, [_ | _], [_|_]}} + {ok, {{_,200,_}, [_ | _], [_|_]}} = httpc:request(post, {URL301, [],"text/plain", "foobar"}, [], []). %%------------------------------------------------------------------------- @@ -533,7 +533,7 @@ redirect_found(Config) when is_list(Config) -> {ok, {{_,200,_}, [_ | _], []}} = httpc:request(head, {URL302, []}, [], []), - {ok, {{_,302,_}, [_ | _], [_|_]}} + {ok, {{_,200,_}, [_ | _], [_|_]}} = httpc:request(post, {URL302, [],"text/plain", "foobar"}, [], []). %%------------------------------------------------------------------------- @@ -570,7 +570,7 @@ redirect_temporary_redirect(Config) when is_list(Config) -> {ok, {{_,200,_}, [_ | _], []}} = httpc:request(head, {URL307, []}, [], []), - {ok, {{_,307,_}, [_ | _], [_|_]}} + {ok, {{_,200,_}, [_ | _], [_|_]}} = httpc:request(post, {URL307, [],"text/plain", "foobar"}, [], []). -- cgit v1.2.3 From 494f6335ffebd3efd3eb443f4a0bfdbb2702b8c4 Mon Sep 17 00:00:00 2001 From: Ahmed Shafeeq Bin Mohd Shariff Date: Tue, 27 Sep 2016 14:59:27 -0500 Subject: Update test case docs with RFC-7231 --- lib/inets/test/httpc_SUITE.erl | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/inets/test/httpc_SUITE.erl b/lib/inets/test/httpc_SUITE.erl index dd09f2f8a4..57da82c6ad 100644 --- a/lib/inets/test/httpc_SUITE.erl +++ b/lib/inets/test/httpc_SUITE.erl @@ -500,10 +500,11 @@ redirect_multiple_choises(Config) when is_list(Config) -> httpc:request(get, {URL300, []}, [{autoredirect, false}], []). %%------------------------------------------------------------------------- redirect_moved_permanently() -> - [{doc, "If the 301 status code is received in response to a request other " - "than GET or HEAD, the user agent MUST NOT automatically redirect the request " - "unless it can be confirmed by the user, since this might change " - "the conditions under which the request was issued."}]. + [{doc, "The server SHOULD generate a Location header field in the response " + "containing a preferred URI reference for the new permanent URI. The user " + "agent MAY use the Location field value for automatic redirection. The server's " + "response payload usually contains a short hypertext note with a " + "hyperlink to the new URI(s)."}]. redirect_moved_permanently(Config) when is_list(Config) -> URL301 = url(group_name(Config), "/301.html", Config), @@ -519,10 +520,11 @@ redirect_moved_permanently(Config) when is_list(Config) -> [], []). %%------------------------------------------------------------------------- redirect_found() -> - [{doc," If the 302 status code is received in response to a request other " - "than GET or HEAD, the user agent MUST NOT automatically redirect the " - "request unless it can be confirmed by the user, since this might change " - "the conditions under which the request was issued."}]. + [{doc, "The server SHOULD generate a Location header field in the response " + "containing a URI reference for the different URI. The user agent MAY " + "use the Location field value for automatic redirection. The server's " + "response payload usually contains a short hypertext note with a " + "hyperlink to the different URI(s)."}]. redirect_found(Config) when is_list(Config) -> URL302 = url(group_name(Config), "/302.html", Config), @@ -540,7 +542,7 @@ redirect_found(Config) when is_list(Config) -> redirect_see_other() -> [{doc, "The different URI SHOULD be given by the Location field in the response. " "Unless the request method was HEAD, the entity of the response SHOULD contain a short " - "hypertext note with a hyperlink to the new URI(s). "}]. + "hypertext note with a hyperlink to the new URI(s)."}]. redirect_see_other(Config) when is_list(Config) -> URL303 = url(group_name(Config), "/303.html", Config), @@ -556,10 +558,11 @@ redirect_see_other(Config) when is_list(Config) -> [], []). %%------------------------------------------------------------------------- redirect_temporary_redirect() -> - [{doc," If the 307 status code is received in response to a request other " - "than GET or HEAD, the user agent MUST NOT automatically redirect the request " - "unless it can be confirmed by the user, since this might change " - "the conditions under which the request was issued."}]. + [{doc, "The server SHOULD generate a Location header field in the response " + "containing a URI reference for the different URI. The user agent MAY " + "use the Location field value for automatic redirection. The server's " + "response payload usually contains a short hypertext note with a " + "hyperlink to the different URI(s)."}]. redirect_temporary_redirect(Config) when is_list(Config) -> URL307 = url(group_name(Config), "/307.html", Config), -- cgit v1.2.3