From 5f9f7f667442540aac3399995c684b2a24afb9b2 Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Mon, 23 Sep 2024 16:12:46 +0200 Subject: [PATCH] Simplify markdown preview - Do not let unauthenticated people render a markdown preview - Remove Haml::Filters monkey patch, it just replaces some text. Do that in a MarkdownHelper instead. - Do not render markdown with MarkdownHelper, use a haml partial instead --- app/controllers/markdown_controller.rb | 11 +----- app/helpers/application_helper.rb | 8 ----- app/helpers/markdown_helper.rb | 21 ++++++++--- app/views/comments/_comment.html.haml | 4 +-- app/views/comments/_form.html.haml | 2 +- app/views/keywords/edit.html.haml | 2 +- app/views/markdown/_preview.html.haml | 2 ++ app/views/markdown/preview.js.erb | 2 +- app/views/projects/_form.html.haml | 2 +- app/views/projects/show.html.haml | 2 +- lib/haml/filters/markdown.rb | 36 ------------------- spec/controllers/markdown_controller_spec.rb | 13 ++++--- spec/helpers/markdown_helper_spec.rb | 27 +++++++------- spec/lib/haml/filters/markdown_spec.rb | 7 ---- .../shared_examples/a_markdown_renderer.rb | 24 ------------- 15 files changed, 48 insertions(+), 115 deletions(-) create mode 100644 app/views/markdown/_preview.html.haml delete mode 100644 lib/haml/filters/markdown.rb delete mode 100644 spec/lib/haml/filters/markdown_spec.rb delete mode 100644 spec/support/shared_examples/a_markdown_renderer.rb diff --git a/app/controllers/markdown_controller.rb b/app/controllers/markdown_controller.rb index 93d4d3b6f..137ed1ef1 100644 --- a/app/controllers/markdown_controller.rb +++ b/app/controllers/markdown_controller.rb @@ -1,14 +1,5 @@ class MarkdownController < ApplicationController - skip_before_action :authenticate_user!, only: [:preview] - respond_to :js - def preview - if params[:source] - markdown_source = params[:source].to_str.gsub(/(?<=^|\s):([\w+-]+):(?=\s|$)/) do |match| - %(![add-emoji](https://github.githubassets.com/images/icons/emoji/#{match.to_str.tr(':', '')}.png)) - end - end - @rendered = MarkdownHelper.render markdown_source - respond_with @rendered + @markdown_source = helpers.enrich_markdown(markdown: params[:source]) end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 2be35b8eb..98599ad6d 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -21,12 +21,4 @@ def active_page_size(page_size, param = nil) 'active' end end - - def emojify(content) - if content.present? - content.to_str.gsub(/(?<=^|\s):([\w+-]+):(?=\s|$)/) do |match| - %(![add-emoji](https://github.githubassets.com/images/icons/emoji/#{match.to_str.tr(':', '')}.png)) - end.html_safe - end - end end diff --git a/app/helpers/markdown_helper.rb b/app/helpers/markdown_helper.rb index 0ec16acb8..a81529e7d 100644 --- a/app/helpers/markdown_helper.rb +++ b/app/helpers/markdown_helper.rb @@ -1,9 +1,22 @@ module MarkdownHelper - def self.render(markdown_source) - Haml::Filters::Markdown.new.render markdown_source + def mdpreview(markdown_source, lines: 3) + markdown_source.lines[0..lines - 1].join end - def mdpreview(markdown_source, lines: 3) - markdown_source.lines.grep_v(/\[comment\]/).grep(/\S/)[0..lines - 1].join + def enrich_markdown(markdown:) + # replace :smiley: with a link to github.com emojis + markdown.gsub!(/(?<=^|\s):([\w+-]+):(?=\s|$)/) do |match| + %(![add-emoji](https://github.githubassets.com/images/icons/emoji/#{match.to_str.tr(':', '')}.png)) + end + # replace @hans with a link to the user with the login hans + markdown.gsub!(/([^\w]|^)@([-\w]+)([^\w]|$)/) do + "#{Regexp.last_match(1)}[@#{Regexp.last_match(2)}](#{::Rails.application.routes.url_helpers(only_path: true).user_path(Regexp.last_match(2))})#{Regexp.last_match(3)}" + end + # replace hw#my-project with a link to the project with the slug my-project + markdown.gsub!(/([^\w]|^)hw#([-\w]+)([^\w]|$)/) do + "#{Regexp.last_match(1)}[hw##{Regexp.last_match(2)}](#{::Rails.application.routes.url_helpers(only_path: true).project_path(Regexp.last_match(2))})#{Regexp.last_match(3)}" + end + + markdown end end diff --git a/app/views/comments/_comment.html.haml b/app/views/comments/_comment.html.haml index ce1c010cf..8533196da 100644 --- a/app/views/comments/_comment.html.haml +++ b/app/views/comments/_comment.html.haml @@ -16,7 +16,7 @@ Edit %p :markdown - #{ emojify comment.text } + #{ enrich_markdown(markdown: comment.text) } - if !comment.comments.empty? %ul.media-list = render :partial => 'comments/comment', :collection => comment.comments, object: comment @@ -46,7 +46,7 @@ .modal-body %p :markdown - #{ emojify comment.text } + #{ enrich_markdown(markdown: comment.text) } %hr #replyform = render partial: 'comments/form', locals: { comment: @new_comment, parent: comment, id: rand(36**10).to_s(36).upcase[0,5] } diff --git a/app/views/comments/_form.html.haml b/app/views/comments/_form.html.haml index 61a9e868c..4ff6643d1 100644 --- a/app/views/comments/_form.html.haml +++ b/app/views/comments/_form.html.haml @@ -13,7 +13,7 @@ .comment-form-body .tab-content .tab-pane.active.fade.in{ role: 'tab-pane', id: "markdown-source#{id}" } - = f.text_area :text, :placeholder => "Your comment. You can use markdown.", :class => 'form-control input-lg markdown-source-text', :required => "required" + = f.text_area :text, :placeholder => "Your comment. You can use markdown.", :class => 'form-control input-lg', :required => "required" .tab-pane.fade{ role: 'tab-pane', id: "markdown-preview#{id}" } .loading-spinner = icon('fas', 'spinner pulse 3x') diff --git a/app/views/keywords/edit.html.haml b/app/views/keywords/edit.html.haml index c2ae5ac60..9d684d8d7 100644 --- a/app/views/keywords/edit.html.haml +++ b/app/views/keywords/edit.html.haml @@ -15,7 +15,7 @@ .form-group = f.label('Description (maximum 255 characters)') - = f.text_area :description, maxlength: "255", rows: 5, id: 'keyword_description', class: 'form-control input-lg markdown-source-text' + = f.text_area :description, maxlength: "255", rows: 5, id: 'keyword_description', class: 'form-control input-lg' .form-group = f.label('Keyword Logo (Will be resized to 150x150 Pixels)') = f.file_field :avatar diff --git a/app/views/markdown/_preview.html.haml b/app/views/markdown/_preview.html.haml new file mode 100644 index 000000000..df2172a4f --- /dev/null +++ b/app/views/markdown/_preview.html.haml @@ -0,0 +1,2 @@ +:markdown + #{markdown_source} diff --git a/app/views/markdown/preview.js.erb b/app/views/markdown/preview.js.erb index 5c46d7ff0..bcc3864ba 100644 --- a/app/views/markdown/preview.js.erb +++ b/app/views/markdown/preview.js.erb @@ -1,4 +1,4 @@ -$('#<%= params[:form_parent]%> .preview-contents').html("<%=j raw @rendered %>"); +$('#<%= params[:form_parent]%> .preview-contents').html("<%= escape_javascript(render partial: 'preview', locals: { markdown_source: @markdown_source }) %>"); $('#<%= params[:form_parent]%> .loading-spinner').addClass('hidden'); $('#<%= params[:form_parent]%> .preview-contents').removeClass('hidden'); $('input[name="authenticity_token"]').val('<%= form_authenticity_token %>'); diff --git a/app/views/projects/_form.html.haml b/app/views/projects/_form.html.haml index 8de360557..c6f875d2f 100644 --- a/app/views/projects/_form.html.haml +++ b/app/views/projects/_form.html.haml @@ -28,7 +28,7 @@ #markdown-source.tab-pane.active.fade.in{ role: 'tab-pane' } .form-group = f.text_area :description, rows: 20, id: 'project_description', - class: 'form-control input-lg markdown-source-text' + class: 'form-control input-lg' #markdown-preview.tab-pane.fade{ role: 'tab-pane' } .loading-spinner = icon('fas', 'spinner pulse 3x') diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index d70c09c6a..15a7b1022 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -27,7 +27,7 @@ .row .col-sm-8.project-style :markdown - #{ emojify @project.description } + #{ enrich_markdown(markdown: @project.description) } .col-sm-4 .row .col-sm-12 diff --git a/lib/haml/filters/markdown.rb b/lib/haml/filters/markdown.rb deleted file mode 100644 index 8c6c5256e..000000000 --- a/lib/haml/filters/markdown.rb +++ /dev/null @@ -1,36 +0,0 @@ -require 'redcarpet' -require 'rouge/plugins/redcarpet' - -module Haml - class Filters - class Markdown < TiltBase - # remove_filter('Markdown') # remove the existing Markdown filter - - class RougeRender < ::Redcarpet::Render::HTML - include Rouge::Plugins::Redcarpet - end - - def render(text) - # replace @user with a link to user - text.gsub!(/([^\w]|^)@([-\w]+)([^\w]|$)/) do - "#{Regexp.last_match(1)}[@#{Regexp.last_match(2)}](#{::Rails.application.routes.url_helpers(only_path: true).user_path(Regexp.last_match(2))})#{Regexp.last_match(3)}" - end - # replace hw#slug with a link to project - text.gsub!(/([^\w]|^)hw#([-\w]+)([^\w]|$)/) do - "#{Regexp.last_match(1)}[hw##{Regexp.last_match(2)}](#{::Rails.application.routes.url_helpers(only_path: true).project_path(Regexp.last_match(2))})#{Regexp.last_match(3)}" - end - - renderer_options = { escape_html: true, - safe_links_only: true } - markdown_options = { fenced_code_blocks: true, - disable_indented_code_blocks: true, - autolink: true } - Redcarpet::Markdown.new(RougeRender.new(renderer_options), markdown_options).render(text) - end - - def compile(node) - compile_with_tilt(node, 'markdown') - end - end - end -end diff --git a/spec/controllers/markdown_controller_spec.rb b/spec/controllers/markdown_controller_spec.rb index 606ab10be..138bdc304 100644 --- a/spec/controllers/markdown_controller_spec.rb +++ b/spec/controllers/markdown_controller_spec.rb @@ -1,14 +1,13 @@ require 'rails_helper' RSpec.describe MarkdownController, type: :controller do - describe 'GET #preview' do - it 'correctly assigns rendered html' do - source = '*italic*' - - get :preview, xhr: true, params: { source: source } + render_views - expect(response).to be_successful - expect(assigns(:rendered)).to eq "

italic

\n" + describe 'GET #preview' do + it 'renders a markdown preview' do + sign_in create :user + get :preview, xhr: true, params: { source: '**hans**' } + expect(response.body).to include('$(\'# .preview-contents\').html("

hans<\/strong><\/p>\n\n");') end end end diff --git a/spec/helpers/markdown_helper_spec.rb b/spec/helpers/markdown_helper_spec.rb index 59a7b48fa..8fddf1903 100644 --- a/spec/helpers/markdown_helper_spec.rb +++ b/spec/helpers/markdown_helper_spec.rb @@ -1,17 +1,20 @@ require 'rails_helper' -# Specs in this file have access to a helper object that includes -# the MarkdownHelper. For example: -# -# describe MarkdownHelper do -# describe "string concat" do -# it "concats two strings with spaces" do -# expect(helper.concat_strings("this","that")).to eq("this that") -# end -# end -# end RSpec.describe MarkdownHelper, type: :helper do - describe '#render' do - it_behaves_like 'a markdown renderer' + describe '.enrich_markdown' do + it 'translates emoji' do + text = 'I need :coffee: so badly, working openSUSE:Factory:Staging:F' + expect(enrich_markdown(markdown: text)).to eq('I need ![add-emoji](https://github.githubassets.com/images/icons/emoji/coffee.png) so badly, working openSUSE:Factory:Staging:F') + end + + it 'translate @user links' do + text = 'Hey @hans, how are you?' + expect(enrich_markdown(markdown: text)).to eq('Hey [@hans](/users/hans), how are you?') + end + + it 'translates hw#slug links' do + text = 'Have you seen hw#super-cool? Its awesome' + expect(enrich_markdown(markdown: text)).to eq('Have you seen [hw#super-cool](/projects/super-cool)? Its awesome') + end end end diff --git a/spec/lib/haml/filters/markdown_spec.rb b/spec/lib/haml/filters/markdown_spec.rb deleted file mode 100644 index b268e5cef..000000000 --- a/spec/lib/haml/filters/markdown_spec.rb +++ /dev/null @@ -1,7 +0,0 @@ -require 'rails_helper' - -describe Haml::Filters::Markdown do - describe '#render' do - it_behaves_like 'a markdown renderer' - end -end diff --git a/spec/support/shared_examples/a_markdown_renderer.rb b/spec/support/shared_examples/a_markdown_renderer.rb deleted file mode 100644 index 4f02b2ed3..000000000 --- a/spec/support/shared_examples/a_markdown_renderer.rb +++ /dev/null @@ -1,24 +0,0 @@ -RSpec.shared_examples 'a markdown renderer' do - it 'correctly renders simple tags' do - source = '**bold**' - expect(subject.render(source)).to eq "

bold

\n" - end - - it 'escapes HTML tags' do - source = '' - escaped_source = "

<img src=1 onerror=alert(&#34;pwned&#34;)>

\n" - expect(subject.render(source)).to eq escaped_source - end - - it 'supports @user links' do - source = 'Hey @hans, how are you?' - escaped_source = "

Hey @hans, how are you?

\n" - expect(subject.render(source)).to eq escaped_source - end - - it 'supports hw#slug links' do - source = 'Have you seen hw#super-cool? Its awesome' - escaped_source = "

Have you seen hw#super-cool? Its awesome

\n" - expect(subject.render(source)).to eq escaped_source - end -end