Defect #35441

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

Added by Brice Beaumesnil 3 months ago. Updated about 1 month ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Text formatting
Target version:4.2.2
Resolution:Fixed Affected version:4.2.1

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

redmine4.2.1-Ruby2.7.3.png (22.9 KB) Brice Beaumesnil, 2021-06-24 17:50

35441.patch Magnifier (1.28 KB) Yuichi HARADA, 2021-07-02 04:56

35441-v2.patch Magnifier (1.43 KB) Go MAEDA, 2021-07-24 08:59


Related issues

Related to Redmine - Feature #31500: Ruby 2.7 support Closed
Related to Redmine - Defect #29681: "x%x%" is rendered as "&" in Textile formatter New

Associated revisions

Revision 21101
Added by Go MAEDA about 1 month ago

Inline image in Textile is not displayed if the image URL contains ampersands (#35441).

Contributed by Yuichi HARADA.

Revision 21108
Added by Go MAEDA about 1 month ago

Merged r21101 from trunk to 4.2-stable (#35441).

History

#1 Updated by Brice Beaumesnil 3 months 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!

#2 Updated by Brice Beaumesnil 3 months ago

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

#3 Updated by Go MAEDA 3 months 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").

#4 Updated by Brice Beaumesnil 3 months 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

#5 Updated by Yuichi HARADA 3 months 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

#6 Updated by Go MAEDA about 1 month 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.

#7 Updated by Go MAEDA about 1 month ago

  • File 35441-v2.patchMagnifier 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

#8 Updated by Go MAEDA about 1 month ago

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

Setting the target version to 4.2.2.

#9 Updated by Go MAEDA about 1 month ago

#10 Updated by Mischa The Evil about 1 month 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:

#11 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. 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.

#12 Updated by Go MAEDA about 1 month ago

  • Status changed from Resolved to Closed

#13 Updated by Mischa The Evil about 1 month ago

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

Also available in: Atom PDF