Defect #19304
closed
<a> tag without attributes in description results in undefined method + for nil:NilClass
0%
Description
Observed Behavior
Create new issue with description "<a>link.com</a>" (without tag attributes)
ActionView::Template::Error: undefined method `+' for nil:NilClass
[RAILS_ROOT]/app/helpers/application_helper.rb:872:in `block in parse_redmine_links'
[RAILS_ROOT]/app/helpers/application_helper.rb:741:in `gsub!'
[RAILS_ROOT]/app/helpers/application_helper.rb:741:in `parse_redmine_links'
[RAILS_ROOT]/app/helpers/application_helper.rb:583:in `block (2 levels) in textilizable'
Workaround
http://www.redmine.org/projects/redmine/repository/revisions/14003/entry/trunk/app/helpers/application_helper.rb#L741
<a( [^>]+?)?>(.*?)</a> should be <a( [^>]+?)>(.*?)</a>
Environment:
Redmine version 3.0.0.stable
Ruby version 2.1.5-p273 (2014-11-13) [i386-mingw32]
Rails version 4.2.0
Environment production
Database adapter Mysql2
Files
Updated by Pavel Rosický over 9 years ago
Updated by Jean-Philippe Lang over 9 years ago
- Status changed from New to Needs feedback
- Resolution set to Cant reproduce
I'm not able to reproduce, could you write a test case for this?
Updated by Pavel Rosický over 9 years ago
I forgot, that standard textile formatters unescape html tags before, but this method should handle this anyway. Test case attached.
Original result: error
With patch: returns nil as it should
Updated by Toshi MARUYAMA over 9 years ago
- Status changed from Needs feedback to New
- Target version set to 3.0.2
- Resolution deleted (
Cant reproduce)
Updated by Toshi MARUYAMA over 9 years ago
- Related to Defect #18301: Revision shortlink at end of URL breaks URL autolinking added
Updated by Toshi MARUYAMA over 9 years ago
- Related to deleted (Defect #18301: Revision shortlink at end of URL breaks URL autolinking)
Updated by Toshi MARUYAMA over 9 years ago
- Status changed from New to Needs feedback
- Target version deleted (
3.0.2) - Resolution set to Cant reproduce
I cannot reproduce too.
diff --git a/test/unit/helpers/application_helper_test.rb b/test/unit/helpers/application_helper_test.rb
--- a/test/unit/helpers/application_helper_test.rb
+++ b/test/unit/helpers/application_helper_test.rb
@@ -1261,6 +1261,19 @@ RAW
end
end
+ def test_should_handle_a_tag_without_attributes
+ text = '<a>http://example.com</a>'
+ with_settings :text_formatting => 'textile' do
+ assert_not_nil textilizable(text)
+ end
+ with_settings :text_formatting => 'markdown' do
+ assert_not_nil textilizable(text)
+ end
+ with_settings :text_formatting => 'unknown' do
+ assert_not_nil textilizable(text)
+ end
+ end
+
def test_due_date_distance_in_words
to_test = { Date.today => 'Due in 0 days',
Date.today + 1 => 'Due in 1 day',
Updated by Pavel Rosický about 9 years ago
Hi, this bug is still present, but you can reproduce it only if you use non-redmine custom text formatter which supports html (like ckeditor, markdown/textile formatter escape brackets, so <a> will be &\lt;a&\gt; and the affected regex will not fail).
To avoid that I need to patch this method and gsub! these occurances, which causes unnecessary overhead. Or I can patch the whole method (~140 lines!!! definetely worth to refactor) only because of one character.
Can someone take a look at it again? Patch, test-case and backtrace are already attached.
Updated by Toshi MARUYAMA about 9 years ago
Pavel Rosický wrote:
Hi, this bug is still present, but you can reproduce it only if you use non-redmine custom text formatter which supports html (like ckeditor, markdown/textile formatter escape brackets, so <a> will be <a> and the affected regex will not fail).
Could you add test cases for it?
Updated by Pavel Rosický about 9 years ago
Toshi MARUYAMA wrote:
Could you add test cases for it?
I already did, 7 months ago.
Test case - http://www.redmine.org/attachments/13295/application_helper_test.rb.patch :
def test_should_handle_a_tag_without_attributes
text = '<a>http://example.com</a>'
assert_nil parse_redmine_links(text, nil, nil, nil, true, {})
end
Without patch:
1) Error:
ApplicationHelperTest#test_should_handle_a_tag_without_attributes:
NoMethodError: undefined method `+' for nil:NilClass
app/helpers/application_helper.rb:873:in `block in parse_redmine_links'
app/helpers/application_helper.rb:742:in `gsub!'
app/helpers/application_helper.rb:742:in `parse_redmine_links'
test/unit/helpers/application_helper_test.rb:1262:in `test_should_handle_a_t
ag_without_attributes'
94 runs, 379 assertions, 0 failures, 1 errors, 0 skips
With patch - http://www.redmine.org/attachments/13280/application_helper.rb.patch :
94 runs, 380 assertions, 0 failures, 0 errors, 0 skips
What else can I do?
If you want to test textilizable(), I'll have to include ckeditor and a custom html formatter (plugin). Such tests can't be included in a standard Redmine's test suite.
Updated by Toshi MARUYAMA about 9 years ago
Pavel Rosický wrote:
If you want to test textilizable(), I'll have to include ckeditor and a custom html formatter (plugin).
Could you add dummy formatter?
Updated by Jean-Philippe Lang about 9 years ago
- Category changed from Text formatting to Code cleanup/refactoring
- Status changed from Needs feedback to Closed
- Assignee set to Jean-Philippe Lang
- Target version set to 3.2.0
Fixed in r14774.