Fix redis-rack for rack 2.0.8 #50

le0pard · 2019-12-19T13:55:02Z

This version need rack >= 2.0.8

le0pard · 2019-12-19T13:55:26Z

This should fix this issue redis-store/redis-store#324

le0pard · 2019-12-19T15:34:28Z

WIP, because still in rails app tests get session as string like this:

Failure/Error: get sidekiq_path

     NoMethodError:
       undefined method `private_id' for "7ce9207a5fa61b1a49c5c87c879fd772":String
       Did you mean?  private_methods
     # /Users/leo/Downloads/redis-rack/lib/rack/session/redis.rb:57:in `block (2 levels) in delete_session'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/connection_pool-2.2.2/lib/connection_pool.rb:65:in `block (2 levels) in with'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/connection_pool-2.2.2/lib/connection_pool.rb:64:in `handle_interrupt'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/connection_pool-2.2.2/lib/connection_pool.rb:64:in `block in with'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/connection_pool-2.2.2/lib/connection_pool.rb:61:in `handle_interrupt'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/connection_pool-2.2.2/lib/connection_pool.rb:61:in `with'
     # /Users/leo/Downloads/redis-rack/lib/redis/rack/connection.rb:20:in `with'
     # /Users/leo/Downloads/redis-rack/lib/rack/session/redis.rb:82:in `with'
     # /Users/leo/Downloads/redis-rack/lib/rack/session/redis.rb:56:in `block in delete_session'
     # /Users/leo/Downloads/redis-rack/lib/rack/session/redis.rb:70:in `with_lock'
     # /Users/leo/Downloads/redis-rack/lib/rack/session/redis.rb:55:in `delete_session'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/rack-2.0.8/lib/rack/session/abstract/id.rb:371:in `commit_session'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/rack-2.0.8/lib/rack/session/abstract/id.rb:261:in `context'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/rack-2.0.8/lib/rack/session/abstract/id.rb:253:in `call'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/actionpack-6.0.2.1/lib/action_dispatch/middleware/cookies.rb:648:in `call'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/actionpack-6.0.2.1/lib/action_dispatch/middleware/callbacks.rb:27:in `block in call'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/activesupport-6.0.2.1/lib/active_support/callbacks.rb:101:in `run_callbacks'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/actionpack-6.0.2.1/lib/action_dispatch/middleware/callbacks.rb:26:in `call'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/actionpack-6.0.2.1/lib/action_dispatch/middleware/actionable_exceptions.rb:17:in `call'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/actionpack-6.0.2.1/lib/action_dispatch/middleware/debug_exceptions.rb:32:in `call'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/actionpack-6.0.2.1/lib/action_dispatch/middleware/show_exceptions.rb:33:in `call'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/railties-6.0.2.1/lib/rails/rack/logger.rb:38:in `call_app'
     # /Users/leo/.rvm/gems/ruby-2.6.3/gems/railties-6.0.2.1/lib/rails/rack/logger.rb:26:in `block in call'

Maybe you @tubbo have ideas

tubbo

I was not able to reproduce your use case. Here's what I did:

$ rails new testapp --no-rc
# wait for bundle to install
$ cd testapp
$ bundle add redis-rack sidekiq

I then changed config/routes.rb to the following:

require 'sidekiq/web'

Rails.application.routes.draw do
  mount Sidekiq::Web, at: '/sidekiq', as: :sidekiq
end

And added an integration test into test/integration/sidekiq_integration_test.rb...

require 'test_helper'

class SidekiqIntegrationTest < ActionDispatch::IntegrationTest
  test 'sidekiq' do
    get sidekiq_path

    assert_response :success
  end
end

This test passed. I am not using a connection pool, and from the backtrace it looks like maybe you are?

test/rack/session/redis_test.rb

le0pard · 2019-12-19T16:28:27Z

@tubbo you need add devise in route:

authenticate :user do
    mount Sidekiq::Web => '/sidekiq', as: :sidekiq
end

It will fail after login in tests:

context 'admin user' do
      let(:user) { FactoryBot.create(:user, role: 'admin') }

      before do
        login_as(user, scope: user.class.name.underscore.to_sym)
        get sidekiq_path
      end

      subject { response }

      it { is_expected.to be_successful }
      it { is_expected.to have_http_status(:ok) }
    end

le0pard · 2019-12-19T16:32:56Z

Ok, I found, that we also need this change, so it will work with new rack 2.0.8 and rails changes: redis-store/redis-actionpack#28

tubbo · 2019-12-19T16:35:10Z

~~I changed my test and routes to the following, but still didn't get the expected failure...~~

  include Devise::Test::IntegrationHelpers

  test 'sidekiq' do
    user = users(:one)

    login_as user, scope: user.class.name.underscore.to_sym
    get sidekiq_path

    assert_response :success
  end

require 'sidekiq/web'

Rails.application.routes.draw do
  devise_for :users
  authenticate :user do
    mount Sidekiq::Web, at: '/sidekiq', as: :sidekiq
  end
end

update: since you found that the problem was in redis-actionpack, and i am not using redis-actionpack, that would make sense :)

le0pard · 2019-12-19T16:49:49Z

@tubbo rollback tests. Now I am not sure, how to deal with this changes, because it is clearly "broken changes" for redis-rack and redis-actionpack gems

But I checked, both this changes for redis-rack and redis-actionpack works with new rails 6.0.2.1 and rack 2.0.8.

So you should decide what to do next with all of this.

le0pard · 2019-12-19T20:26:42Z

@tubbo BTW, if you need any additional help for this changes in gems - just ping me (I am very interesting, that this gems will maintained)

P.S. Looks like need release version, where

redis-rack need rack >= 2.0.6
redis-actionpack need this new redis-rack

johan-smits · 2020-01-02T12:48:28Z

When will this be merged?

hasghari · 2020-01-06T05:25:51Z

@le0pard @tubbo Any ETA on merging and releasing this change?

le0pard · 2020-01-06T05:32:42Z

@hasghari I am not repo owner or contributor, so I don't know ETA

tubbo · 2020-01-13T19:38:37Z

@hasghari @le0pard @johan-smits Hey folks, sorry for the delay...holidays and then getting back from holidays had me real busy. This all looks good to me!

gingerlime · 2020-01-14T05:48:40Z

I upgraded to redis-rack 2.1.0 and now get these errors :(

hasghari · 2020-01-14T05:52:09Z

@gingerlime I was facing the same issue. You likely need to upgrade to redis-actionpack:5.2.0

gingerlime · 2020-01-14T05:55:31Z

Thanks @hasghari seems to do the trick 👍

johan-smits · 2020-01-14T07:54:49Z

@tubbo for me with both packages updated I get this error:

le0pard · 2020-01-14T07:56:34Z

@johan-smits read this comment rack/rack#1432 (comment)

johan-smits · 2020-01-14T08:00:39Z

@le0pard thnx, missed that issue.

Hoping it will be fixed in the next release I added: gem 'rack', '!= 2.1.0', '!= 2.1.1' to my Gemfile.

Looooong · 2020-02-06T10:05:50Z

I still get this error undefined method 'private_id' for #<String:0x000055deef339018> when visiting Sidekiq UI.

rails 5.2.4.1
redis-actionpack 5.2.0
redis-rack 2.1.0
rack 2.1.2, 2.0.8
devise 4.7.1
sidekiq 5.2.7

Routes:

  authenticate :user do
    mount Sidekiq::Web, at: '/sidekiq'
  end

tubbo · 2020-02-06T16:18:07Z

@Looooong can you paste the full backtrace?

Looooong · 2020-02-07T03:29:45Z

@tubbo Here is the backtrace:

NoMethodError - undefined method `private_id' for #<String:0x000055d964857aa8>
Did you mean?  private_methods:
  redis-rack (2.1.0) lib/rack/session/redis.rb:49:in `block (2 levels) in write_session'
  redis-rack (2.1.0) lib/redis/rack/connection.rb:22:in `with'
  redis-rack (2.1.0) lib/rack/session/redis.rb:82:in `with'
  redis-rack (2.1.0) lib/rack/session/redis.rb:49:in `block in write_session'
  redis-rack (2.1.0) lib/rack/session/redis.rb:70:in `with_lock'
  redis-rack (2.1.0) lib/rack/session/redis.rb:48:in `write_session'
  rack (2.1.2) lib/rack/session/abstract/id.rb:391:in `commit_session'
  rack (2.1.2) lib/rack/session/abstract/id.rb:271:in `context'
  rack (2.1.2) lib/rack/session/abstract/id.rb:263:in `call'
  actionpack (5.2.4.1) lib/action_dispatch/middleware/cookies.rb:670:in `call'
  activerecord (5.2.4.1) lib/active_record/migration.rb:559:in `call'
  actionpack (5.2.4.1) lib/action_dispatch/middleware/callbacks.rb:28:in `block in call'
  activesupport (5.2.4.1) lib/active_support/callbacks.rb:98:in `run_callbacks'
  actionpack (5.2.4.1) lib/action_dispatch/middleware/callbacks.rb:26:in `call'
  actionpack (5.2.4.1) lib/action_dispatch/middleware/executor.rb:14:in `call'
  better_errors (2.5.1) lib/better_errors/middleware.rb:84:in `protected_app_call'
  better_errors (2.5.1) lib/better_errors/middleware.rb:79:in `better_errors_call'
  better_errors (2.5.1) lib/better_errors/middleware.rb:57:in `call'
  airbrake (10.0.1) lib/airbrake/rack/middleware.rb:34:in `call!'
  airbrake (10.0.1) lib/airbrake/rack/middleware.rb:23:in `call'
  actionpack (5.2.4.1) lib/action_dispatch/middleware/debug_exceptions.rb:61:in `call'
  actionpack (5.2.4.1) lib/action_dispatch/middleware/show_exceptions.rb:33:in `call'
  railties (5.2.4.1) lib/rails/rack/logger.rb:38:in `call_app'
  railties (5.2.4.1) lib/rails/rack/logger.rb:26:in `block in call'
  activesupport (5.2.4.1) lib/active_support/tagged_logging.rb:71:in `block in tagged'
  activesupport (5.2.4.1) lib/active_support/tagged_logging.rb:28:in `tagged'
  activesupport (5.2.4.1) lib/active_support/tagged_logging.rb:71:in `tagged'
  railties (5.2.4.1) lib/rails/rack/logger.rb:26:in `call'
  ahoy_matey (3.0.1) lib/ahoy/engine.rb:22:in `call_with_quiet_ahoy'
  sprockets-rails (3.2.1) lib/sprockets/rails/quiet_assets.rb:13:in `call'
  actionpack (5.2.4.1) lib/action_dispatch/middleware/remote_ip.rb:81:in `call'
  request_store (1.5.0) lib/request_store/middleware.rb:19:in `call'
  actionpack (5.2.4.1) lib/action_dispatch/middleware/request_id.rb:27:in `call'
  rack (2.1.2) lib/rack/method_override.rb:24:in `call'
  rack (2.1.2) lib/rack/runtime.rb:24:in `call'
  activesupport (5.2.4.1) lib/active_support/cache/strategy/local_cache_middleware.rb:29:in `call'
  actionpack (5.2.4.1) lib/action_dispatch/middleware/executor.rb:14:in `call'
  actionpack (5.2.4.1) lib/action_dispatch/middleware/static.rb:127:in `call'
  rack (2.1.2) lib/rack/sendfile.rb:113:in `call'
  rack-cors (1.1.1) lib/rack/cors.rb:100:in `call'
  secure_headers (6.3.0) lib/secure_headers/middleware.rb:11:in `call'
  railties (5.2.4.1) lib/rails/engine.rb:524:in `call'
  puma (4.3.1) lib/puma/configuration.rb:228:in `call'
  puma (4.3.1) lib/puma/server.rb:681:in `handle_request'
  puma (4.3.1) lib/puma/server.rb:472:in `process_client'
  puma (4.3.1) lib/puma/server.rb:328:in `block in run'
  puma (4.3.1) lib/puma/thread_pool.rb:134:in `block in spawn_thread'

lib/rack/session/redis.rb

Fix redis-rack for rack 2.0.8

34e21dd

le0pard requested a review from tubbo as a code owner December 19, 2019 13:55

Fix redis-rack for rack 2.0.8, fix generate_unique_sid

389d813

le0pard changed the title ~~Fix redis-rack for rack 2.0.8~~ [WIP] Fix redis-rack for rack 2.0.8 Dec 19, 2019

Fix redis-rack for rack 2.0.8, small improvements

2e58985

tubbo requested changes Dec 19, 2019

View reviewed changes

test/rack/session/redis_test.rb Outdated Show resolved Hide resolved

le0pard changed the title ~~[WIP] Fix redis-rack for rack 2.0.8~~ Fix redis-rack for rack 2.0.8 Dec 19, 2019

back tests to original state

5a297ce

tubbo self-requested a review December 19, 2019 19:05

tubbo approved these changes Dec 19, 2019

View reviewed changes

le0pard mentioned this pull request Dec 20, 2019

After security update for rack (2.0.8) redis-store raise errors redis-store/redis-store#324

Closed

jdurand mentioned this pull request Jan 7, 2020

Possible issue with 2.0.8 security fix rack/rack#1432

Closed

borama mentioned this pull request Jan 10, 2020

Address CVE-2019-16782 rails/activerecord-session_store#151

Merged

tubbo approved these changes Jan 13, 2020

View reviewed changes

tubbo merged commit 51d6948 into redis-store:master Jan 13, 2020

le0pard deleted the rack_208 branch January 13, 2020 20:11

worldlywisdom mentioned this pull request Mar 18, 2020

2.1.2 - NoMethodError - undefined method `private_id' #55

Closed

stanhu reviewed Mar 21, 2020

View reviewed changes

lib/rack/session/redis.rb Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix redis-rack for rack 2.0.8 #50

Fix redis-rack for rack 2.0.8 #50

le0pard commented Dec 19, 2019 •

edited

Loading

le0pard commented Dec 19, 2019

le0pard commented Dec 19, 2019

tubbo left a comment

le0pard commented Dec 19, 2019

le0pard commented Dec 19, 2019 •

edited

Loading

tubbo commented Dec 19, 2019 •

edited

Loading

le0pard commented Dec 19, 2019 •

edited

Loading

le0pard commented Dec 19, 2019 •

edited

Loading

johan-smits commented Jan 2, 2020

hasghari commented Jan 6, 2020

le0pard commented Jan 6, 2020

tubbo commented Jan 13, 2020

gingerlime commented Jan 14, 2020

hasghari commented Jan 14, 2020

gingerlime commented Jan 14, 2020

johan-smits commented Jan 14, 2020

le0pard commented Jan 14, 2020

johan-smits commented Jan 14, 2020 •

edited

Loading

Looooong commented Feb 6, 2020 •

edited

Loading

tubbo commented Feb 6, 2020

Looooong commented Feb 7, 2020

Fix redis-rack for rack 2.0.8 #50

Fix redis-rack for rack 2.0.8 #50

Conversation

le0pard commented Dec 19, 2019 • edited Loading

le0pard commented Dec 19, 2019

le0pard commented Dec 19, 2019

tubbo left a comment

Choose a reason for hiding this comment

le0pard commented Dec 19, 2019

le0pard commented Dec 19, 2019 • edited Loading

tubbo commented Dec 19, 2019 • edited Loading

le0pard commented Dec 19, 2019 • edited Loading

le0pard commented Dec 19, 2019 • edited Loading

johan-smits commented Jan 2, 2020

hasghari commented Jan 6, 2020

le0pard commented Jan 6, 2020

tubbo commented Jan 13, 2020

gingerlime commented Jan 14, 2020

hasghari commented Jan 14, 2020

gingerlime commented Jan 14, 2020

johan-smits commented Jan 14, 2020

le0pard commented Jan 14, 2020

johan-smits commented Jan 14, 2020 • edited Loading

Looooong commented Feb 6, 2020 • edited Loading

tubbo commented Feb 6, 2020

Looooong commented Feb 7, 2020

le0pard commented Dec 19, 2019 •

edited

Loading

le0pard commented Dec 19, 2019 •

edited

Loading

tubbo commented Dec 19, 2019 •

edited

Loading

le0pard commented Dec 19, 2019 •

edited

Loading

le0pard commented Dec 19, 2019 •

edited

Loading

johan-smits commented Jan 14, 2020 •

edited

Loading

Looooong commented Feb 6, 2020 •

edited

Loading