From da009d8d7e9ce9749b8a502e99f3bb9d9c3d6636 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Thu, 22 Feb 2018 09:23:01 +0100 Subject: [PATCH] Fix autolinks in rinku and add specs --- Gemfile | 5 +- Gemfile.lock | 5 +- .../filters/autolink_filter.rb | 59 ++ .../filters/markdown_filter.rb | 2 +- .../formatters/markdown/formatter.rb | 10 +- .../matchers/link_handlers/colon_separator.rb | 14 +- .../markdown/markdown_formatting_spec.rb | 264 ++++++ .../text_formatting/markdown/markdown_spec.rb | 755 ++++++++++++++++++ 8 files changed, 1098 insertions(+), 16 deletions(-) create mode 100644 lib/open_project/text_formatting/filters/autolink_filter.rb create mode 100644 spec/lib/open_project/text_formatting/markdown/markdown_formatting_spec.rb create mode 100644 spec/lib/open_project/text_formatting/markdown/markdown_spec.rb diff --git a/Gemfile b/Gemfile index f73d589a860..018979a01b1 100644 --- a/Gemfile +++ b/Gemfile @@ -47,8 +47,6 @@ gem 'request_store', '~> 1.3.1' gem 'warden', '~> 1.2' gem 'warden-basic_auth', '~> 0.2.1' -# TODO: adds #auto_link which was deprecated in rails 3.1 -gem 'rails_autolink', '~> 1.1.6' gem 'will_paginate', '~> 3.1.0' gem 'friendly_id', '~> 5.2.1' @@ -78,6 +76,9 @@ gem 'commonmarker', '~> 0.17.5' gem 'html-pipeline', '~> 2.7.1' # HTML sanitization used for html-pipeline gem 'sanitize', '~> 4.5.0' +# HTML autolinking for mails and urls (replaces autolink) +gem 'rinku', '~> 2.0.4' + # generates SVG Graphs # used for statistics on svn repositories diff --git a/Gemfile.lock b/Gemfile.lock index 85c788574d3..a1d8e1c4b1a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -454,8 +454,6 @@ GEM rails_12factor (0.0.3) rails_serve_static_assets rails_stdout_logging - rails_autolink (1.1.6) - rails (> 3.1) rails_serve_static_assets (0.0.5) rails_stdout_logging (0.0.5) railties (5.0.6) @@ -492,6 +490,7 @@ GEM mime-types (>= 1.16, < 4.0) netrc (~> 0.8) retriable (3.0.2) + rinku (2.0.4) roar (1.1.0) representable (~> 3.0.0) rspec (3.5.0) @@ -705,13 +704,13 @@ DEPENDENCIES rails-angular-xss! rails-controller-testing (~> 1.0.2) rails_12factor - rails_autolink (~> 1.1.6) rdoc (>= 2.4.2) reform (~> 2.2.0) reform-rails (~> 0.1.7) request_store (~> 1.3.1) responders (~> 2.4) retriable (~> 3.0) + rinku (~> 2.0.4) roar (~> 1.1.0) rspec (~> 3.5.0) rspec-activemodel-mocks (~> 1.0.3)! diff --git a/lib/open_project/text_formatting/filters/autolink_filter.rb b/lib/open_project/text_formatting/filters/autolink_filter.rb new file mode 100644 index 00000000000..797372f8a70 --- /dev/null +++ b/lib/open_project/text_formatting/filters/autolink_filter.rb @@ -0,0 +1,59 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2017 the OpenProject Foundation (OPF) +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2017 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See doc/COPYRIGHT.rdoc for more details. +#++ +require 'rinku' + +module OpenProject::TextFormatting + module Filters + # HTML Filter for auto_linking urls in HTML. + # + # Context options: + # + # autolink: + # classes: (string) Classes to add to auto linked urls and mails + # enabled: (boolean) + # + # This filter does not write additional information to the context. + class AutolinkFilter < HTML::Pipeline::Filter + def call + autolink_context = default_autolink_options.merge context.fetch(:autolink, {}) + return doc if autolink_context[:enabled] == false + + ::Rinku.auto_link(html, :all, "class=\"#{autolink_context[:classes]}\"") + end + + def default_autolink_options + { + enabled: true, + classes: 'rinku-autolink', + } + end + end + end +end diff --git a/lib/open_project/text_formatting/filters/markdown_filter.rb b/lib/open_project/text_formatting/filters/markdown_filter.rb index fe96dffa125..291aa2b25d7 100644 --- a/lib/open_project/text_formatting/filters/markdown_filter.rb +++ b/lib/open_project/text_formatting/filters/markdown_filter.rb @@ -37,7 +37,7 @@ module OpenProject::TextFormatting options = [:GITHUB_PRE_LANG] options << :HARDBREAKS if context[:gfm] != false extensions = context.fetch :commonmarker_extensions, - %i[table strikethrough tagfilter autolink] + %i[table strikethrough tagfilter] html = CommonMarker.render_html(text, options, extensions) html.rstrip! diff --git a/lib/open_project/text_formatting/formatters/markdown/formatter.rb b/lib/open_project/text_formatting/formatters/markdown/formatter.rb index 3bdb6cb42a8..08137518cba 100644 --- a/lib/open_project/text_formatting/formatters/markdown/formatter.rb +++ b/lib/open_project/text_formatting/formatters/markdown/formatter.rb @@ -35,11 +35,14 @@ module OpenProject::TextFormatting::Formatters def initialize(context) @context = context - @pipeline = HTML::Pipeline.new(located_filters, context) + @pipeline = ::HTML::Pipeline.new(located_filters, context) end def to_html(text) - pipeline.to_html(text, context).html_safe + result = pipeline.call(text, context) + output = result[:output].to_s + + output.html_safe end def to_document(text) @@ -50,7 +53,8 @@ module OpenProject::TextFormatting::Formatters [ :markdown, :sanitization, - :pattern_matcher + :pattern_matcher, + :autolink ] end diff --git a/lib/open_project/text_formatting/matchers/link_handlers/colon_separator.rb b/lib/open_project/text_formatting/matchers/link_handlers/colon_separator.rb index ffb3a9d8f79..23f811beed1 100644 --- a/lib/open_project/text_formatting/matchers/link_handlers/colon_separator.rb +++ b/lib/open_project/text_formatting/matchers/link_handlers/colon_separator.rb @@ -70,7 +70,7 @@ module OpenProject::TextFormatting::Matchers end def render_version - if project && version = project.versions.visible.find_by(name: name) + if project && version = project.versions.visible.find_by(name: oid) link_to h(version.name), { only_path: context[:only_path], controller: '/versions', action: 'show', id: version }, class: 'version' @@ -78,7 +78,7 @@ module OpenProject::TextFormatting::Matchers end def render_commit - if project && project.repository && (changeset = Changeset.visible.where(['repository_id = ? AND scmid LIKE ?', project.repository.id, "#{name}%"]).first) + if project && project.repository && (changeset = Changeset.visible.where(['repository_id = ? AND scmid LIKE ?', project.repository.id, "#{oid}%"]).first) link_to h("#{project_prefix}#{name}"), { only_path: context[:only_path], controller: '/repositories', action: 'revision', project_id: project, rev: changeset.identifier }, class: 'changeset', @@ -88,11 +88,11 @@ module OpenProject::TextFormatting::Matchers def render_source if project && project.repository && User.current.allowed_to?(:browse_repository, project) - name =~ %r{\A[/\\]*(.*?)(@([0-9a-f]+))?(#(L\d+))?\z} + oid =~ %r{\A[/\\]*(.*?)(@([0-9a-f]+))?(#(L\d+))?\z} path = $1 rev = $3 anchor = $5 - link_to h("#{project_prefix}#{prefix}:#{name}"), + link_to h("#{project_prefix}#{prefix}:#{oid}"), { controller: '/repositories', action: 'entry', project_id: project, @@ -108,7 +108,7 @@ module OpenProject::TextFormatting::Matchers def render_attachment attachments = context[:attachments] || context[:object].try(:attachments) - if attachments && attachment = attachments.detect { |a| a.filename == name } + if attachments && attachment = attachments.detect { |a| a.filename == oid } link_to h(attachment.filename), { only_path: context[:only_path], controller: '/attachments', action: 'download', id: attachment }, class: 'attachment' @@ -118,7 +118,7 @@ module OpenProject::TextFormatting::Matchers def render_project p = Project .visible - .where(['projects.identifier = :s OR LOWER(projects.name) = :s', { s: name.downcase }]) + .where(['projects.identifier = :s OR LOWER(projects.name) = :s', { s: oid.downcase }]) .first if p link_to_project(p, { only_path: context[:only_path] }, class: 'project') @@ -126,7 +126,7 @@ module OpenProject::TextFormatting::Matchers end def render_user - if user = User.in_visible_project.find_by(login: name) + if user = User.in_visible_project.find_by(login: oid) link_to_user(user, class: 'user-mention') end end diff --git a/spec/lib/open_project/text_formatting/markdown/markdown_formatting_spec.rb b/spec/lib/open_project/text_formatting/markdown/markdown_formatting_spec.rb new file mode 100644 index 00000000000..7fe07360c74 --- /dev/null +++ b/spec/lib/open_project/text_formatting/markdown/markdown_formatting_spec.rb @@ -0,0 +1,264 @@ +#-- encoding: UTF-8 +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2018 the OpenProject Foundation (OPF) +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2017 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See docs/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' + +describe OpenProject::TextFormatting::Formatters::Markdown::Formatter do + MODIFIERS = { + '**' => 'strong', # bold + '_' => 'em', # italic + '~' => 'del' # deleted + } + + it 'should modifiers' do + assert_html_output( + '**bold**' => 'bold', + 'before **bold**' => 'before bold', + '**bold** after' => 'bold after', + '**two words**' => 'two words', + '**two*words**' => 'two*words', + '**two * words**' => 'two * words', + '**two** **words**' => 'two words', + '**(two)** **(words)**' => '(two) (words)' + ) + end + + it 'should modifiers combination' do + MODIFIERS.each do |m1, tag1| + MODIFIERS.each do |m2, tag2| + next if m1 == m2 + text = "#{m2}#{m1}Phrase modifiers#{m1}#{m2}" + html = "<#{tag2}><#{tag1}>Phrase modifiers" + assert_html_output text => html + end + end + end + + it 'should inline code' do + assert_html_output( + 'this is `some code`' => 'this is some code', + '``' => '<Location /redmine>' + ) + end + + it 'should escaping' do + assert_html_output( + 'this is a ' + } + + before do + project_2.reload + + wiki_page_2_1 = FactoryGirl.create :wiki_page_with_content, + wiki: project_2.wiki, + title: 'Start Page' + + project_2.wiki.pages << wiki_page_2_1 + project_2.wiki.start_page = 'Start Page' + project_2.wiki.save! + + project.wiki = wiki_1 + + wiki_1.pages << wiki_page_1_1 + wiki_1.pages << wiki_page_1_2 + wiki_1.pages << wiki_page_1_3 + end + + context 'Plain wiki link' do + subject { format_text('[[CookBook documentation]]') } + + it { is_expected.to be_html_eql("

CookBook documentation

") } + end + + context 'Arbitrary wiki link' do + title = '' + subject { format_text("[[#{title}]]") } + + it { is_expected.to be_html_eql("

#{h(title)}

") } + end + + context 'Plain wiki page link' do + subject { format_text('[[Another page|Page]]') } + + it { is_expected.to be_html_eql("

Page

") } + end + + context 'Wiki link with anchor' do + subject { format_text('[[CookBook documentation#One-section]]') } + + it { is_expected.to be_html_eql("

CookBook documentation

") } + end + + context 'Wiki page link with anchor' do + subject { format_text('[[Another page#anchor|Page]]') } + + it { is_expected.to be_html_eql("

Page

") } + end + + context 'Wiki link to an unknown page' do + subject { format_text('[[Unknown page]]') } + + it { is_expected.to be_html_eql("

Unknown page

") } + end + + context 'Wiki page link to an unknown page' do + subject { format_text('[[Unknown page|404]]') } + + it { is_expected.to be_html_eql("

404

") } + end + + context "Link to another project's wiki" do + subject { format_text('[[onlinestore:]]') } + + it { is_expected.to be_html_eql("

onlinestore

") } + end + + context "Link to another project's wiki with label" do + subject { format_text('[[onlinestore:|Wiki]]') } + + it { is_expected.to be_html_eql("

Wiki

") } + end + + context "Link to another project's wiki page" do + subject { format_text('[[onlinestore:Start page]]') } + + it { is_expected.to be_html_eql("

Start Page

") } + end + + context "Link to another project's wiki page with label" do + subject { format_text('[[onlinestore:Start page|Text]]') } + + it { is_expected.to be_html_eql("

Text

") } + end + + context 'Link to an unknown wiki page in another project' do + subject { format_text('[[onlinestore:Unknown page]]') } + + it { is_expected.to be_html_eql("

Unknown page

") } + end + + context 'Struck through link to wiki page' do + subject { format_text('-[[Another page|Page]]-') } + + it { is_expected.to be_html_eql("

Page

") } + end + + context 'Named struck through link to wiki page' do + subject { format_text('-[[Another page|Page]] link-') } + + it { is_expected.to be_html_eql("

Page link

") } + end + + context 'Escaped link to wiki page' do + subject { format_text('![[Another page|Page]]') } + + it { is_expected.to be_html_eql('

[[Another page|Page]]

') } + end + + context 'Link to wiki of non-existing project' do + subject { format_text('[[unknowproject:Start]]') } + + it { is_expected.to be_html_eql('

[[unknowproject:Start]]

') } + end + + context 'Link to wiki page of non-existing project' do + subject { format_text('[[unknowproject:Start|Page title]]') } + + it { is_expected.to be_html_eql('

[[unknowproject:Start|Page title]]

') } + end + end + + context 'Redmine links' do + let(:repository) do + FactoryGirl.build_stubbed :repository_subversion, project: project + end + let(:source_url) do + { controller: 'repositories', + action: 'entry', + project_id: identifier, + path: 'some/file' } + end + let(:source_url_with_ext) do + { controller: 'repositories', + action: 'entry', + project_id: identifier, + path: 'some/file.ext' } + end + + before do + allow(project).to receive(:repository).and_return(repository) + allow(User).to receive(:current).and_return(project_member) + allow(project_member) + .to receive(:allowed_to?) + .with(:browse_repository, project) + .and_return(true) + + @to_test = { + # source + 'source:/some/file' => link_to('source:/some/file', source_url, class: 'source'), + 'source:/some/file.' => link_to('source:/some/file', source_url, class: 'source') + '.', + 'source:/some/file.ext.' => link_to('source:/some/file.ext', source_url_with_ext, class: 'source') + '.', + 'source:/some/file. ' => link_to('source:/some/file', source_url, class: 'source') + '.', + 'source:/some/file.ext. ' => link_to('source:/some/file.ext', source_url_with_ext, class: 'source') + '.', + 'source:/some/file, ' => link_to('source:/some/file', source_url, class: 'source') + ',', + 'source:/some/file@52' => link_to('source:/some/file@52', source_url.merge(rev: 52), class: 'source'), + 'source:/some/file.ext@52' => link_to('source:/some/file.ext@52', source_url_with_ext.merge(rev: 52), class: 'source'), + 'source:/some/file#L110' => link_to('source:/some/file#L110', source_url.merge(anchor: 'L110'), class: 'source'), + 'source:/some/file.ext#L110' => link_to('source:/some/file.ext#L110', source_url_with_ext.merge(anchor: 'L110'), class: 'source'), + 'source:/some/file@52#L110' => link_to('source:/some/file@52#L110', source_url.merge(rev: 52, anchor: 'L110'), class: 'source'), + 'export:/some/file' => link_to('export:/some/file', source_url.merge(format: 'raw'), class: 'source download'), + # escaping + '!source:/some/file' => 'source:/some/file', + # invalid expressions + 'source:' => 'source:' + } + end + + it '' do + @to_test.each do |text, result| + expect(format_text(text)).to be_html_eql("

#{result}

") + end + end + end + + context 'Pre content should not parse wiki and redmine links' do + let(:wiki) { + FactoryGirl.create :wiki, + start_page: 'CookBook documentation', + project: project + } + let(:wiki_page) { + FactoryGirl.create :wiki_page_with_content, + wiki: wiki, + title: 'CookBook documentation' + } + let(:raw) { + <<-RAW +[[CookBook documentation]] + +##{issue.id} + +
+[[CookBook documentation]]
+
+##{issue.id}
+
+ RAW + } + + let(:expected) { + <<-EXPECTED +

CookBook documentation

+

##{issue.id}

+
+[[CookBook documentation]]
+
+##{issue.id}
+
+ EXPECTED + } + + before do + project.wiki = wiki + wiki.pages << wiki_page + end + + subject { format_text(raw) } + + it { is_expected.to be_html_eql(expected) } + end + + describe 'options' do + describe '#format' do + it 'uses format of Settings, if nothing is specified' do + expect(format_text('*Stars!*')).to be_html_eql('

Stars!

') + end + + it 'uses format of options, if specified' do + expect(format_text('*Stars!*', format: 'plain')).to be_html_eql('

*Stars!*

') + end + end + end + end + + describe 'macros within pre block' do + let(:wiki_text) { + <<-WIKI_TEXT + 
{{include(wiki)}}
+ + {{include(wiki)}} + WIKI_TEXT + } + + subject(:html) { format_text(wiki_text) } + + it 'does not expand the macro within 
' do
+      expect(html).to be_html_eql(%[
+        
{{ $root.DOUBLE_LEFT_CURLY_BRACE }}include(wiki)}}

+        

+           Error executing the macro include (Page not found) 
+        

+      ])
+    end
+  end
+
+  describe '{{toc}}', 'table of contents macro' do
+    # Source: http://en.wikipedia.org/wiki/Orange_(fruit)
+    let(:wiki_text) {
+      <<-WIKI_TEXT
+{{toc}}
+
+h1. Orange
+
+h2. Varietes
+
+h3. Common Oranges
+
+h4. Valencia
+
+h5. Naranjito
+
+h4. Hart's Tardiff Valencia
+
+h3. Navel Oranges
+
+h3. Blood Oranges
+
+h3. Acidless Oranges
+
+h2. Attributes
+
+      WIKI_TEXT
+    }
+
+    subject(:html) { format_text(wiki_text) }
+
+    context 'w/o request present' do
+      let(:request) { nil }
+
+      it 'emits a table of contents for headings h1-h4 with anchors' do
+        expect(html).to be_html_eql(%{
+          

+            
+              Table of Contents
+            
+            

+              
+            

+          

+        }).at_path('fieldset')
+      end
+    end
+  end
+end