unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help
 help / color / mirror / code / Atom feed
From: Jean Boussier <jean.boussier@shopify.com>
To: Eric Wong <e@yhbt.net>
Cc: unicorn-public@yhbt.net
Subject: Re: [PATCH] Add early hints support
Date: Thu, 16 Jul 2020 13:41:36 +0200	[thread overview]
Message-ID: <242F0859-0F83-4F14-A0FF-5BE392BB01E6@shopify.com> (raw)
In-Reply-To: <20200716105037.GA26605@dcvr>

Thanks for the very timely response.

> Since this RDoc ends up on the website, links to any relevant
> Rails documentation and RFC 8297 would also be appropriate.
> Otherwise non-Rails users like me might have no clue what
> it's for.

I updated the documentation, let me know what you think of it.

> Are the method calls for .to_s necessary?

I don't think they are, I mostly took inspiration from the puma implementation
that does all this defensive checks. Based on how that interface is
used by Rails, we could assume both keys and values are strings already.

I simplified the implementation.

> Eep, extra branch...  What's the performance impact for existing
> users when not activated? (on Unix sockets).

Extremely small.

> 
> Perhaps bypassing the method and accessing the @early_hints ivar
> directly can be slightly faster w/o method dispatch.  That
> should also allow using attr_writer instead of attr_accessor,
> I think.

 attr_reader is very optimized in MRI, it's barely slower than @early_hints.
Also it ensure that it doesn't emit a warning in verbose mode if the variable
isn't initialized.

From e0494e10de6549d1b513eef03e68bfa58a6b26ec Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Thu, 16 Jul 2020 11:39:30 +0200
Subject: [PATCH] Add early hints support

While not part of the rack spec, this API is exposed
by both puma and falcon, and Rails use it when available.

The 103 Early Hints response code is specified in RFC 8297.
---
 lib/unicorn/configurator.rb |  9 +++++++++
 lib/unicorn/http_server.rb  | 31 +++++++++++++++++++++++++++++--
 test/unit/test_server.rb    | 30 ++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/lib/unicorn/configurator.rb b/lib/unicorn/configurator.rb
index c3a4f2d..b0606af 100644
--- a/lib/unicorn/configurator.rb
+++ b/lib/unicorn/configurator.rb
@@ -276,6 +276,15 @@ def default_middleware(bool)
     set_bool(:default_middleware, bool)
   end
 
+  # sets whether to enable the proposed early hints Rack API.
+  # If enabled, Rails 5.2+ will automatically send a 103 Early Hint
+  # for all the `javascript_include_tag` and `stylesheet_link_tag`
+  # in your response. See: https://api.rubyonrails.org/v5.2/classes/ActionDispatch/Request.html#method-i-send_early_hints
+  # See also https://tools.ietf.org/html/rfc8297
+  def early_hints(bool)
+    set_bool(:early_hints, bool)
+  end
+
   # sets listeners to the given +addresses+, replacing or augmenting the
   # current set.  This is for the global listener pool shared by all
   # worker processes.  For per-worker listeners, see the after_fork example
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index 45a2e97..05dad99 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -15,7 +15,7 @@ class Unicorn::HttpServer
                 :before_fork, :after_fork, :before_exec,
                 :listener_opts, :preload_app,
                 :orig_app, :config, :ready_pipe, :user,
-                :default_middleware
+                :default_middleware, :early_hints
   attr_writer   :after_worker_exit, :after_worker_ready, :worker_exec
 
   attr_reader :pid, :logger
@@ -588,6 +588,25 @@ def handle_error(client, e)
   rescue
   end
 
+  def e103_response_write(client, headers)
+    response = if @request.response_start_sent
+      "103 Early Hints\r\n"
+    else
+      "HTTP/1.1 103 Early Hints\r\n"
+    end
+
+    headers.each_pair do |k, vs|
+      next if !vs || vs.empty?
+      values = vs.to_s.split("\n".freeze)
+      values.each do |v|
+        response << "#{k}: #{v}\r\n"
+      end
+    end
+    response << "\r\n".freeze
+    response << "HTTP/1.1 ".freeze if @request.response_start_sent
+    client.write(response)
+  end
+
   def e100_response_write(client, env)
     # We use String#freeze to avoid allocations under Ruby 2.1+
     # Not many users hit this code path, so it's better to reduce the
@@ -602,7 +621,15 @@ def e100_response_write(client, env)
   # once a client is accepted, it is processed in its entirety here
   # in 3 easy steps: read request, call app, write app response
   def process_client(client)
-    status, headers, body = @app.call(env = @request.read(client))
+    env = @request.read(client)
+
+    if early_hints
+      env["rack.early_hints"] = lambda do |headers|
+        e103_response_write(client, headers)
+      end
+    end
+
+    status, headers, body = @app.call(env)
 
     begin
       return if @request.hijacked?
diff --git a/test/unit/test_server.rb b/test/unit/test_server.rb
index 8096955..d706243 100644
--- a/test/unit/test_server.rb
+++ b/test/unit/test_server.rb
@@ -23,6 +23,16 @@ def call(env)
   end
 end
 
+class TestEarlyHintsHandler
+  def call(env)
+    while env['rack.input'].read(4096)
+    end
+    env['rack.early_hints'].call(
+      "Link" => "</style.css>; rel=preload; as=style\n</script.js>; rel=preload"
+    )
+    [200, { 'Content-Type' => 'text/plain' }, ['hello!\n']]
+  end
+end
 
 class WebServerTest < Test::Unit::TestCase
 
@@ -84,6 +94,26 @@ def test_preload_app_config
     tmp.close!
   end
 
+  def test_early_hints
+    teardown
+    redirect_test_io do
+      @server = HttpServer.new(TestEarlyHintsHandler.new,
+                               :listeners => [ "127.0.0.1:#@port"],
+                               :early_hints => true)
+      @server.start
+    end
+
+    sock = TCPSocket.new('127.0.0.1', @port)
+    sock.syswrite("GET / HTTP/1.0\r\n\r\n")
+
+    responses = sock.sysread(4096)
+    assert_match %r{\AHTTP/1.[01] 103\b}, responses
+    assert_match %r{^Link: </style\.css>}, responses
+    assert_match %r{^Link: </script\.js>}, responses
+
+    assert_match %r{^HTTP/1.[01] 200\b}, responses
+  end
+
   def test_broken_app
     teardown
     app = lambda { |env| raise RuntimeError, "hello" }
-- 
2.26.2



  reply	other threads:[~2020-07-16 11:41 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-16 10:05 Jean Boussier
2020-07-16 10:50 ` Eric Wong
2020-07-16 11:41   ` Jean Boussier [this message]
2020-07-16 12:16     ` Eric Wong
2020-07-16 12:24       ` Jean Boussier
2020-07-17  1:19         ` Eric Wong
2020-07-20  9:18           ` Jean Boussier
2020-07-20 10:09             ` Eric Wong
2020-07-20 10:27               ` Jean Boussier
2020-07-20 10:55                 ` Eric Wong
2020-07-20 11:53                   ` Jean Boussier
2020-07-20 20:27                     ` Eric Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://yhbt.net/unicorn/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=242F0859-0F83-4F14-A0FF-5BE392BB01E6@shopify.com \
    --to=jean.boussier@shopify.com \
    --cc=e@yhbt.net \
    --cc=unicorn-public@yhbt.net \
    --subject='Re: [PATCH] Add early hints support' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	../../unicorn.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).