Skip to content

Commit

Permalink
Moved Domain regexp to a constant and added comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
@rizwanreza @josevalim
rizwanreza authored and josevalim committed Jun 11, 2010
1 parent edbb78d commit 5609149
Showing 1 changed file with 8 additions and 1 deletion.
9 changes: 8 additions & 1 deletion actionpack/lib/action_dispatch/middleware/session/abstract_store.rb
View file
Expand Up @@ -93,6 +93,13 @@ def stale_session_check!
:cookie_only => true
}

# This regular expression is used to split the levels of a domain:
# So www.example.co.uk gives:
# $1 => www.
# $2 => example
# $3 => co.uk
DOMAIN_REGEXP = /^(.*\.)*(.*)\.(...|...\...|....|..\...|..)$/

This comment has been minimized.

Sign in to view

Copy link
@mat813

mat813 Jun 11, 2010

Contributor

I'm wondering what it does in case the tld is longer than four characters, like, say, in
http://موقع.وزارة-الاتصالات.مصر/
which should translate to
http://xn--4gbrim.xn----ymcbaaajlc6dj7bxne2c.xn--wgbh1c/

This comment has been minimized.

Sign in to view

Copy link
@guillermo

guillermo Jun 24, 2010

Contributor

Or just .museum or .travel

This comment has been minimized.

Sign in to view

Copy link
@rizwanreza

rizwanreza Jun 24, 2010

Author Contributor

What are the ideas to properly capture them? I can add |.....| etc... but that would be awful. I will have a look at more ways also.

This comment has been minimized.

Sign in to view

Copy link
@guillermo

guillermo Jun 24, 2010

Contributor

For that reason i just only support second level domains in the first commit.

If you need something special, you must explicit declare the domain as a string:

:domain => :all # For 80% of cases second.level.
:domain => "mysyte.co.uk"
:domain => "guillermoÁlvarez.nom.es"

2 .- The other implementation I though, maybe uglier, was:
:domain => :first_level # maybe for intranets or special domains: ".museum"
:domain => :second_level # normal cases: ".mydomain.com"
:doamin => :all # alias for :second_level domain
:domain => :third_level # ".google.co.uk" or "guillermo.nom.es.uk"
:domain => :fifth_level # ...
:domain => :sixth_level # ...

3.- # the last implementation i though was usign a proc:
:domain => :all
:domain => lambda {|env| rails.development? ? ".localhost" : "."+(env["HTTP_HOST"].split(".")[-4..-1].join(".")) }

As I did not come to any conclusions, i decided to just include :all for the 80% of the cases, and use a string for the others.

I can pass you any of this implementations tomorrow, if you want.

I think the current code does not satisfy the principle of minimum surprise, as you don't know what it will do in all circunstancies. Some times it will be second level and others third, and others, like .museum :domain will crash cookies as domain will be ".."

Obviously, i prefer my first implementation, maybe improving documentation and maybe w/ the second proposal (:first_level, :second_level, :third_level, ...) in this document .

This comment has been minimized.

Sign in to view

Copy link
@guillermo

guillermo Jun 24, 2010

Contributor

After study, I considered not feasible to implement the existing mnemonics, and quite difficult to maintain.

def initialize(app, options = {})
@app = app
@default_options = DEFAULT_OPTIONS.merge(options)
Expand Down Expand Up @@ -123,7 +130,7 @@ def call(env)
end

if options[:domain] == :all
env["HTTP_HOST"] =~ /^(.*\.)*(.*)\.(...|...\...|....|..\...)$/
env["HTTP_HOST"] =~ DOMAIN_REGEXP
options[:domain] = ".#{$2}.#{$3}"
end

Expand Down

0 comments on commit 5609149

Please sign in to comment.