Merge pull request #287 from webmachine/orien/rack

Author: orien

.github/workflows/test.yml

@@ -4,22 +4,27 @@ on: [push, pull_request]
 
 jobs:
   test:
+    name: Test (Ruby ${{ matrix.ruby }}, Rack ${{ matrix.rack }})
     runs-on: "ubuntu-latest"
     continue-on-error: ${{ matrix.experimental }}
     strategy:
       fail-fast: false
       matrix:
-        ruby_version: ["2.6", "2.7", "3.0", "3.1", "3.2", "3.3", "3.4", "4.0"]
+        ruby: ["2.6", "2.7", "3.0", "3.1", "3.2", "3.3", "3.4", "4.0"]
+        rack: ["2", "3"]
         experimental: [false]
         include:
-          - ruby_version: "ruby-head"
+          - ruby: "ruby-head"
+            rack: "3"
             experimental: true
+    env:
+      RACK_VERSION: ${{ matrix.rack }}
 
     steps:
       - uses: actions/checkout@v6
       - uses: ruby/setup-ruby@v1
         with:
-          ruby-version: ${{ matrix.ruby_version }}
+          ruby-version: ${{ matrix.ruby }}
           bundler-cache: true
       - run: "bundle exec rubocop lib/ spec/"
       - run: "bundle exec rspec spec/ -b"

Gemfile

@@ -8,7 +8,9 @@ group :development do
 end
 
 group :test do
-  gem 'rack', '~> 2'
+  rack_version = ENV.fetch('RACK_VERSION', '3')
+  gem 'rack', "~> #{rack_version}"
+  gem 'rackup' if rack_version == '3'
   gem 'rack-test', '~> 2'
   gem 'rspec', '~> 3.0', '>= 3.6.0'
   gem 'rspec-its', '~> 1.2'

lib/webmachine/adapters/rack.rb

@@ -52,7 +52,12 @@ def run
           Host: application.configuration.ip
         }).merge(application.configuration.adapter_options)
 
-        @server = ::Rack::Server.new(options)
+        if rack_v3?
+          require 'rackup'
+          @server = ::Rackup::Server.new(options)
+        else
+          @server = ::Rack::Server.new(options)
+        end
         @server.start
       end
 
@@ -70,18 +75,33 @@ def call(env)
         response.headers[SERVER] = VERSION_STRING
 
         rack_status = response.code
-        rack_headers = response.headers.flattened(NEWLINE)
+        rack_headers = build_rack_response_headers(response.headers)
         rack_body = case response.body
         when String # Strings are enumerable in ruby 1.8
           [response.body]
         else
           if (io_body = IO.try_convert(response.body))
             io_body
           elsif response.body.respond_to?(:call)
-            Webmachine::ChunkedBody.new(Array(response.body.call))
+            if rack_v3?
+              # In Rack 3 the server (e.g. WEBrick via Rackup) buffers the body
+              # and applies chunked encoding itself when Transfer-Encoding is set,
+              # so we must not pre-encode with ChunkedBody.
+              [response.body.call]
+            else
+              # Rack 2's WEBrick handler sends the body as-is; ChunkedBody is
+              # required to produce valid chunked-encoded wire data.
+              Webmachine::ChunkedBody.new(Array(response.body.call))
+            end
           elsif response.body.respond_to?(:each)
-            # This might be an IOEncoder with a Content-Length, which shouldn't be chunked.
-            if response.headers[TRANSFER_ENCODING] == 'chunked'
+            if rack_v3?
+              # Return the plain enumerable. Rackup buffers it into a String then
+              # WEBrick chunks that String when Transfer-Encoding: chunked is set.
+              response.body
+            elsif response.headers[TRANSFER_ENCODING] == 'chunked'
+              # Rack 2: only pre-encode bodies that are already marked chunked;
+              # IOEncoder bodies carry their own Content-Length and must not be
+              # wrapped.
               Webmachine::ChunkedBody.new(response.body)
             else
               response.body
@@ -97,6 +117,45 @@ def call(env)
 
       protected
 
+      # Build a Rack-compatible response headers hash from Webmachine's response
+      # headers.
+      #
+      # Header names are always lowercased: this is required by Rack 3 and is
+      # harmless for Rack 2 (all Rack 2 handlers match header names
+      # case-insensitively).
+      #
+      # The +set-cookie+ value is formatted differently per Rack version:
+      #
+      # * Rack 3 / Rackup: the value must be an Array. Rackup's WEBrick handler
+      #   deletes the lowercase +set-cookie+ key and calls
+      #   +res.cookies.concat(Array(value))+, emitting one Set-Cookie line per
+      #   cookie. Joining with +\n+ instead would produce a header value
+      #   containing a newline, which WEBrick 1.9+ rejects as
+      #   +WEBrick::HTTPResponse::InvalidHeader+.
+      #
+      # * Rack 2 / Rack::Handler::WEBrick: the handler splits on +\n+ before
+      #   adding cookies (+vs.split("\n")+), so the value must be a newline-joined
+      #   String. Passing an Array causes a +NoMethodError+ because +Array+ does
+      #   not define +#split+.
+      def build_rack_response_headers(response_headers)
+        response_headers.each_with_object({}) do |(key, value), h|
+          rack_key = key.downcase
+          h[rack_key] = if rack_key == 'set-cookie'
+            if rack_v3?
+              # Array lets Rackup emit one Set-Cookie header per cookie.
+              Array(value)
+            else
+              # Rack 2's handler splits on \n; give it a newline-joined String.
+              Array(value).join(NEWLINE)
+            end
+          elsif value.is_a?(Array)
+            value.join(NEWLINE)
+          else
+            value
+          end
+        end
+      end
+
       def routing_tokens(rack_req)
         nil # no-op for default, un-mapped rack adapter
       end
@@ -107,6 +166,11 @@ def base_uri(rack_req)
 
       private
 
+      # Returns true when running under Rack 3.x.
+      def rack_v3?
+        ::Rack.release.start_with?('3.')
+      end
+
       def build_webmachine_request(rack_req, headers)
         RackRequest.new(rack_req.request_method,
           rack_req.url,
@@ -128,6 +192,9 @@ def initialize(method, uri, headers, body, routing_tokens, base_uri, env)
 
       class RackResponse
         ONE_FIVE = '1.5'.freeze
+        # Header names are normalised to lowercase by build_rack_response_headers,
+        # so use the lowercase form everywhere inside RackResponse too.
+        LOWERCASE_CONTENT_TYPE = 'content-type'.freeze
 
         def initialize(body, status, headers)
           @body = body
@@ -136,8 +203,8 @@ def initialize(body, status, headers)
         end
 
         def finish
-          @headers[CONTENT_TYPE] ||= TEXT_HTML if rack_release_enforcing_content_type
-          @headers.delete(CONTENT_TYPE) if response_without_body
+          @headers[LOWERCASE_CONTENT_TYPE] ||= TEXT_HTML if rack_release_enforcing_content_type
+          @headers.delete(LOWERCASE_CONTENT_TYPE) if response_without_body
           [@status, @headers, @body]
         end
 
@@ -177,8 +244,12 @@ def to_s
           if @value
             @value.join
           else
-            @request.body.rewind
-            @request.body.read
+            # Rack 3 removed the requirement for rack.input to implement #rewind
+            # (Rack::Lint::InputWrapper in Rack 3 does not define it), so guard
+            # the call to avoid a NoMethodError on every PUT/POST request.
+            body = @request.body
+            body.rewind if body.respond_to?(:rewind)
+            body.read
           end
         end
 

spec/webmachine/adapters/rack_spec.rb

@@ -22,7 +22,7 @@
     it 'should add Content-Type header on not acceptable response' do
       rack_response = described_class.new(double(:body), 406, {})
       _rack_status, rack_headers, _rack_body = rack_response.finish
-      expect(rack_headers).to have_key('Content-Type')
+      expect(rack_headers).to have_key('content-type')
     end
   end
 
@@ -32,7 +32,7 @@
     it 'should not add Content-Type header on not acceptable response' do
       rack_response = described_class.new(double(:body), 406, {})
       _rack_status, rack_headers, _rack_body = rack_response.finish
-      expect(rack_headers).not_to have_key('Content-Type')
+      expect(rack_headers).not_to have_key('content-type')
     end
   end
 end
@@ -57,7 +57,9 @@
 
     it 'provides the rack env on the request' do
       rack_response = get 'test', nil, {'HTTP_ACCEPT' => 'test/response.rack_env'}
-      expect(JSON.parse(rack_response.body).keys).to include 'rack.input'
+      # rack-test does not populate rack.input for GET requests in Rack 3,
+      # so check for rack.errors which is always present in every Rack version.
+      expect(JSON.parse(rack_response.body).keys).to include 'rack.errors'
     end
   end
 end