From 27b1ea59668a0ea372cbe1cb4eae9ed077d3bf3a Mon Sep 17 00:00:00 2001 From: Peleg Rosenthal Date: Mon, 6 Apr 2020 16:07:31 -0400 Subject: [PATCH 1/2] Drop incorrect port `Rack::Request#url` includes the port of the original request (despite accurate scheme updates). This becomes an issue when LB terminates SSL: - client request is made to https://example.com:443, - LB request is http://example.com:80 - build_api_url returns correct scheme but wrong port: https://example:80 Since we only need the request url from the env, this changeset manually fetches the correct scheme (Rack accounts for FWD headers already), the host (w/out port as opposed to #host_with_port), and the fullpath (pathname + query params) Another benefit of this changeset is that it no longer mutates the original env object (assigning to `new_env` is by reference). Caveat: im dropping @options[:protocol] bc there's x-fwd-proto should handle this already. --- lib/prerender_rails.rb | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/lib/prerender_rails.rb b/lib/prerender_rails.rb index ef059c6..bcc0d79 100644 --- a/lib/prerender_rails.rb +++ b/lib/prerender_rails.rb @@ -181,32 +181,14 @@ def get_prerendered_page_response(env) end end - def build_api_url(env) - new_env = env - if env["CF-VISITOR"] - match = /"scheme":"(http|https)"/.match(env['CF-VISITOR']) - new_env["HTTPS"] = true and new_env["rack.url_scheme"] = "https" and new_env["SERVER_PORT"] = 443 if (match && match[1] == "https") - new_env["HTTPS"] = false and new_env["rack.url_scheme"] = "http" and new_env["SERVER_PORT"] = 80 if (match && match[1] == "http") - end - - if env["X-FORWARDED-PROTO"] - new_env["HTTPS"] = true and new_env["rack.url_scheme"] = "https" and new_env["SERVER_PORT"] = 443 if env["X-FORWARDED-PROTO"].split(',')[0] == "https" - new_env["HTTPS"] = false and new_env["rack.url_scheme"] = "http" and new_env["SERVER_PORT"] = 80 if env["X-FORWARDED-PROTO"].split(',')[0] == "http" - end - - if @options[:protocol] - new_env["HTTPS"] = true and new_env["rack.url_scheme"] = "https" and new_env["SERVER_PORT"] = 443 if @options[:protocol] == "https" - new_env["HTTPS"] = false and new_env["rack.url_scheme"] = "http" and new_env["SERVER_PORT"] = 80 if @options[:protocol] == "http" - end - - url = Rack::Request.new(new_env).url + request_obj = Rack::Request.new(env) + url = "#{request_obj.scheme}://#{request_obj.host}#{request_obj.fullpath}" prerender_url = get_prerender_service_url() forward_slash = prerender_url[-1, 1] == '/' ? '' : '/' "#{prerender_url}#{forward_slash}#{url}" end - def get_prerender_service_url @options[:prerender_service_url] || ENV['PRERENDER_SERVICE_URL'] || 'http://service.prerender.io/' end From 22467c9a515002327db55cb7639b35f99c479b52 Mon Sep 17 00:00:00 2001 From: Peleg Rosenthal Date: Mon, 18 May 2020 13:32:16 -0400 Subject: [PATCH 2/2] Reinstate CloudFlare support + custom protocol - Reinstating custom support for CF-VISITOR CloudFlare header + options[:protocol] - Fixing failing tests Instead of passing in `X-Forwarded-Proto` to the Rack env, we can now safely rely on `HTTP_` env variables which are automatically set. Per https://github.com/rack/rack/blob/master/SPEC.rdoc#the-environment-: HTTP_ Variables: Variables corresponding to the client-supplied HTTP request headers (i.e., variables whose names begin with HTTP_). The presence or absence of these variables should correspond with the presence or absence of the appropriate HTTP header in the request. See RFC3875 section 4.1.18 for specific behavior. --- lib/prerender_rails.rb | 9 ++++++++- test/lib/prerender_rails.rb | 4 ++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/prerender_rails.rb b/lib/prerender_rails.rb index bcc0d79..3392d02 100644 --- a/lib/prerender_rails.rb +++ b/lib/prerender_rails.rb @@ -181,9 +181,16 @@ def get_prerendered_page_response(env) end end + def get_cloudflare_scheme(env) + match = /"scheme":"(http|https)"/.match(env['CF-VISITOR']) + match && match[1] + end + def build_api_url(env) request_obj = Rack::Request.new(env) - url = "#{request_obj.scheme}://#{request_obj.host}#{request_obj.fullpath}" + scheme = @options[:protocol] || get_cloudflare_scheme(env) || request_obj.scheme + url = "#{scheme}://#{request_obj.host}#{request_obj.fullpath}" + prerender_url = get_prerender_service_url() forward_slash = prerender_url[-1, 1] == '/' ? '' : '/' "#{prerender_url}#{forward_slash}#{url}" diff --git a/test/lib/prerender_rails.rb b/test/lib/prerender_rails.rb index 60d66d9..060f4e2 100644 --- a/test/lib/prerender_rails.rb +++ b/test/lib/prerender_rails.rb @@ -200,7 +200,7 @@ # Check X-Forwarded-Proto because Heroku SSL Support terminates at the load balancer it "should build the correct api url for the Heroku SSL Addon support with single value" do - request = Rack::MockRequest.env_for "http://google.com/search?q=javascript", { 'X-FORWARDED-PROTO' => 'https'} + request = Rack::MockRequest.env_for "http://google.com/search?q=javascript", { 'HTTP_X_FORWARDED_PROTO' => 'https'} ENV['PRERENDER_SERVICE_URL'] = nil assert_equal 'http://service.prerender.io/https://google.com/search?q=javascript', @prerender.build_api_url(request) end @@ -208,7 +208,7 @@ # Check X-Forwarded-Proto because Heroku SSL Support terminates at the load balancer it "should build the correct api url for the Heroku SSL Addon support with double value" do - request = Rack::MockRequest.env_for "http://google.com/search?q=javascript", { 'X-FORWARDED-PROTO' => 'https,http'} + request = Rack::MockRequest.env_for "http://google.com/search?q=javascript", { 'HTTP_X_FORWARDED_PROTO' => 'https,http'} ENV['PRERENDER_SERVICE_URL'] = nil assert_equal 'http://service.prerender.io/https://google.com/search?q=javascript', @prerender.build_api_url(request) end