Rails Ruby on Rails

Comments and changes to this ticket

• You flagged this item as spam.
  

  Bryce Thornton July 19th, 2010 @ 12:48 AM

  • Tag changed from “3.x cookie_store” to “3.x, cookie_store, patch”

  Here's the patch.

  • tld_fix_for_cookie_session_store.patch 3.3 KB

• You flagged this item as spam.
  

  Mislav July 19th, 2010 @ 10:35 PM

  Since when is this broken? I have some apps on Rails 3 in development using ".local" and I didn't notice session problems. I have cookie-based sessions, all default params.

  I've taken a look at the patch and the new regex doesn't seem much better than the old one. What problem are we solving here exactly? Is splitting a hostname into subdomains/domain/TLD really possible, even with regexes?

  Consider a host like "mislav.com.hr". Can anyone tell me (without looking at Wikipedia) is "mislav" a subdomain or a domain under the ".com.hr" TLD?

• You flagged this item as spam.
  

  Bryce Thornton July 19th, 2010 @ 11:14 PM

  Are you sure you're using the ":domain => :all" parameter? The problem only happens when using that option. By default, the rails3 session_store.rb initializer does not use that option. Here's the ticket where this option originated:

  https://rails.lighthouseapp.com/projects/8994/tickets/4818-share-se...

  No, I don't think it's fully possible to make this work for 100% of cases. No matter what technique is used, there are still going to be some edge cases. The current solution does not work for domains like ".local" or ".co.uk", so that is the benefit of my patch.

  The tests included in the patch demonstrate the issue. If you run them with the current solution you'll see that they don't handle these TLDs correctly.

  So, my patch is very liberal in guessing the TLD. Anything after the final period is assumed to be the TLD unless the host ends with a .. or .*. style string. I feel like this will catch the majority of cases. The main ones it will fail on are hosts that have a two character country code and a 2-3 character domain. For example, "http://www.abc.in/" would create the cookie for ".www.abc.in" instead of ".abc.in".

  BTW, here's the case I'm using this option for: I'm building a site that creates subdomain URL's for each account. Normally, I could just pass ":domain => '.mynewsite.com'" and be done. The cookie would work for all subdomains. The problem is that each account has the option to set a custom URL using their own domain name. In that case, the cookie would not work since it would be for the wrong domain. So, I'm using the ":domain => :all" option to work in both of these cases. It's a great option.

• You flagged this item as spam.
  

  Doug Mayer August 2nd, 2010 @ 07:03 PM

  I'm getting this same problem. In Rails 3.0.0.rc, when I set :domain => :all, I get an InvalidAuthenticityToken when trying to post to any form at any.smackaho.st or smackaho.st. Setting :domain => ".smackaho.st" works, but there is clearly something wrong with the #4818 patch in that it ignores auth token stuff like this.

• You flagged this item as spam.
  

  Bryce Thornton August 2nd, 2010 @ 07:56 PM

  Doug,

  I'm still having the problem as well. The patch from #4818 doesn't ignore the auth token logic, it simply can't figure out the domain for the cookie to use. Thus, it tries to create a cookie without a domain and that fails. The auth token code it's trying to check against is stored in the session, which relies on the cookie that isn't there.

  My patch fixes this issue. Hopefully they apply it or something similar soon.

• 

  José Valim August 14th, 2010 @ 11:07 AM

  • Importance changed from “” to “Low”

  The patch looks great. The only issue I saw is that you changed an existing test while you should only add new ones. Could you please provide a new patch? Thanks!

• 

  José Valim August 14th, 2010 @ 12:07 PM

  • Assigned user set to “José Valim”

• You flagged this item as spam.
  

  Bryce Thornton August 14th, 2010 @ 06:44 PM

  Thanks for the feedback José. I'm attaching a revised patch. Thanks!

  • revised_tld_fix_for_cookie_session_store.patch 3.4 KB

• You flagged this item as spam.
  

  Bryce Thornton August 14th, 2010 @ 08:17 PM

  I found another issue with the way this feature handles the cookie domain. It would include the port # in the cookie domain, causing the cookie to not be created. I'm attaching a revised patch that also fixes this issue.

  Here's a cookie domain that would have been created before:

  .mysite.local:3000

  Now, it will correctly be created as ".mysite.local".

  The fix was to pull the host from "request.host" instead of "request.env["HTTP_HOST"]".

  Thanks!

  • revised_tld_fix_for_cookie_session_store_2.patch 3.8 KB

• You flagged this item as spam.
  

  Bryce Thornton August 14th, 2010 @ 08:36 PM

  Small tweak to last patch to add the "Resolved" line to the commit message.

  • revised_tld_fix_for_cookie_session_store_3.patch 3.8 KB
• 

  Repository August 15th, 2010 @ 08:50 PM

  • State changed from “new” to “resolved”

  (from [fd78bb727045547371179428886c9b262d66091d]) Allow for any possible TLD when using the :all option with the cookie session store. This works for subdomain.mysite.local, google.co.uk, google.com.au, etc. [#5147 state:resolved]

  Signed-off-by: José Valim jose.valim@gmail.com
  http://github.com/rails/rails/commit/fd78bb727045547371179428886c9b...

• 

  Repository August 15th, 2010 @ 08:55 PM

  (from [1091a6e9b700bd713c8a6818761a27aa72b1fe93]) Allow for any possible TLD when using the :all option with the cookie session store. This works for subdomain.mysite.local, google.co.uk, google.com.au, etc. [#5147 state:resolved]

  Signed-off-by: José Valim jose.valim@gmail.com
  http://github.com/rails/rails/commit/1091a6e9b700bd713c8a6818761a27...