Improve the cowboy_static consistency across platforms

As a result we explictly reject path_info components that include
a forward slash, backward slash or NUL character. This only applies
to the [...] part of the path for dir/priv_dir configuration.

Also improve the tests so that they work on Windows.

src/cowboy_static.erl

@@ -119,35 +119,51 @@ init_dir(Req, Path, HowToAccess, Extra) when is_list(Path) ->
 	init_dir(Req, list_to_binary(Path), HowToAccess, Extra);
 init_dir(Req, Path, HowToAccess, Extra) ->
 	Dir = fullpath(filename:absname(Path)),
-	PathInfo = cowboy_req:path_info(Req),
+	case cowboy_req:path_info(Req) of
-	Filepath = filename:join([Dir|escape_reserved(PathInfo)]),
+		%% When dir/priv_dir are used and there is no path_info
-	Len = byte_size(Dir),
+		%% this is a configuration error and we abort immediately.
-	case fullpath(Filepath) of
+		undefined ->
-		<< Dir:Len/binary, $/, _/binary >> ->
+			{ok, cowboy_req:reply(500, Req), error};
-			init_info(Req, Filepath, HowToAccess, Extra);
+		PathInfo ->
-		<< Dir:Len/binary >> ->
+			case validate_reserved(PathInfo) of
-			init_info(Req, Filepath, HowToAccess, Extra);
+				error ->
-		_ ->
+					{cowboy_rest, Req, error};
-			{cowboy_rest, Req, error}
+				ok ->
+					Filepath = filename:join([Dir|PathInfo]),
+					Len = byte_size(Dir),
+					case fullpath(Filepath) of
+						<< Dir:Len/binary, $/, _/binary >> ->
+							init_info(Req, Filepath, HowToAccess, Extra);
+						<< Dir:Len/binary >> ->
+							init_info(Req, Filepath, HowToAccess, Extra);
+						_ ->
+							{cowboy_rest, Req, error}
+					end
+			end
 	end.
 
-escape_reserved([]) -> [];
+validate_reserved([]) ->
-escape_reserved([P|Tail]) -> [escape_reserved(P, <<>>)|escape_reserved(Tail)].
+	ok;
+validate_reserved([P|Tail]) ->
+	case validate_reserved1(P) of
+		ok -> validate_reserved(Tail);
+		error -> error
+	end.
 
-%% We escape the slash found in path segments because
+%% We always reject forward slash, backward slash and NUL as
-%% a segment corresponds to a directory entry, and
+%% those have special meanings across the supported platforms.
-%% therefore those slashes are expected to be part of
+%% We could support the backward slash on some platforms but
-%% the directory name.
+%% for the sake of consistency and simplicity we don't.
-%%
+validate_reserved1(<<>>) ->
-%% Note that on most systems the slash is prohibited
+	ok;
-%% and cannot appear in filenames, which means the
+validate_reserved1(<<$/, _/bits>>) ->
-%% requested file will end up being not found.
+	error;
-escape_reserved(<<>>, Acc) ->
+validate_reserved1(<<$\\, _/bits>>) ->
-	Acc;
+	error;
-escape_reserved(<< $/, Rest/bits >>, Acc) ->
+validate_reserved1(<<0, _/bits>>) ->
-	escape_reserved(Rest, << Acc/binary, $\\, $/ >>);
+	error;
-escape_reserved(<< C, Rest/bits >>, Acc) ->
+validate_reserved1(<<_, Rest/bits>>) ->
-	escape_reserved(Rest, << Acc/binary, C >>).
+	validate_reserved1(Rest).
 
 fullpath(Path) ->
 	fullpath(filename:split(Path), []).
@@ -290,7 +306,7 @@ bad_path_win32_check_test_() ->
 -endif.
 
 %% Reject requests that tried to access a file outside
-%% the target directory.
+%% the target directory, or used reserved characters.
 
 -spec malformed_request(Req, State)
 	-> {boolean(), Req, State}.

test/static_handler_SUITE.erl

@@ -67,13 +67,14 @@ init_per_suite(Config) ->
 	true = code:add_pathz(filename:join(
 		[config(data_dir, Config), "static_files_app", "ebin"])),
 	ok = application:load(static_files_app),
-	%% A special folder contains files of 1 character from 0 to 127.
+	%% A special folder contains files of 1 character from 1 to 127
+	%% excluding / and \ as they are always rejected.
 	CharDir = config(priv_dir, Config) ++ "/char",
 	ok = filelib:ensure_dir(CharDir ++ "/file"),
 	Chars0 = lists:flatten([case file:write_file(CharDir ++ [$/, C], [C]) of
 		ok -> C;
 		{error, _} -> []
-	end || C <- lists:seq(0, 127)]),
+	end || C <- (lists:seq(1, 127) -- "/\\")]),
 	%% Determine whether we are on a case insensitive filesystem and
 	%% remove uppercase characters in that case. On case insensitive
 	%% filesystems we end up overwriting the "A" file with the "a" contents.
@@ -134,7 +135,8 @@ init_large_file(Filename) ->
 			"" = os:cmd("truncate -s 32M " ++ Filename),
 			ok;
 		{win32, _} ->
-			ok
+			Size = 32*1024*1024,
+			ok = file:write_file(Filename, <<0:Size/unit:8>>)
 	end.
 
 %% Routes.
@@ -458,21 +460,28 @@ dir_error_slash(Config) ->
 	{403, _, _} = do_get(config(prefix, Config) ++ "//", Config),
 	ok.
 
-dir_error_slash_urlencoded(Config) ->
+dir_error_reserved_urlencoded(Config) ->
-	doc("Try to get a file named '/' percent encoded."),
+	doc("Try to get a file named '/' or '\\' or 'NUL' percent encoded."),
-	{404, _, _} = do_get(config(prefix, Config) ++ "/%2f", Config),
+	{400, _, _} = do_get(config(prefix, Config) ++ "/%2f", Config),
+	{400, _, _} = do_get(config(prefix, Config) ++ "/%5c", Config),
+	{400, _, _} = do_get(config(prefix, Config) ++ "/%00", Config),
 	ok.
 
 dir_error_slash_urlencoded_dotdot_file(Config) ->
 	doc("Try to use a percent encoded slash to access an existing file."),
 	{200, _, _} = do_get(config(prefix, Config) ++ "/directory/../style.css", Config),
-	{404, _, _} = do_get(config(prefix, Config) ++ "/directory%2f../style.css", Config),
+	{400, _, _} = do_get(config(prefix, Config) ++ "/directory%2f../style.css", Config),
 	ok.
 
 dir_error_unreadable(Config) ->
-	doc("Try to get a file that can't be read."),
+	case os:type() of
-	{403, _, _} = do_get(config(prefix, Config) ++ "/unreadable", Config),
+		{win32, _} ->
-	ok.
+			{skip, "ACL not enabled by default under MSYS2."};
+		{unix, _} ->
+			doc("Try to get a file that can't be read."),
+			{403, _, _} = do_get(config(prefix, Config) ++ "/unreadable", Config),
+			ok
+	end.
 
 dir_html(Config) ->
 	doc("Get a .html file."),
@@ -899,10 +908,12 @@ unicode_basic_latin(Config) ->
 		"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
 		"0123456789"
 		":@-_~!$&'()*+,;=",
-	Chars = case config(case_sensitive, Config) of
+	Chars1 = case config(case_sensitive, Config) of
 		false -> Chars0 -- "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
 		true -> Chars0
 	end,
+	%% Remove the characters for which we have no corresponding file.
+	Chars = Chars1 -- (Chars1 -- config(chars, Config)),
 	_ = [case do_get("/char/" ++ [C], Config) of
 		{200, _, << C >>} -> ok;
 		Error -> exit({error, C, Error})