Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reset session if env present, may be prevent for hijacking #21388

Closed

Conversation

Gaurav2728
Copy link
Contributor

cc @tenderlove Please review..!!

@rafaelfranca
Copy link
Member

Could you explain better why this change is made?

@Gaurav2728
Copy link
Contributor Author

Rails/Rack Session will expire when any env. should present. If request will come from any external source (CSRF/attackers) then we need to check env. where it is comming from. Like middleware_stack will respond according to ActionDispatch::Request.new env @rafaelfranca

@rafaelfranca
Copy link
Member

I still don't get it, why we don't want to reset the session with a request? Is not env always present?

@Gaurav2728
Copy link
Contributor Author

It'll reset the session when any env is available. I try to say session will reset for particular env where request is come from. Rails should response the request when --->

 if env.present?
   Http::Headers.from_hash(request_env).merge!(env)
end

But request will come from proxy --->

Rack::Request.new(RAILS_ENV=nil, RACK_ENV =nil)

@rafaelfranca
Copy link
Member

Could you create a test case to demonstrate the issue that this code is
fixing?

On Tue, Sep 1, 2015, 05:22 Gaurav Sharma notifications@github.com wrote:

It'll reset the session when any env is available. I try to say session
will reset for particular env where request is come from. Rails should
response the request when --->

if env.present?
Http::Headers.from_hash(request_env).merge!(env)
end

But request will come from proxy --->

Rack::Request.new(RAILS_ENV=nil, RACK_ENV =nil)


Reply to this email directly or view it on GitHub
#21388 (comment).

@Gaurav2728
Copy link
Contributor Author

@rafaelfranca i wrote the test case but WARNING: env is deprecated and will be removed from Rails 5.0 So this change NO longer needed. Thanks for your response. 😄

@Gaurav2728 Gaurav2728 closed this Sep 1, 2015
@Gaurav2728 Gaurav2728 deleted the gaurav-actionpack_need_env branch December 28, 2015 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants