Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #7947 (new defect)

Opened 3 years ago

Last modified 3 years ago

[PATCH] ssl_requirement may not redirect from https to http in some cases

Reported by: nfstern Assigned to: core
Priority: normal Milestone: 2.x
Component: Plugins Version: edge
Severity: normal Keywords: ssl_requirement
Cc:

Description

I encountered this in my development environment using InstantRails, RadRails, Lighty, Windows XP, IE 6 & Firefox. I am running my application on port 3000.

Basically, I have a login screen that I set ssl_required for. No problem there. The problem was after I successfully logged in, I was getting a 404 page not found error when the plugin redirected it to http.

This patch fixed the problem for me: def ensure_proper_protocol

return true if ssl_allowed?

if ssl_required? && !request.ssl?

redirect_to "https://" + request.host + request.request_uri return false

elsif request.ssl? && !ssl_required?

redirect_to RAILS_ENV == ('production') ? "http://" + request.host + request.request_uri : "http://" + request.host + ":3000" + request.request_uri return false

end

end

Probably, the port should not be hardcoded like this, but it fixed the problem.

Attachments

7947_ssl_requirement_host_with_port.diff (3.3 kB) - added by jonahb on 04/09/07 07:01:08.
Patch

Change History

03/30/07 03:03:36 changed by nfstern

  • keywords set to ssl_requirement.

Observe the 2nd redirect clause interrogating for the environment.

04/09/07 07:01:08 changed by jonahb

  • attachment 7947_ssl_requirement_host_with_port.diff added.

Patch

04/09/07 07:19:32 changed by jonahb

  • summary changed from ssl_requirement may not redirect from https to http in some cases to [PATCH] ssl_requirement may not redirect from https to http in some cases.

This bug occurs because SSL Requirement omits the port number in the redirect URL, i.e. it assumes people will be using the standard HTTP and HTTPS ports, even in the development environment.

The patch makes two key changes:

1) In the development environment, SSL Requirement assumes port 3000 for both HTTP and HTTPS.

2) Non-standard ports (i.e. not 80 for HTTP or not 443 for HTTPS) are included in the redirect URL.

Users who are using ports other than 3000 in the development environment or ports other than the standard ones in non-development environments can override the protected port_for_protocol method.

Tests were not passing because they tested the HTTP header "location" instead of "Location". I've changed them to test TestResponse.redirect_url, which accesses the correct header.

04/09/07 15:55:43 changed by nfstern

I think you may want to change this

# Override if you are using a non-standard http or https port def port_for_protocol(protocol)

if ENVRAILS_ENV? == "development"

3000

else

standard_port_for_protocol(protocol)

end

end

to this

# Override if you are using a non-standard http or https port def port_for_protocol(protocol)

if ENVRAILS_ENV? == "production"

standard_port_for_protocol(protocol)

else

3000

end

end

The reason being twofold. I think typically, the testsuite is run on the same box as the development box and therefore also uses port 3000. The 2nd reason is that when running in production, it saves a cpu cycle or two & this is more important in production mode than in development or test mode.

Noah

04/09/07 19:27:44 changed by jonahb

I don't see how the change you suggest would save any cycles in production.

Although tests are usually run on the dev box, port 80 is the default port in the test environment. See TestProcess.initialize_default_values in action_controller/test_process.rb

04/09/07 19:41:27 changed by nfstern

It saves you the else. Trivial, but it's one less instruction in production.