Add tests for charsets_provided

Fix cases where the q-value is 0 and where a wildcard
was sent in the accept-charset header.

Also don't send a charset in the content-type of the
response if the media type is not text.

Thanks to Philip Witty for help figuring this out.

doc/src/manual/cowboy_rest.asciidoc

@@ -175,6 +175,9 @@ req() :: #{
 }
 ----
 
+Note that Cowboy will only append the charset to the
+content-type header of the response if the media type is text.
+
 === content_types_accepted
 
 [source,erlang]

src/cowboy_rest.erl

@@ -680,11 +680,16 @@ prioritize_charsets(AcceptCharsets) ->
 
 choose_charset(Req, State, []) ->
 	not_acceptable(Req, State);
+%% A q-value of 0 means not acceptable.
+choose_charset(Req, State, [{_, 0}|Tail]) ->
+	choose_charset(Req, State, Tail);
 choose_charset(Req, State=#state{charsets_p=CP}, [Charset|Tail]) ->
 	match_charset(Req, State, Tail, CP, Charset).
 
 match_charset(Req, State, Accept, [], _Charset) ->
 	choose_charset(Req, State, Accept);
+match_charset(Req, State, _Accept, [Provided|_], {<<"*">>, _}) ->
+	set_content_type(Req, State#state{charset_a=Provided});
 match_charset(Req, State, _Accept, [Provided|_], {Provided, _}) ->
 	set_content_type(Req, State#state{charset_a=Provided});
 match_charset(Req, State, Accept, [_|Tail], Charset) ->
@@ -695,9 +700,11 @@ set_content_type(Req, State=#state{
 		charset_a=Charset}) ->
 	ParamsBin = set_content_type_build_params(Params, []),
 	ContentType = [Type, <<"/">>, SubType, ParamsBin],
-	ContentType2 = case Charset of
-		undefined -> ContentType;
-		Charset -> [ContentType, <<"; charset=">>, Charset]
+	ContentType2 = case {Type, Charset} of
+		{<<"text">>, Charset} when Charset =/= undefined ->
+			[ContentType, <<"; charset=">>, Charset];
+		_ ->
+			ContentType
 	end,
 	Req2 = cowboy_req:set_resp_header(<<"content-type">>, ContentType2, Req),
 	encodings_provided(Req2#{charset => Charset}, State).

test/handlers/charsets_provided_empty_h.erl

@@ -0,0 +1,30 @@
+%% This module has a text and non-text media type,
+%% but provides no charset. All requests will result
+%% in a 406 not acceptable.
+
+-module(charsets_provided_empty_h).
+
+-export([init/2]).
+-export([content_types_provided/2]).
+-export([charsets_provided/2]).
+-export([get_text_plain/2]).
+-export([get_application_json/2]).
+
+init(Req, Opts) ->
+	{cowboy_rest, Req, Opts}.
+
+content_types_provided(Req, State) ->
+	{[
+		{{<<"text">>, <<"plain">>, []}, get_text_plain},
+		{{<<"application">>, <<"json">>, []}, get_application_json}
+	], Req, State}.
+
+charsets_provided(Req, State) ->
+	{[], Req, State}.
+
+get_text_plain(Req, State) ->
+	{<<"This is REST!">>, Req, State}.
+
+get_application_json(Req, State) ->
+	{<<"{\"hello\": \"rest\"}">>, Req, State}.
+

test/handlers/charsets_provided_h.erl

@@ -0,0 +1,28 @@
+%% This module has a text and non-text media type,
+%% and provides two charsets.
+
+-module(charsets_provided_h).
+
+-export([init/2]).
+-export([content_types_provided/2]).
+-export([charsets_provided/2]).
+-export([get_text_plain/2]).
+-export([get_application_json/2]).
+
+init(Req, Opts) ->
+	{cowboy_rest, Req, Opts}.
+
+content_types_provided(Req, State) ->
+	{[
+		{{<<"text">>, <<"plain">>, []}, get_text_plain},
+		{{<<"application">>, <<"json">>, []}, get_application_json}
+	], Req, State}.
+
+charsets_provided(Req, State) ->
+	{[<<"utf-8">>, <<"utf-16">>], Req, State}.
+
+get_text_plain(Req, State) ->
+	{<<"This is REST!">>, Req, State}.
+
+get_application_json(Req, State) ->
+	{<<"{\"hello\": \"rest\"}">>, Req, State}.

test/rest_handler_SUITE.erl

@@ -39,6 +39,8 @@ end_per_group(Name, _) ->
 init_dispatch(_) ->
 	cowboy_router:compile([{'_', [
 		{"/", rest_hello_h, []},
+		{"/charsets_provided", charsets_provided_h, []},
+		{"/charsets_provided_empty", charsets_provided_empty_h, []},
 		{"/charset_in_content_types_provided",
 			charset_in_content_types_provided_h, []},
 		{"/charset_in_content_types_provided_implicit",
@@ -148,6 +150,136 @@ charset_in_content_types_provided_implicit_no_callback(Config) ->
 	{_, <<"text/plain">>} = lists:keyfind(<<"content-type">>, 1, Headers),
 	ok.
 
+charsets_provided_match_text(Config) ->
+	doc("When the media type is text and the charsets_provided callback exists "
+		"and the accept-charset header was sent, the selected charset is sent "
+		"back in the content-type of the response."),
+	ConnPid = gun_open(Config),
+	Ref = gun:get(ConnPid, "/charsets_provided", [
+		{<<"accept">>, <<"text/plain">>},
+		{<<"accept-charset">>, <<"utf-8;q=0.5, utf-16">>},
+		{<<"accept-encoding">>, <<"gzip">>}
+	]),
+	{response, _, 200, Headers} = gun:await(ConnPid, Ref),
+	{_, <<"text/plain; charset=utf-16">>} = lists:keyfind(<<"content-type">>, 1, Headers),
+	ok.
+
+charsets_provided_match_other(Config) ->
+	doc("When the media type is not text and the charsets_provided callback exists "
+		"and the accept-charset header was sent, the selected charset is not sent "
+		"back in the content-type of the response."),
+	ConnPid = gun_open(Config),
+	Ref = gun:get(ConnPid, "/charsets_provided", [
+		{<<"accept">>, <<"application/json">>},
+		{<<"accept-charset">>, <<"utf-8;q=0.5, utf-16">>},
+		{<<"accept-encoding">>, <<"gzip">>}
+	]),
+	{response, _, 200, Headers} = gun:await(ConnPid, Ref),
+	{_, <<"application/json">>} = lists:keyfind(<<"content-type">>, 1, Headers),
+	ok.
+
+charsets_provided_wildcard_text(Config) ->
+	doc("When the media type is text and the charsets_provided callback exists "
+		"and a wildcard accept-charset header was sent, the selected charset is sent "
+		"back in the content-type of the response."),
+	ConnPid = gun_open(Config),
+	Ref = gun:get(ConnPid, "/charsets_provided", [
+		{<<"accept">>, <<"text/plain">>},
+		{<<"accept-charset">>, <<"*">>},
+		{<<"accept-encoding">>, <<"gzip">>}
+	]),
+	{response, _, 200, Headers} = gun:await(ConnPid, Ref),
+	{_, <<"text/plain; charset=utf-8">>} = lists:keyfind(<<"content-type">>, 1, Headers),
+	ok.
+
+charsets_provided_wildcard_other(Config) ->
+	doc("When the media type is not text and the charsets_provided callback exists "
+		"and a wildcard accept-charset header was sent, the selected charset is not sent "
+		"back in the content-type of the response."),
+	ConnPid = gun_open(Config),
+	Ref = gun:get(ConnPid, "/charsets_provided", [
+		{<<"accept">>, <<"application/json">>},
+		{<<"accept-charset">>, <<"*">>},
+		{<<"accept-encoding">>, <<"gzip">>}
+	]),
+	{response, _, 200, Headers} = gun:await(ConnPid, Ref),
+	{_, <<"application/json">>} = lists:keyfind(<<"content-type">>, 1, Headers),
+	ok.
+
+charsets_provided_nomatch(Config) ->
+	doc("Regardless of the media type negotiated, if no charset is found in the "
+		"accept-charset header match a charset configured in charsets_provided, "
+		"then a 406 not acceptable response is sent back."),
+	ConnPid = gun_open(Config),
+	Ref = gun:get(ConnPid, "/charsets_provided", [
+		{<<"accept">>, <<"text/plain">>},
+		{<<"accept-charset">>, <<"utf-8;q=0, iso-8859-1">>},
+		{<<"accept-encoding">>, <<"gzip">>}
+	]),
+	{response, _, 406, _} = gun:await(ConnPid, Ref),
+	ok.
+
+charsets_provided_noheader_text(Config) ->
+	doc("When the media type is text and the charsets_provided callback exists "
+		"but the accept-charset header was not sent, the first charset in the "
+		"list is selected and sent back in the content-type of the response."),
+	ConnPid = gun_open(Config),
+	Ref = gun:get(ConnPid, "/charsets_provided", [
+		{<<"accept">>, <<"text/plain">>},
+		{<<"accept-encoding">>, <<"gzip">>}
+	]),
+	{response, _, 200, Headers} = gun:await(ConnPid, Ref),
+	{_, <<"text/plain; charset=utf-8">>} = lists:keyfind(<<"content-type">>, 1, Headers),
+	ok.
+
+charsets_provided_noheader_other(Config) ->
+	doc("When the media type is not text and the charsets_provided callback exists "
+		"but the accept-charset header was not sent, the first charset in the "
+		"list is selected but is not sent back in the content-type of the response."),
+	ConnPid = gun_open(Config),
+	Ref = gun:get(ConnPid, "/charsets_provided", [
+		{<<"accept">>, <<"application/json">>},
+		{<<"accept-encoding">>, <<"gzip">>}
+	]),
+	{response, _, 200, Headers} = gun:await(ConnPid, Ref),
+	{_, <<"application/json">>} = lists:keyfind(<<"content-type">>, 1, Headers),
+	ok.
+
+charsets_provided_empty(Config) ->
+	doc("Regardless of the media type negotiated, if the charsets_provided "
+		"callback returns an empty list a 406 not acceptable response is sent back."),
+	ConnPid = gun_open(Config),
+	Ref = gun:get(ConnPid, "/charsets_provided_empty", [
+		{<<"accept">>, <<"text/plain">>},
+		{<<"accept-charset">>, <<"utf-8q=0.5, utf-16">>},
+		{<<"accept-encoding">>, <<"gzip">>}
+	]),
+	{response, _, 406, _} = gun:await(ConnPid, Ref),
+	ok.
+
+charsets_provided_empty_wildcard(Config) ->
+	doc("Regardless of the media type negotiated, if the charsets_provided "
+		"callback returns an empty list a 406 not acceptable response is sent back."),
+	ConnPid = gun_open(Config),
+	Ref = gun:get(ConnPid, "/charsets_provided_empty", [
+		{<<"accept">>, <<"text/plain">>},
+		{<<"accept-charset">>, <<"*">>},
+		{<<"accept-encoding">>, <<"gzip">>}
+	]),
+	{response, _, 406, _} = gun:await(ConnPid, Ref),
+	ok.
+
+charsets_provided_empty_noheader(Config) ->
+	doc("Regardless of the media type negotiated, if the charsets_provided "
+		"callback returns an empty list a 406 not acceptable response is sent back."),
+	ConnPid = gun_open(Config),
+	Ref = gun:get(ConnPid, "/charsets_provided_empty", [
+		{<<"accept">>, <<"text/plain">>},
+		{<<"accept-encoding">>, <<"gzip">>}
+	]),
+	{response, _, 406, _} = gun:await(ConnPid, Ref),
+	ok.
+
 provide_callback_missing(Config) ->
 	doc("A 500 response must be sent when the ProvideCallback can't be called."),
 	ConnPid = gun_open(Config),