Return 416 for invalid range requests in Plug.Static (#1317)

Author: braidonw

lib/plug/static.ex

@@ -289,6 +289,7 @@ defmodule Plug.Static do
          {range_start, range_end} <- start_and_end(bytes, file_size) do
       send_range(conn, path, range_start, range_end, file_size, options)
     else
+      :unsatisfiable -> send_unsatisfiable_range(conn, file_size, options)
       _ -> send_entire_file(conn, path, options)
     end
   end
@@ -297,6 +298,8 @@ defmodule Plug.Static do
     send_entire_file(conn, path, options)
   end
 
+  defp start_and_end(_range, 0), do: :error
+
   defp start_and_end("-" <> rest, file_size) do
     case Integer.parse(rest) do
       {last, ""} when last > 0 and last <= file_size -> {file_size - last, file_size - 1}
@@ -306,15 +309,18 @@ defmodule Plug.Static do
 
   defp start_and_end(range, file_size) do
     case Integer.parse(range) do
-      {first, "-"} when first >= 0 ->
+      {first, "-"} when first >= 0 and first < file_size ->
         {first, file_size - 1}
 
-      {first, "-" <> rest} when first >= 0 ->
+      {first, "-" <> rest} when first >= 0 and first < file_size ->
         case Integer.parse(rest) do
           {last, ""} when last >= first -> {first, min(last, file_size - 1)}
           _ -> :error
         end
 
+      {first, "-" <> _} when first >= file_size ->
+        :unsatisfiable
+
       _ ->
         :error
     end
@@ -333,6 +339,14 @@ defmodule Plug.Static do
     |> halt()
   end
 
+  defp send_unsatisfiable_range(conn, file_size, options) do
+    conn
+    |> maybe_add_vary(options)
+    |> put_resp_header("content-range", "bytes */#{file_size}")
+    |> send_resp(416, "")
+    |> halt()
+  end
+
   defp send_entire_file(conn, path, options) do
     conn
     |> maybe_add_vary(options)

test/fixtures/empty.txt

@@ -468,6 +468,52 @@ defmodule Plug.StaticTest do
       assert get_resp_header(conn, "content-type") == ["text/plain"]
     end
 
+    test "returns 416 if range start is past the end of the file" do
+      conn =
+        conn(:get, "/public/fixtures/static.txt", [])
+        |> put_req_header("range", "bytes=10-20")
+        |> call()
+
+      assert conn.status == 416
+      assert conn.resp_body == ""
+      assert get_resp_header(conn, "content-range") == ["bytes */5"]
+      assert get_resp_header(conn, "accept-ranges") == ["bytes"]
+    end
+
+    test "returns 416 if open-ended range start is past the end of the file" do
+      conn =
+        conn(:get, "/public/fixtures/static.txt", [])
+        |> put_req_header("range", "bytes=10-")
+        |> call()
+
+      assert conn.status == 416
+      assert get_resp_header(conn, "content-range") == ["bytes */5"]
+    end
+
+    test "returns 416 if range start equals the file size" do
+      for range <- ["bytes=5-", "bytes=5-9"] do
+        conn =
+          conn(:get, "/public/fixtures/static.txt", [])
+          |> put_req_header("range", range)
+          |> call()
+
+        assert conn.status == 416, "expected 416 for #{range}"
+        assert get_resp_header(conn, "content-range") == ["bytes */5"]
+      end
+    end
+
+    test "ignores range and serves the file when it is zero bytes" do
+      for range <- ["bytes=0-", "bytes=1-", "bytes=10-20"] do
+        conn =
+          conn(:get, "/public/fixtures/empty.txt", [])
+          |> put_req_header("range", range)
+          |> call()
+
+        assert conn.status == 200, "expected 200 for #{range}"
+        assert conn.resp_body == ""
+      end
+    end
+
     test "performs etag negotiation" do
       conn =
         conn(:get, "/public/fixtures/static.txt")