From c917ac526df214611ec33c21de2b070452ec8434 Mon Sep 17 00:00:00 2001
From: Dirkjan Bussink <d.bussink@gmail.com>
Date: Mon, 8 Mar 2021 09:51:09 +0100
Subject: Allocate a new request for each client

This removes the reuse of the parser between requests. Reusing these is
risky in the context of running any other threads within the unicorn
process, also for threads that run background tasks.

If any other thread accidentally grabs hold of the request it can modify
things for the next request in flight.

The downside here is that we allocate more for each request, but that is
worth the trade off here and the security risk we otherwise would carry
to leaking wrong and incorrect data.
---
 t/hijack.ru            | 55 --------------------------------------------------
 t/t0200-rack-hijack.sh | 51 ----------------------------------------------
 2 files changed, 106 deletions(-)
 delete mode 100644 t/hijack.ru
 delete mode 100755 t/t0200-rack-hijack.sh

(limited to 't')

diff --git a/t/hijack.ru b/t/hijack.ru
deleted file mode 100644
index 02260e2..0000000
--- a/t/hijack.ru
+++ /dev/null
@@ -1,55 +0,0 @@
-use Rack::Lint
-use Rack::ContentLength
-use Rack::ContentType, "text/plain"
-class DieIfUsed
-  @@n = 0
-  def each
-    abort "body.each called after response hijack\n"
-  end
-
-  def close
-    warn "closed DieIfUsed #{@@n += 1}\n"
-  end
-end
-
-envs = []
-
-run lambda { |env|
-  case env["PATH_INFO"]
-  when "/hijack_req"
-    if env["rack.hijack?"]
-      io = env["rack.hijack"].call
-      envs << env
-      if io.respond_to?(:read_nonblock) &&
-         env["rack.hijack_io"].respond_to?(:read_nonblock)
-
-        # exercise both, since we Rack::Lint may use different objects
-        env["rack.hijack_io"].write("HTTP/1.0 200 OK\r\n\r\n")
-        io.write("request.hijacked")
-        io.close
-        return [ 500, {}, DieIfUsed.new ]
-      end
-    end
-    [ 500, {}, [ "hijack BAD\n" ] ]
-  when "/hijack_res"
-    r = "response.hijacked"
-    [ 200,
-      {
-        "Content-Length" => r.bytesize.to_s,
-        "rack.hijack" => proc do |io|
-          envs << env
-          io.write(r)
-          io.close
-        end
-      },
-      DieIfUsed.new
-    ]
-  when "/normal_env_id"
-    b = "#{env.object_id}\n"
-    h = {
-      'Content-Type' => 'text/plain',
-      'Content-Length' => b.bytesize.to_s,
-    }
-    [ 200, h, [ b ] ]
-  end
-}
diff --git a/t/t0200-rack-hijack.sh b/t/t0200-rack-hijack.sh
deleted file mode 100755
index fee0791..0000000
--- a/t/t0200-rack-hijack.sh
+++ /dev/null
@@ -1,51 +0,0 @@
-#!/bin/sh
-. ./test-lib.sh
-t_plan 9 "rack.hijack tests (Rack 1.5+ (Rack::VERSION >= [ 1,2]))"
-
-t_begin "setup and start" && {
-	unicorn_setup
-	unicorn -D -c $unicorn_config hijack.ru
-	unicorn_wait_start
-}
-
-t_begin "normal env reused between requests" && {
-	env_a="$(curl -sSf http://$listen/normal_env_id)"
-	b="$(curl -sSf http://$listen/normal_env_id)"
-	test x"$env_a" = x"$b"
-}
-
-t_begin "check request hijack" && {
-	test "xrequest.hijacked" = x"$(curl -sSfv http://$listen/hijack_req)"
-}
-
-t_begin "env changed after request hijack" && {
-	env_b="$(curl -sSf http://$listen/normal_env_id)"
-	test x"$env_a" != x"$env_b"
-}
-
-t_begin "check response hijack" && {
-	test "xresponse.hijacked" = x"$(curl -sSfv http://$listen/hijack_res)"
-}
-
-t_begin "env changed after response hijack" && {
-	env_c="$(curl -sSf http://$listen/normal_env_id)"
-	test x"$env_b" != x"$env_c"
-}
-
-t_begin "env continues to be reused between requests" && {
-	b="$(curl -sSf http://$listen/normal_env_id)"
-	test x"$env_c" = x"$b"
-}
-
-t_begin "killing succeeds after hijack" && {
-	kill $unicorn_pid
-}
-
-t_begin "check stderr for hijacked body close" && {
-	check_stderr
-	grep 'closed DieIfUsed 1\>' $r_err
-	grep 'closed DieIfUsed 2\>' $r_err
-	! grep 'closed DieIfUsed 3\>' $r_err
-}
-
-t_done
-- 
cgit v1.2.3-24-ge0c7