librelist archives

« back to archive

Re: [PATCH] Adding port number so that things resolve properly.

Re: [PATCH] Adding port number so that things resolve properly.

From:
Eric Wong
Date:
2013-03-05 @ 04:45
(Correcting local-openid -> local.openid@librelist.org)
(And librelist doesn't like Cc:)

Eric Wong <normalperson@yhbt.net> wrote:
> Nathan Paul Simons <E1EtL9ToE-git@hardcorehackers.com> wrote:
> > diff --git a/lib/local_openid.rb b/lib/local_openid.rb
> > index ab342a9..d1911f4 100644
> > --- a/lib/local_openid.rb
> > +++ b/lib/local_openid.rb
> > @@ -164,7 +164,7 @@ class LocalOpenID < Sinatra::Base
> >    # being proxied and being hit with an appropriate HTTP Host: header.
> >    # Don't expect OpenID consumers to handle port != 80.
> >    def server_root
> > -    "http://#{request.host}/"
> > +    "http://#{request.host}:#{request.port}/"
> >    end
> 
> Thanks for noticing this.
> 
> Nowadays (since Rack 1.3.0), we can use a base_url method, which
> a) allows us to account for HTTPS
> b) doesn't blindly append standard ports (80/443) when they're not needed
> 
> Perhaps the following (untested) patch to s/server_root/base_url
> will work.  I will test later tonight.

Of course, my earlier patch was totally untested and broken.  Below
is what I pushed out.

Will release 0.4.0 with this in a few days pending any other changes/bugs.

From 16c7bcebb9c0ddc1e94bd313fa460efe4884bbe0 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Tue, 5 Mar 2013 03:48:44 +0000
Subject: [PATCH] use Rack::Request#base_url for server_root

Nathan Paul Simons reported the need to include the port for
certain consumers.  Since we don't want to blindly add the port
for users on standard HTTP/HTTPS ports, we can use
Rack::Request#base_url to generate the URL for us.

Rack::Request#base_url is available since Rack 1.3.0
---
 lib/local_openid.rb  | 5 +----
 local-openid.gemspec | 1 +
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/local_openid.rb b/lib/local_openid.rb
index ab342a9..0edbf37 100644
--- a/lib/local_openid.rb
+++ b/lib/local_openid.rb
@@ -160,11 +160,8 @@ class LocalOpenID < Sinatra::Base
     finalize_response(oidresp)
   end
 
-  # we're the provider for exactly one identity.  However, we do rely on
-  # being proxied and being hit with an appropriate HTTP Host: header.
-  # Don't expect OpenID consumers to handle port != 80.
   def server_root
-    "http://#{request.host}/"
+    "#{request.base_url}/"
   end
 
   def server
diff --git a/local-openid.gemspec b/local-openid.gemspec
index 8b129db..da872b6 100644
--- a/local-openid.gemspec
+++ b/local-openid.gemspec
@@ -20,6 +20,7 @@ Gem::Specification.new do |s|
   s.summary = summary
   s.rdoc_options = rdoc_options
   s.rubyforge_project = %q{qrp}
+  s.add_dependency(%q<rack>, ["~> 1.3"])
   s.add_dependency(%q<sinatra>, ["~> 1.0"])
   s.add_dependency(%q<ruby-openid>, ["~> 2.1.7"])
   # s.licenses = %w(AGPLv3) # accessor not compatible with older RubyGems
-- 
Eric Wong