Defect #3692
open
URL-generation is brittle with trailing slashes
Added by Wolfgang Schnerring about 15 years ago.
Updated about 14 years ago.
Category:
Email notifications
Description
When the setting Adminstration/Settings/General/"Hostname and path" has a trailing slash (e. g. "http://example.com/redmine/", the links in the "issue updated" emails contain duplicate slashes (e. g. "http://example.com/redmine//repositories/revision/myproject/17") which cannot be routed and give 404 instead.
I've scrutinized the code, and the underlying reason is that url_for does not care about a trailing slash in the "host" part of the URL and simply appends the full path even if that duplicates the slash. I'm not sure whether it should do so, or if Redmine should be careful to strip any trailing slashes instead (in app/models/mailer.rb Mailer::default_url_options).
Either way, this behaviour is quite brittle and also rather non-obvious to figure out, so it would be nice if Redmine did the Right Thing(tm) regardless of trailing slashes in the "hostname" setting.
Files
Created a small patch to clean up trailing slash when using the mailer (in app/models/mailer.rb).
A simple regular expression used to cut the slash of if it exists.
Test added into mailer_test.rb (in test/unit/mailer_test.rb) named "test_generated_links_in_emails_with_trailing_slash".
All test ran without failures.
NOTE that using a ready-made library (e.g. UrlWriter / UrlRewriter if they support this kind of operation) might be better than doing a manual string modification, but it's debatable.
Patch created against redmine-trunk (rev. 3771).
People probably use Setting.hostname
throughout the code and in plugins. Maybe you want to put the patch there (into the Setting
model).
Holger Just wrote:
People probably use Setting.hostname
throughout the code and in plugins. Maybe you want to put the patch there (into the Setting
model).
I tested that also. I wanted this kind of feedback to decide where to do the modification.
The reason I chose mailer.rb is that this trailing-slash bug is only in the mailer. It seems that the rest of the system is able to fix this automatically.
I can post the setting-controller fix also if people want.
Holger Just wrote:
People probably use Setting.hostname
throughout the code and in plugins. Maybe you want to put the patch there (into the Setting
model).
Come to think of it, I agree with you. Better to remove the problem at the source rather than hoping that people will check for this in all modules.
I'll post a patch shortly.
I've created a patch which cuts of a trailing forward-slash of the host-name before saving it down into the model.
I could not find a clean way of doing this in the model class, but it is most likely possible.
I included additional test for the settings_controller to proof the functionality.
Attached is an alternative patch at model level. Although I'm not sure, which is better.
Also available in: Atom
PDF