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 5 days ago
- Related to Defect #31831: Back url parse in validation added
Updated by Go MAEDA 5 days 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 4 days 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 4 days 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.