Project

General

Profile

Actions

Defect #35441

closed

Inline image in Textile is not displayed if the image URL contains ampersands

Added by Brice Beaumesnil over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Text formatting
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

Description

Hello,

In my projects, i include Sonar Badges (images without extension) in project's description with

!!
(textile)

But since redmine 4.2 this include don't work anymore.
I test on new fresh redmine installation too (REDMINE 4.2.1 on ruby 2.7.3) : the same result, no image.

If i tried with markdown format : i have the image.

A Sonar Badget have this format : https://sonarqube.dev.mydomain/api/project_badges/measure?project=fr.mydomain.myproject%3Amyproject&metric=alert_status : no file extension (i think it's the problem because if i try an image with an extension, the image is include on the page)

This include works fine in redmine 4.1 and before

thanks


Files

redmine4.2.1-Ruby2.7.3.png (22.9 KB) redmine4.2.1-Ruby2.7.3.png Brice Beaumesnil, 2021-06-24 17:50
35441.patch (1.28 KB) 35441.patch Yuichi HARADA, 2021-07-02 04:56
35441-v2.patch (1.43 KB) 35441-v2.patch Go MAEDA, 2021-07-24 08:59

Related issues

Related to Redmine - Feature #31500: Ruby 2.7 supportClosedGo MAEDA

Actions
Related to Redmine - Defect #29681: "x%x%" is rendered as "&" in Textile formatterNew

Actions
Actions #1

Updated by Brice Beaumesnil over 2 years ago

I tried to see the result here :

On redmine 4.2.1, the image don't appear, I see all the textile code in place.

!https://www.gravatar.com/avatar/4a042b8382a008d344561c8301509f3a?s=32&d=identicon&r=PG!
Actions #2

Updated by Brice Beaumesnil over 2 years ago

Test on new installation of REDMINE 4.2.1 on Windows with Ruby 2.7.3

Actions #3

Updated by Go MAEDA over 2 years ago

  • Status changed from New to Confirmed

Thank you for reporting the issue. I have confirmed the issue on the latest trunk.

But the latest 4.2-stable branch (including released 4.2.1) is not affected. Maybe you are using the trunk or the master branch of GitHub, aren't you? Their version number is also "4.2.1" but has ".devel" suffix (e.g. "4.2.1.devel").

Actions #4

Updated by Brice Beaumesnil over 2 years ago

I used github 4.2-stable branch for my install

Environment:
Redmine version 4.2.1.stable
Ruby version 2.7.3-p183 (2021-04-05) [x64-mingw32]
Rails version 5.2.6
Environment production
Database adapter PostgreSQL
Mailer queue ActiveJob::QueueAdapters::AsyncAdapter
Mailer delivery smtp
SCM:
Subversion 1.8.16
Git 2.29.2
Filesystem
Redmine plugins:
no plugin installed

Actions #5

Updated by Yuichi HARADA over 2 years ago

In the case of Textile, RedCloth3#inline_textile_image(source:/trunk/lib/redmine/wiki_formatting/textile/redcloth3.rb#L950) is executed to generate the image tag, but the image could not be displayed because False is returned in Redmine::Helpers::URL#uri_with_safe_scheme?(source:/trunk/lib/redmine/helpers/url.rb#L25).
In uri_with_safe_scheme?, the string converted of the input URI (including query string) by RedCloth3#incomming_entries(source:/trunk/lib/redmine/wiki_formatting/textile/redcloth3.rb#L1001) is passed as a parameter. However, an error occurred because this conversion string could not be parsed by URI.parse of source:/trunk/lib/redmine/helpers/url.rb#L31 .
The following patch removes the query string from the conversion string.

diff --git a/lib/redmine/helpers/url.rb b/lib/redmine/helpers/url.rb
index 0c6cbdecd7..40801ee7c8 100644
--- a/lib/redmine/helpers/url.rb
+++ b/lib/redmine/helpers/url.rb
@@ -28,7 +28,7 @@ module Redmine
         return true unless uri.to_s.include? ":" 

         # Other URLs need to be parsed
-        schemes.include? URI.parse(uri).scheme
+        schemes.include? URI.parse(uri.split('?').first).scheme
       rescue URI::Error
         false
       end
Actions #6

Updated by Go MAEDA over 2 years ago

I found that the issue is reproducible when the version of Ruby is 2.7 or higher. This is because URI.parse in Ruby 2.7 or later raises URI::InvalidURIError if the query string in a given URI is invalid.

As explained in #35441#note-5, the Textile formatter temporarily replaces "&" in URIs with two "x%" (source:tags/4.2.1/lib/redmine/wiki_formatting/textile/redcloth3.rb#L1001). The conversion makes a URI with a query string malformed. The malformed URI causes URI::InvalidURIError while checking if the URI scheme is safe at source:tags/4.2.1/lib/redmine/helpers/url.rb#L31.

It has never caused the reported problem before because URI.parse in Ruby 2.6 or earlier allows URIs with such a malformed query string.

Actions #7

Updated by Go MAEDA over 2 years ago

  • File 35441-v2.patch 35441-v2.patch added
  • Subject changed from include image without extension failed in redmine 4.2 to Inline image in Textile is not displayed if the image URL contains ampersands
  • Target version set to Candidate for next minor release

Update the patch.

  • Add a test that describes the issue to ApplicationHelperTest#test_inline_images
  • Remove a query string in RedCloth3#inline_textile_image instead of Redmine::Helpers::URL.uri_with_safe_scheme?. This is because the cause of the issue is the temporary data conversion of RedCloth3#inline_textile_image and Markdown formatter is not affected by the issue at all
  • Replaced String#split with faster String#partition
Actions #8

Updated by Go MAEDA over 2 years ago

  • Target version changed from Candidate for next minor release to 4.2.2

Setting the target version to 4.2.2.

Actions #9

Updated by Go MAEDA over 2 years ago

Actions #10

Updated by Mischa The Evil over 2 years ago

I did some research on the background of this change in Ruby to see what changed exactly and why, and if it was an intentional change or a regression bug.

To summarize:
Actions #11

Updated by Go MAEDA over 2 years ago

  • Status changed from Confirmed to Resolved
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed the patch. Thank you for your contribution.

Mischa The Evil wrote:

I did some research on the background of this change in Ruby to see what changed exactly and why, and if it was an intentional change or a regression bug.

To summarize:

Thank you for your detailed investigation.

Actions #12

Updated by Go MAEDA over 2 years ago

  • Status changed from Resolved to Closed
Actions #13

Updated by Mischa The Evil over 2 years ago

  • Related to Defect #29681: "x%x%" is rendered as "&" in Textile formatter added
Actions

Also available in: Atom PDF