This project is archived and is in readonly mode.

#1957 ✓resolved
Nolan Eakins

Sessions break in 2.3 with Mongrel

Reported by Nolan Eakins | February 13th, 2009 @ 01:25 AM

Starting Mongrel with the following command while using Rails v2.3.0 (git tag) and cookie stored sessions prevents any updates to my session from getting communicated back to the browser.

The only test I have at the moment is to start Mongrel using "mongrel_rails start -d --port=3001 --environment=development --pid pwd/tmp/pids/mongrel_selenium.pid" and to do something such as a login.

I don't have this problem using Passenger, Thin, or ./script/server with Mongrel. Scott Taylor posted a comment regarding his experience with this issue over at http://rails.lighthouseapp.com/p...

My Mongrel version is 1.1.5.

Comments and changes to this ticket

  • Scott Taylor

    Scott Taylor February 13th, 2009 @ 01:48 AM

    See the comments in #1823 for more info

  • Scott Taylor

    Scott Taylor February 13th, 2009 @ 04:51 AM

    • Tag changed from 2.3, actionpack, bugs, cookiestore, sessions to 2.3, actionpack, bug, bugs, cookiestore, patch, sessions

    Nolan - Can you try out the patch I'm attaching to this ticket?

    It looks like the Set-Cookie header isn't being set properly. Mongrel expects an options hash like {:Set-Cookie => "var=value"}, but occasionally an array will be given instead, like so: {"Set-Cookie" => ["var=value", "var2=value"]}

    Looks like thin handles these nested values. Mongrel doesn't, and ends up sending the header "Set-Cookie: " - i.e., with an empty value.

  • Scott Taylor

    Scott Taylor February 13th, 2009 @ 04:56 AM

    Actually, try the following patch. The code is a bit cleaner.

  • Nolan Eakins

    Nolan Eakins February 14th, 2009 @ 01:26 AM

    Well Scott, your patch seemed to fix the issue I was having. Congrats.

    Now to annoy a comitter...

  • josh

    josh February 14th, 2009 @ 03:44 AM

    • Milestone cleared.
    • Assigned user set to “josh”
  • Tim Pope

    Tim Pope February 14th, 2009 @ 09:44 AM

    The real issue (at least that I'm seeing) is that multiple cookies in a single response are concatenated into a single malformed Set-Cookie header (as if Array#to_s is called) rather than separate headers. The patch prevents this by giving the last cookie precedence but at the cost of limiting a response to setting a single cookie.

    I've yet to find the proper fix for this issue but -1 on this patch.

  • Nolan Eakins

    Nolan Eakins February 14th, 2009 @ 06:51 PM

    • Assigned user cleared.

    I'm starting to look at this a little deeper. I'm seeing a ton of duplicated cookie building code in ActionPack and Rack. Namely in:

    • Rails actionpack/lib/action_controller/response.rb actionpack/lib/action_controller/session/cookie_store.rb
    • Rack: rack/lib/rack/response.rb

    Not sure why there needs to be three implementations that at first glance seem to do the same.

  • Nolan Eakins

    Nolan Eakins February 14th, 2009 @ 06:52 PM

    • Assigned user set to “josh”

    Oops. Cleared the assigned user on this ticket.

  • Nolan Eakins

    Nolan Eakins February 14th, 2009 @ 06:59 PM

    Looks like actionpack/lib/action_controller/session/abstract_store.rb also includes the same cookie building snippet as those files. Definitely getting off topic here, but I'm seeing this all over the place:

              cookie = Rack::Utils.escape(@key) + '=' + Rack::Utils.escape(sid)
              cookie << "; domain=#{options[:domain]}" if options[:domain]
              cookie << "; path=#{options[:path]}" if options[:path]
              if options[:expire_after]
                expiry = Time.now + options[:expire_after]
                cookie << "; expires=#{expiry.httpdate}"
              end
              cookie << "; Secure" if options[:secure]
              cookie << "; HttpOnly" if options[:httponly]
    
              headers = response[1]
              case a = headers[SET_COOKIE]
              when Array
                a << cookie
              when String
                headers[SET_COOKIE] = [a, cookie]
              when nil
                headers[SET_COOKIE] = cookie
              end
            end
    
  • Nolan Eakins

    Nolan Eakins February 14th, 2009 @ 08:06 PM

    More poking. My finger is tempted to point at the mongrel_rails command for not using Rack. Did come across http://www.ruby-forum.com/topic/... which is a thread about this exact issue.

  • Josh Pencheon

    Josh Pencheon February 14th, 2009 @ 09:32 PM

    I commented on Scott's ticket, but I seem to be having a similar problem with Passenger.

  • josh

    josh February 15th, 2009 @ 12:27 AM

    There were some 1.9 spec changes in Rack recently. Headers need to be strings instead of arrays.

    
              cookie = build_cookie(@key, cookie.merge(options))
              unless headers[HTTP_SET_COOKIE].blank?
                headers[HTTP_SET_COOKIE] << "\n#{cookie}"
              else
                headers[HTTP_SET_COOKIE] = cookie
              end
    

    Not sure how this changes things. Can you please try again with edge rails.

  • Nolan Eakins

    Nolan Eakins February 15th, 2009 @ 12:35 AM

    Joshua, I just tried master with mongrel_rails and no improvement there. I should give Mongrel's head a try, but a no go w/ 1.1.5 which given my poking around earlier today I'm assuming is not using Rack.

  • David Reese

    David Reese February 15th, 2009 @ 05:03 PM

    On edge rails (238a6bb62dc153743a0abc6eb1e35392ac799d65) and passenger (2.0.6, osx), I still have problems setting any kind of cookies. On edge last week (as of feb 10), I had the same problem running mongrel; today, mongrel sets cookies where passenger fails.

    == the details...

    Literally spent hours one day last week trying to debug this, and got nowhere. Watching the cookies get set with the charles proxy, my impression was that the header was malformed -- too many newlines I think -- and the cookie header ended up in the content. Testing now again with edge seems to support that -- I'm actually getting "Set-Cookie: _myapp_session=BAh7CToPc2Vzc2lv..." displayed in the browser, before the <!DOCTYPE. !

    But there are so many moving parts (rails/rack/passenger) I wasn't sure where to look to narrow down the problem. Hopefully you know where to look? Not sure if I'm doing anything weird -- it's a pretty vanilla resful_auth setup.

  • Scott Taylor

    Scott Taylor February 16th, 2009 @ 01:23 AM

    I take Tim Pope's and Nolan's comments to heart. I'm not super fond of the patch, and there is a crapload of duplicated code (almost all of the session cookie store and the abstract session store are duplicated code).

    I'll look into searching for all of the places where a cookie may be built into an array, which is the real issue.

    Does the http spec allow sending multiple Set-Cookie headers?

  • josh

    josh February 16th, 2009 @ 09:23 PM

    • State changed from “new” to “open”

    Confirmed. Working on issue.

    The fix should be applied to ActionController::CGIHandler

    
    $ curl -I localhost:3000/hello
    HTTP/1.1 200 OK
    Connection: close
    Date: Mon, 16 Feb 2009 21:21:35 GMT
    X-Runtime: 2
    ETag: "bea8252ff4e80f41719ea13cdf007273"
    X-Test: 1
    Content-Type: text/html; charset=utf-8
    Cache-Control: private, max-age=0, must-revalidate
    Content-Length: 14
    
    $ curl -I localhost:3000/hello
    HTTP/1.1 200 OK
    Connection: close
    Date: Mon, 16 Feb 2009 21:21:54 GMT
    Status: 200
    ETag: "bea8252ff4e80f41719ea13cdf007273"
    X-Runtime: 5
    Cache-Control: private, max-age=0, must-revalidate
    X-Test: 1
    2
    3
    Server: Mongrel 1.1.5
    Content-Type: text/html; charset=utf-8
    Set-Cookie: 
    Content-Length: 14
    
  • Repository

    Repository February 17th, 2009 @ 04:18 AM

    • State changed from “open” to “resolved”

    (from [b6e56efe07cb3c2e999216f995403aa9206226a2]) Special case in deprecated CGI proxy layer for Mongrel CGI cookies [#1957 state:resolved] http://github.com/rails/rails/co...

  • Nolan Eakins

    Nolan Eakins February 17th, 2009 @ 05:10 AM

    Good job there Josh. Checked out "master" and mongrel_rails allowed me to login. Seems to fix my issue. Now I just need a reason to drop Thin. :-)

  • David Reese

    David Reese February 17th, 2009 @ 05:23 AM

    Didn't fix what I assumed was the same issue -- the session isn't working in Passenger either, which Josh Pencheon and I commented on above.

    On edge after that last commit (b6e56), I still get the "Set-Cookie" header showing up in the rendered HTML. Firefox 3, OSX, Passenger.

    If I should open a new ticket ("Sessions break in 2.3 with Passenger"), I will, but I don't know much more than I mentioned in my last comment.

  • josh

    josh February 17th, 2009 @ 05:33 AM

    @David The latest version version of passenger does not support the new rack 1.9 spec yet. I'll hook up with Hongli and we'll get this in.

  • David Reese

    David Reese February 17th, 2009 @ 05:43 AM

    Thanks josh, I missed that somehow.

    So, all passenger users will have to upgrade (to a future Passenger version) to run 2.3? We should put that in writing somewhere, right? Just so nobody trying to help test 2.3 wastes any time trying to fix their cookies (like i did).

  • josh

    josh February 17th, 2009 @ 05:58 AM

    David, I'm not sure. It may be possible for them to release a backwards and forward compat version of passenger. We'll see.

    Watch the passenger blog for updates: http://blog.phusion.nl/

  • David Reese

    David Reese February 17th, 2009 @ 05:44 PM

    @Joshua -- it's a marketing/documentation thing more than anything -- that nowhere on the blog post about the Rails 2.3 beta, or on the release notes, or the changelog even, does it say anything about the session issue with Passenger. I'm not sure where I file a bug on the release notes?

    Let me put it another way -- the 2.3RC1 blog post says, "So please help us do thorough testing of this release candidate. Lots of the underpinnings changed. Especially the move to Rack." So how many other people like me are trying it out, trying to be helpful and tracking down bugs, and then getting stuck on the passenger issue? Like, maybe there should be a "Known issues" list or something.

    Just trying to be helpful here, let me know if there's something I'm missing.

  • Luigi Montanez

    Luigi Montanez February 18th, 2009 @ 07:40 PM

    The Phusion guys just fixed the Passenger problem:

    http://github.com/FooBarWidget/p...

    Just clone the repo and follow the README. Hurrah!

  • Ryan Bigg

    Ryan Bigg October 9th, 2010 @ 09:45 PM

    • Tag cleared.
    • Importance changed from “” to “Low”

    Automatic cleanup of spam.

  • Jeff Kreeftmeijer
  • bingbing

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Attachments

Referenced by

Pages