Skip to content

Commit 59cf2b5

Browse files
authored
Fix raise_on_missing_only raising on non-matching paths with colons (#1298)
When `Plug.Static` is mounted at `/` with `raise_on_missing_only: true`, application routes containing colons (e.g. `/app/resource:id`) would i raise `InvalidPathError` because `invalid_path?` ran before the file existence check. This fix splits the catch-all status branch into explicit `:allowed` and `:raise` clauses so that `:raise` only fires when a valid path resolves to a real file on disk.
1 parent 200d432 commit 59cf2b5

File tree

2 files changed

+77
-10
lines changed

2 files changed

+77
-10
lines changed

lib/plug/static.ex

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ defmodule Plug.Static do
204204
:forbidden ->
205205
conn
206206

207-
status ->
207+
:allowed ->
208208
segments = Enum.map(segments, &URI.decode/1)
209209

210210
if invalid_path?(segments) do
@@ -215,17 +215,19 @@ defmodule Plug.Static do
215215
range = get_req_header(conn, "range")
216216

217217
case file_encoding(conn, path, range, encodings) do
218-
:error ->
219-
conn
218+
:error -> conn
219+
triplet -> serve_static(triplet, conn, segments, range, options)
220+
end
220221

221-
triplet ->
222-
if status == :raise do
223-
raise InvalidPathError,
224-
"static file exists but is not in the :only list: #{Enum.join(segments, "/")}. " <>
225-
"Add it to the :only list or use :only_matching for prefix matching"
226-
end
222+
:raise ->
223+
segments = Enum.map(segments, &URI.decode/1)
227224

228-
serve_static(triplet, conn, segments, range, options)
225+
if not invalid_path?(segments) and regular_file_info(path(from, segments)) do
226+
raise InvalidPathError,
227+
"static file exists but is not in the :only list: #{Enum.join(segments, "/")}. " <>
228+
"Add it to the :only list or use :only_matching for prefix matching"
229+
else
230+
conn
229231
end
230232
end
231233
end

test/plug/static_test.exs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -895,4 +895,69 @@ defmodule Plug.StaticTest do
895895

896896
assert conn.status == 200
897897
end
898+
899+
describe "raise_on_missing_only" do
900+
test "passes through when path has colons and does not match :only" do
901+
conn =
902+
conn(:get, "/public/resource:identifier")
903+
|> call(only: ["assets"], raise_on_missing_only: true)
904+
905+
assert conn.status == 404
906+
assert conn.resp_body == "Passthrough"
907+
end
908+
909+
test "passes through when path has dot-dot segments and does not match :only" do
910+
conn =
911+
conn(:get, "/public/..%2Fsecret")
912+
|> call(only: ["assets"], raise_on_missing_only: true)
913+
914+
assert conn.status == 404
915+
assert conn.resp_body == "Passthrough"
916+
end
917+
918+
test "passes through when path has null bytes and does not match :only" do
919+
conn =
920+
conn(:get, "/public/file%00.txt")
921+
|> call(only: ["assets"], raise_on_missing_only: true)
922+
923+
assert conn.status == 404
924+
assert conn.resp_body == "Passthrough"
925+
end
926+
927+
test "passes through when non-existent file does not match :only" do
928+
conn =
929+
conn(:get, "/public/nonexistent.txt")
930+
|> call(only: ["assets"], raise_on_missing_only: true)
931+
932+
assert conn.status == 404
933+
assert conn.resp_body == "Passthrough"
934+
end
935+
936+
test "passes through with only_matching when path does not match prefix" do
937+
conn =
938+
conn(:get, "/public/resource:identifier")
939+
|> call(only_matching: ["assets"], raise_on_missing_only: true)
940+
941+
assert conn.status == 404
942+
assert conn.resp_body == "Passthrough"
943+
end
944+
945+
test "raises when existing file is not in :only list" do
946+
assert_raise Plug.Static.InvalidPathError,
947+
~r/static file exists but is not in the :only list/,
948+
fn ->
949+
conn(:get, "/public/fixtures/static.txt")
950+
|> call(only: ["assets"], raise_on_missing_only: true)
951+
end
952+
end
953+
954+
test "serves file normally when it matches :only list" do
955+
conn =
956+
conn(:get, "/public/fixtures/static.txt")
957+
|> call(only: ["fixtures"], raise_on_missing_only: true)
958+
959+
assert conn.status == 200
960+
assert conn.resp_body == "HELLO"
961+
end
962+
end
898963
end

0 commit comments

Comments
 (0)