evan / mongrel

Mongrel
http://rubygems.org/gems/mongrel
Other
144 stars 18 forks source link

Session/cookies don't work with Rails 2.3.8 and mongrel_rails #5

Open triemstr opened 14 years ago

triemstr commented 14 years ago

I started my application with linux 2.6, mongrel 1.1.6 (from http://gems.rubyinstaller.org/), mongrel_rails and rails 2.3.8 and was not able to use cookies...no logins, no authenticity form tokens, no nothing requiring cookie usage...basically no cookies are stored in the session at all which I confirmed in my browser.

Interestingly, if I startup with script/server, mongrel boots up and cookies work great.

Also, everything worked either way (mongrel_rails and script/server with mongrel) under rails 2.3.5.

luislavena commented 14 years ago

Hello.

I've just created an application using 2.3.8, mongrel 1.2.0.pre2

http://rubygems.org/gems/mongrel/versions/1.2.0.pre2

And using a simple posts scaffold everything worked out fine.

Do you have a more concrete example? mongrel_rails seems to work fine in my environment, usage of cookies seems to store and retrieve them without issues.

Please provide more feedback.

Thank you.

triemstr commented 14 years ago

Hi Luis,

With an upgraded mongrel 1.2.0pre2 and still with rails 2.3.8, cookies 'appeared' to work. I actually can find them in my browser now. So at least "part" of the issue with mongrel 1.1.6 is solved for me.

However, I have a new problem:

I have a redirect after a user is effectively logged in. The production log shows the redirect being sent and assigned a 502 status code (?). Once again, when I use script/server with mongrel 1.2.0pre2, I do not have this problem - only mongrel_rails. I am routing through apache 2 first to mongrel and I do have another server instance with rails 2.3.5 & mongrel 1.1.6 still running also on the same box. They share gems and perhaps there is something screwed up between the two.

I'm out of town for several days...unless you have an idea, I'd like to perform a few more tests before I give up. I'll log what I find on this issue by the end of the weekend.

triemstr commented 14 years ago

I do see this: http://weblog.rubyonrails.org/2010/5/25/ruby-on-rails-2-3-8-released

See point 12 and 59, another redirect problem.

Also this podcast reports mongrel_rails and cluster problems: http://ruby5.envylabs.com/episodes/82-episode-80-may-28-2010?utm_source=feedburner&utm_medium=feed&utm_campaign=Feed%3A+Ruby5+(Ruby5)

I imagine you've found other people with problems by now, and it looks like rails 2.3.9 may solve it. (?)

triemstr commented 14 years ago

Ok I actually had a chance to finish my testing. I was able to get rails 2.3.8 to work with passenger and I believe a week ago I had it working ok with thin. It is just the mongrel_rails version + rails 2.3.8 that has a problem. If you have any questions, let me know...otherwise, I think I'll wait for rails 2.3.9 before trying mongrel again.

luislavena commented 14 years ago

This is weird, but if Rails ppl say it doesn't work, then it doesn't :P

I'm going to check with EngineYard (one of the biggest users of mongrel_rails) and see if they want to cook a solution for this, as the specific internals of Mongrel associated with this are beyond my expertise and available time for OSS.

MikieC commented 14 years ago

Ran into this issue and my first attempt at fixing was the simple fix found here:

http://github.com/fauna/mongrel/blob/master/lib/mongrel/cgi.rb

with this (most recent?) code in send_cookie:

    else
      to['Set-Cookie'] = head['cookie'].to_s
    end 

The problem is the cookie does not get set correctly. The cookie is a string of cookies like so: "cookie1=382FEEDA382; cookie2=yes; HttpOnly" Setting this as a value just concatenates the same value into one gigantic cookie.

I posted the solution that finally worked for me here:

https://rails.lighthouseapp.com/projects/8994/tickets/4690

It works for my site and the browser shows the cookies properly getting set. I only program Ruby/Rails as a hobby, so I can't say the code is perfect nor the solution. At least it works though.

foresth commented 14 years ago

+1 for the issue with mongrel_rails on 2.3.8 and 1.1.5 mongrel

luislavena commented 14 years ago

Sorry, +1 means that you like it or that you actually tried and worked for you?

bshand commented 14 years ago

MikieC is mostly right: the code in fauna/mongrel only works if a single cookie is set. However, the cookies are separated by newlines, and in his example above, "HttpOnly" is in fact part of cookie2.

I've put an updated patched into https://rails.lighthouseapp.com/projects/8994/tickets/4690

luislavena commented 14 years ago

Hey guys,

Tom Trebisky approached with the following change for send_cookies:

   def send_cookies(to)
     # convert the cookies based on the myriad of possible ways to set a cookie
     if @head['cookie']
       cookie = @head['cookie']
       case cookie
       when Array
         cookie.each {|c| to['Set-Cookie'] = c.to_s }
       when Hash
         cookie.each_value {|c| to['Set-Cookie'] = c.to_s}
       else
         to['Set-Cookie'] = cookie.to_s
       end

       @head.delete('cookie')
     end

     # @output_cookies seems to never be used, but we'll process it just in case
     @output_cookies.each {|c| to['Set-Cookie'] = c.to_s } if @output_cookies
   end

Thoughts?

bshand commented 14 years ago

No, Tom Trebisky's change does not work for our application, which sets 2 cookies: (a) Tom's change embeds extra newlines in the HTTP headers, and (b) setting multiple cookies with a single to['Set-Cookie'] does not work.

However, Ken Collins' mongrel.rb initializer (lighthouse ticket 4690) does work correctly for us (with a minor fix for RubyGems 1.3.5 support): https://rails.lighthouseapp.com/projects/8994/tickets/4690

luislavena commented 14 years ago

@bshand,

Monkeypatching is fine, but I would appreciate a test.

If you look at header method will see that cookie is left out intentionally for later been processed by send_cookies.

I haven't encountered this problem myself, so I couldn't write a test that expose the issue.

If you guys do it, will commit a workable fix.

Thank you.

bilditup1 commented 14 years ago

Ken Collins' monkeypatch is also applicable to 1.20pre; just comment out the first and last lines.

luislavena commented 14 years ago

@bilditup1: Nice, can you show me some test? Monkeypatching is not something good and it broke some existing tests.

bilditup1 commented 14 years ago

@luislavena- I started rails just a few wks ago. I haven't yet got up to speed on tests, but if you have a good quick resource that tells me how, I'll try to help out.

luislavena commented 14 years ago

The power is in the source:

http://github.com/fauna/mongrel/blob/master/test/test_cgi_wrapper.rb

woahdae commented 13 years ago

Man, looked into this bug and fell into a wormhole. Thought I'd help anyone else that comes along.

The OP could be talking about one of 2 possibly interrelated bugs, it seems, both discussed in https://rails.lighthouseapp.com/projects/8994/tickets/4690.

The nil.split issue from that ticket's OP comes from a bug where rack sets keys to nil values, and will return true on include?, resolved in a later version: https://github.com/rack/rack/commit/b937c01bd16243276b24845efea411963c9f2747

Somehow related, https://github.com/rails/rails/commit/b6e56efe was to turn the Set-Cookie string into a 'cookie' array specifically for deprecated mongrel handling.

The above rails commit seems to not be doing what it used to, and 'Set-Cookie' is now a string, which triggers a bug fixed two years ago in https://github.com/fauna/mongrel/commit/7c9d988d4de2e08d67f95ca209196427fd89c9af#diff-10 , but remaining in Mongrel 1.1.5

My take is that all of these issues have patches in recent versions of the projects, but some change in Rails 2.3.8(+) no longer handles Set-Cookie for the "deprecated mongrel cgi proxy layer" as it used to (there weren't any tests associated with that change to rails, so nobody would notice if that happened). The proposed monkey patches seem to be in line with the intent of the current git branches of Mongrel and Rack, although in production I would personally just sidestep the issue and not use mongrel_rails 1.1.5 with rack 1.1.0.

I'm still fuzzy on why 2.3.8+ interacts with mongrel_rails now and didn't before, but hopefully this summary helps someone.

woahdae commented 13 years ago

Re my own "why did it break now?," figured it out. Details:

https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/4690-mongrel-doesnt-work-with-rails-238#ticket-4690-59