Defect #41930
closedRedirection after signing in fails when the back_url includes a port number
0%
Description
If URI contains a port, I cannot redirect to back_url after login.
If back_uri contains port, ApplicationController#validate_back_url raises Addressable::URI::InvalidURIError and returns false.
So, hidden field of back_url is not rendered at '/login'.
The validation method should remove the port before the host.
[:scheme, :host, :port].each do |component|
if uri.send(component).present? && uri.send(component) != request.send(component)
return false
end
uri.send(:"#{component}=", nil) # => raises InvalidURIError
end
Files
Related issues
Updated by Go MAEDA about 1 month ago
- Status changed from New to Confirmed
- Target version set to Candidate for next minor release
As reported, I have confirmed the issue where passing a URL containing a port number, such as http://localhost:3000/admin
, to the ApplicationController#validate_back_url
method results in false
being returned.
Inside the validate_back_url
method, there is a process that removes the protocol, hostname, and port number from the provided URL. Specifically, the following code is responsible for this:
uri = Addressable::URI.parse(back_url)
[:scheme, :host, :port].each do |component|
if uri.send(component).present? && uri.send(component) != request.send(component)
return false
end
uri.send(:"#{component}=", nil)
end
In this code, the protocol, hostname, and port number are removed sequentially from the Addressable::URI
object created from the given URL. However, when processing in this order, an exception occurs when attempting to remove the hostname. This happens because removing the hostname leaves the URL in a state where it has a port number but no hostname, resulting in the following exception:
Addressable::URI::InvalidURIError: Hostname not supplied: '/admin'
As suggested in the proposed patch, this issue can be resolved by removing the port number before removing the hostname. This adjustment ensures that the URL remains valid during the process.
Updated by Go MAEDA about 1 month ago
Here is another solution.
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index a3eaec4bb..9056e1603 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -511,11 +511,10 @@ class ApplicationController < ActionController::Base
if uri.send(component).present? && uri.send(component) != request.send(component)
return false
end
-
- uri.send(:"#{component}=", nil)
end
- # Always ignore basic user:password in the URL
- uri.userinfo = nil
+
+ # Remove the scheme, user:password, host, and port from the URL
+ uri.omit!(:scheme, :userinfo, :host, :port)
rescue Addressable::URI::InvalidURIError
return false
end
Updated by Go MAEDA about 1 month ago
- Target version changed from Candidate for next minor release to 6.0.3
Setting the target version to 6.0.3.
Updated by Go MAEDA about 1 month ago
- File 41930-v2.patch 41930-v2.patch added
- Affected version set to 6.0.0
Here is a new patch. I have updated the code to use the omit!
method, as I suggested in #note-3, to remove unnecessary components from the URI object. By using omit!
to remove multiple components at once, we no longer need to worry about the order in which the components are removed.
Additionally, the current code has a bug where it throws an exception when a URL contains Basic authentication information (e.g., http://user:password@redmine.example.net
) during the uri.send(:"host=")
step. This patch also fixes that issue.
Updated by Go MAEDA about 1 month ago
- Status changed from Confirmed to Resolved
- Assignee set to Go MAEDA
- Resolution set to Fixed
Committed the patch in r23465. Thank you for your contribution.
Updated by Go MAEDA about 1 month ago
- Status changed from Resolved to Closed
Merged the fix into the 6.0-stable branch in r23467.