Commit ccd748d7 authored by Jesse Hall's avatar Jesse Hall

Feature for #16654, embedded audio elements in markdown.

parent e7b3aa9e
......@@ -21,6 +21,7 @@ import Reference from './nodes/reference';
import TableOfContents from './nodes/table_of_contents';
import Video from './nodes/video';
import Audio from './nodes/audio';
import BulletList from './nodes/bullet_list';
import OrderedList from './nodes/ordered_list';
......@@ -78,6 +79,7 @@ export default [
new TableOfContents(),
new Video(),
new Audio(),
new BulletList(),
new OrderedList(),
......
/* eslint-disable class-methods-use-this */
import { Node } from 'tiptap';
import { defaultMarkdownSerializer } from 'prosemirror-markdown';
// Transforms generated HTML back to GFM for Banzai::Filter::AudioLinkFilter
export default class Audio extends Node {
get name() {
return 'audio';
}
get schema() {
return {
attrs: {
src: {},
alt: {
default: null,
},
},
group: 'block',
draggable: true,
parseDOM: [
{
tag: '.audio-container',
skip: true,
},
{
tag: '.audio-container p',
priority: 51,
ignore: true,
},
{
tag: 'audio[src]',
getAttrs: el => ({ src: el.getAttribute('src'), alt: el.dataset.title }),
},
],
toDOM: node => [
'audio',
{
src: node.attrs.src,
controls: true,
'data-setup': '{}',
'data-title': node.attrs.alt,
},
],
};
}
toMarkdown(state, node) {
defaultMarkdownSerializer.nodes.image(state, node);
state.closeBlock(node);
}
}
......@@ -37,7 +37,7 @@ module UploadsActions
expires_in 0.seconds, must_revalidate: true, private: true
end
disposition = uploader.image_or_video? ? 'inline' : 'attachment'
disposition = uploader.embeddable? ? 'inline' : 'attachment'
uploaders = [uploader, *uploader.versions.values]
uploader = uploaders.find { |version| version.filename == params[:filename] }
......@@ -112,8 +112,8 @@ module UploadsActions
uploader
end
def image_or_video?
uploader && uploader.exists? && uploader.image_or_video?
def embeddable?
uploader && uploader.exists? && uploader.embeddable?
end
def find_model
......
......@@ -4,7 +4,7 @@ class Groups::UploadsController < Groups::ApplicationController
include UploadsActions
include WorkhorseRequest
skip_before_action :group, if: -> { action_name == 'show' && image_or_video? }
skip_before_action :group, if: -> { action_name == 'show' && embeddable? }
before_action :authorize_upload_file!, only: [:create, :authorize]
before_action :verify_workhorse_api!, only: [:authorize]
......
......@@ -40,8 +40,8 @@ class HelpController < ApplicationController
end
end
# Allow access to images in the doc folder
format.any(:png, :gif, :jpeg, :mp4) do
# Allow access to specific media files in the doc folder
format.any(:png, :gif, :jpeg, :mp4, :mp3) do
# Note: We are purposefully NOT using `Rails.root.join`
path = File.join(Rails.root, 'doc', "#{@path}.#{params[:format]}")
......
......@@ -6,7 +6,7 @@ class Projects::UploadsController < Projects::ApplicationController
# These will kick you out if you don't have access.
skip_before_action :project, :repository,
if: -> { action_name == 'show' && image_or_video? }
if: -> { action_name == 'show' && embeddable? }
before_action :authorize_upload_file!, only: [:create, :authorize]
before_action :verify_workhorse_api!, only: [:authorize]
......
......@@ -179,6 +179,10 @@ class Blob < SimpleDelegator
UploaderHelper::SAFE_VIDEO_EXT.include?(extension)
end
def audio?
UploaderHelper::SAFE_AUDIO_EXT.include?(extension)
end
def readable_text?
text_in_repo? && !stored_externally? && !truncated?
end
......
......@@ -415,7 +415,7 @@ class Commit
if entry[:type] == :blob
blob = ::Blob.decorate(Gitlab::Git::Blob.new(name: entry[:name]), @project)
blob.image? || blob.video? ? :raw : :blob
blob.image? || blob.video? || blob.audio? ? :raw : :blob
else
entry[:type]
end
......
---
title: Feature enabling embedded audio elements in markdown.
merge_request: 17860
author: Jesse Hall @jessehall3
type: added
......@@ -11,12 +11,12 @@ to log the IP address of the user.
One way to mitigate this is by proxying any external images to a server you
control.
GitLab can be configured to use an asset proxy server when requesting external images/videos in
GitLab can be configured to use an asset proxy server when requesting external images/videos/audio in
issues, comments, etc. This helps ensure that malicious images do not expose the user's IP address
when they are fetched.
We currently recommend using [cactus/go-camo](https://github.com/cactus/go-camo#how-it-works)
as it supports proxying video and is more configurable.
as it supports proxying video, audio, and is more configurable.
## Installing Camo server
......@@ -52,7 +52,7 @@ To install a Camo server as an asset proxy:
## Using the Camo server
Once the Camo server is running and you've enabled the GitLab settings, any image or video that
Once the Camo server is running and you've enabled the GitLab settings, any image, video, or audio that
references an external source will get proxied to the Camo server.
For example, the following is a link to an image in Markdown:
......
......@@ -108,7 +108,7 @@ changing how standard markdown is used:
| [code blocks](#code-spans-and-blocks) | [colored code and syntax highlighting](#colored-code-and-syntax-highlighting) |
| [emphasis](#emphasis) | [multiple underscores in words](#multiple-underscores-in-words-and-mid-word-emphasis)
| [headers](#headers) | [linkable Header IDs](#header-ids-and-links) |
| [images](#images) | [embedded videos](#videos) |
| [images](#images) | [embedded videos](#videos) and [audio](#audio) |
| [linebreaks](#line-breaks) | [more linebreak control](#newlines) |
| [links](#links) | [automatically linking URLs](#url-auto-linking) |
......@@ -899,6 +899,23 @@ Here's a sample video:
![Sample Video](img/markdown_video.mp4)
#### Audio
> If this is not rendered correctly, [view it in GitLab itself](https://gitlab.com/gitlab-org/gitlab/blob/master/doc/user/markdown.md#audio).
Similar to videos, link tags for files with an audio extension are automatically converted to
an audio player. The valid audio extensions are `.mp3`, `.ogg`, and `.wav`:
```md
Here's a sample audio clip:
![Sample Audio](img/markdown_audio.mp3)
```
Here's a sample audio clip:
![Sample Audio](img/markdown_audio.mp3)
### Inline HTML
> To see the markdown rendered within HTML in the second example, [view it in GitLab itself](https://gitlab.com/gitlab-org/gitlab/blob/master/doc/user/markdown.md#inline-html).
......
# frozen_string_literal: true
# Generated HTML is transformed back to GFM by app/assets/javascripts/behaviors/markdown/nodes/audio.js
module Banzai
module Filter
# Find every image that isn't already wrapped in an `a` tag, and that has
# a `src` attribute ending with an audio extension, add a new audio node and
# a "Download" link in the case the audio cannot be played.
class AudioLinkFilter < HTML::Pipeline::Filter
def call
doc.xpath('descendant-or-self::img[not(ancestor::a)]').each do |el|
el.replace(audio_node(doc, el)) if has_audio_extension?(el)
end
doc
end
private
def has_audio_extension?(element)
src = element.attr('data-canonical-src').presence || element.attr('src')
return unless src.present?
src_ext = File.extname(src).sub('.', '').downcase
Gitlab::FileTypeDetection::SAFE_AUDIO_EXT.include?(src_ext)
end
def audio_node(doc, element)
container = doc.document.create_element(
'div',
class: 'audio-container'
)
audio = doc.document.create_element(
'audio',
src: element['src'],
controls: true,
'data-setup' => '{}',
'data-title' => element['title'] || element['alt'])
link = doc.document.create_element(
'a',
element['title'] || element['alt'],
href: element['src'],
target: '_blank',
rel: 'noopener noreferrer',
title: "Download '#{element['title'] || element['alt']}'")
# make sure the original non-proxied src carries over
if element['data-canonical-src']
audio['data-canonical-src'] = element['data-canonical-src']
link['data-canonical-src'] = element['data-canonical-src']
end
download_paragraph = doc.document.create_element('p')
download_paragraph.children = link
container.add_child(audio)
container.add_child(download_paragraph)
container
end
end
end
end
......@@ -65,7 +65,7 @@ module Banzai
el.attribute('href')
end
attrs += doc.search('img, video').flat_map do |el|
attrs += doc.search('img, video, audio').flat_map do |el|
[el.attribute('src'), el.attribute('data-src')]
end
......@@ -83,7 +83,7 @@ module Banzai
get_blob_types(paths).each do |name, type|
if type == :blob
blob = ::Blob.decorate(Gitlab::Git::Blob.new(name: name), project)
uri_types[name] = blob.image? || blob.video? ? :raw : :blob
uri_types[name] = blob.image? || blob.video? || blob.audio? ? :raw : :blob
else
uri_types[name] = type
end
......
......@@ -15,7 +15,7 @@ module Banzai
doc.search('a:not(.gfm)').each { |el| process_link(el.attribute('href'), el) }
doc.search('video').each { |el| process_link(el.attribute('src'), el) }
doc.search('video, audio').each { |el| process_link(el.attribute('src'), el) }
doc.search('img').each do |el|
attr = el.attribute('data-src') || el.attribute('src')
......
......@@ -26,6 +26,7 @@ module Banzai
Filter::ColorFilter,
Filter::MermaidFilter,
Filter::VideoLinkFilter,
Filter::AudioLinkFilter,
Filter::ImageLazyLoadFilter,
Filter::ImageLinkFilter,
Filter::InlineMetricsFilter,
......
......@@ -10,14 +10,14 @@ module Gitlab
return unless name = markdown_name
markdown = "[#{name.gsub(']', '\\]')}](#{secure_url})"
markdown = "!#{markdown}" if image_or_video? || dangerous_image_or_video?
markdown = "!#{markdown}" if embeddable? || dangerous_embeddable?
markdown
end
def markdown_name
return unless filename.present?
image_or_video? ? File.basename(filename, File.extname(filename)) : filename
embeddable? ? File.basename(filename, File.extname(filename)) : filename
end
end
end
......@@ -26,11 +26,13 @@ module Gitlab
# on IE >= 9.
# http://archive.sublimevideo.info/20150912/docs.sublimevideo.net/troubleshooting.html
SAFE_VIDEO_EXT = %w[mp4 m4v mov webm ogv].freeze
SAFE_AUDIO_EXT = %w[mp3 oga ogg spx wav].freeze
# These extension types can contain dangerous code and should only be embedded inline with
# proper filtering. They should always be tagged as "Content-Disposition: attachment", not "inline".
DANGEROUS_IMAGE_EXT = %w[svg].freeze
DANGEROUS_VIDEO_EXT = [].freeze # None, yet
DANGEROUS_AUDIO_EXT = [].freeze # None, yet
def image?
extension_match?(SAFE_IMAGE_EXT)
......@@ -40,8 +42,12 @@ module Gitlab
extension_match?(SAFE_VIDEO_EXT)
end
def image_or_video?
image? || video?
def audio?
extension_match?(SAFE_AUDIO_EXT)
end
def embeddable?
image? || video? || audio?
end
def dangerous_image?
......@@ -52,8 +58,12 @@ module Gitlab
extension_match?(DANGEROUS_VIDEO_EXT)
end
def dangerous_image_or_video?
dangerous_image? || dangerous_video?
def dangerous_audio?
extension_match?(DANGEROUS_AUDIO_EXT)
end
def dangerous_embeddable?
dangerous_image? || dangerous_video? || dangerous_audio?
end
private
......
......@@ -178,6 +178,12 @@ describe 'Copy as GFM', :js do
'![Video](https://example.com/video.mp4)'
)
verify(
'AudioLinkFilter',
'![Audio](https://example.com/audio.wav)'
)
verify(
'MathFilter: math as converted from GFM to HTML',
......
......@@ -320,6 +320,10 @@ describe 'GitLab Markdown', :aggregate_failures do
expect(doc).to parse_video_links
end
aggregate_failures 'AudioLinkFilter' do
expect(doc).to parse_audio_links
end
aggregate_failures 'ColorFilter' do
expect(doc).to parse_colors
end
......
......@@ -101,7 +101,7 @@ describe 'Branches' do
visit project_branches_filtered_path(project, state: 'all')
expect(all('.all-branches').last).to have_selector('li', count: 20)
accept_confirm { find('.js-branch-add-pdf-text-binary .btn-remove').click }
accept_confirm { find('.js-branch-invalid-utf8-diff-paths .btn-remove').click }
expect(all('.all-branches').last).to have_selector('li', count: 19)
end
......
......@@ -286,6 +286,10 @@ However the wrapping tags cannot be mixed as such:
![My Video](/assets/videos/gitlab-demo.mp4)
### Audio
![My Audio Clip](/assets/audio/gitlab-demo.wav)
### Colors
`#F00`
......
# frozen_string_literal: true
require 'spec_helper'
describe Banzai::Filter::AudioLinkFilter do
def filter(doc, contexts = {})
contexts.reverse_merge!({
project: project
})
described_class.call(doc, contexts)
end
def link_to_image(path)
return '<img/>' if path.nil?
%(<img src="#{path}"/>)
end
let(:project) { create(:project, :repository) }
shared_examples 'an audio element' do
let(:image) { link_to_image(src) }
it 'replaces the image tag with an audio tag' do
container = filter(image).children.first
expect(container.name).to eq 'div'
expect(container['class']).to eq 'audio-container'
audio, paragraph = container.children
expect(audio.name).to eq 'audio'
expect(audio['src']).to eq src
expect(paragraph.name).to eq 'p'
link = paragraph.children.first
expect(link.name).to eq 'a'
expect(link['href']).to eq src
expect(link['target']).to eq '_blank'
end
end
shared_examples 'an unchanged element' do |ext|
it 'leaves the document unchanged' do
element = filter(link_to_image(src)).children.first
expect(element.name).to eq 'img'
expect(element['src']).to eq src
end
end
context 'when the element src has an audio extension' do
Gitlab::FileTypeDetection::SAFE_AUDIO_EXT.each do |ext|
it_behaves_like 'an audio element' do
let(:src) { "/path/audio.#{ext}" }
end
it_behaves_like 'an audio element' do
let(:src) { "/path/audio.#{ext.upcase}" }
end
end
end
context 'when the element has no src attribute' do
let(:src) { nil }
it_behaves_like 'an unchanged element'
end
context 'when the element src is an image' do
let(:src) { '/path/my_image.jpg' }
it_behaves_like 'an unchanged element'
end
context 'when the element src has an invalid file extension' do
let(:src) { '/path/my_audio.somewav' }
it_behaves_like 'an unchanged element'
end
context 'when data-canonical-src is empty' do
let(:image) { %(<img src="#{src}" data-canonical-src=""/>) }
context 'and src is audio' do
let(:src) { '/path/audio.wav' }
it_behaves_like 'an audio element'
end
context 'and src is an image' do
let(:src) { '/path/my_image.jpg' }
it_behaves_like 'an unchanged element'
end
end
context 'when data-canonical-src is set' do
it 'uses the correct src' do
proxy_src = 'https://assets.example.com/6d8b63'
canonical_src = 'http://example.com/test.wav'
image = %(<img src="#{proxy_src}" data-canonical-src="#{canonical_src}"/>)
container = filter(image).children.first
expect(container['class']).to eq 'audio-container'
audio, paragraph = container.children
expect(audio['src']).to eq proxy_src
expect(audio['data-canonical-src']).to eq canonical_src
link = paragraph.children.first
expect(link['href']).to eq proxy_src
end
end
end
......@@ -29,6 +29,10 @@ describe Banzai::Filter::RelativeLinkFilter do
%(<video src="#{path}"></video>)
end
def audio(path)
%(<audio src="#{path}"></audio>)
end
def link(path)
%(<a href="#{path}">#{path}</a>)
end
......@@ -82,6 +86,12 @@ describe Banzai::Filter::RelativeLinkFilter do
expect(doc.at_css('video')['src']).to eq 'files/videos/intro.mp4'
end
it 'does not modify any relative URL in audio' do
doc = filter(audio('files/audio/sample.wav'), commit: project.commit('audio'), ref: 'audio')
expect(doc.at_css('audio')['src']).to eq 'files/audio/sample.wav'
end
end
context 'with a project_wiki' do
......@@ -218,6 +228,13 @@ describe Banzai::Filter::RelativeLinkFilter do
.to eq "/#{project_path}/raw/video/files/videos/intro.mp4"
end
it 'rebuilds relative URL for audio in the repo' do
doc = filter(audio('files/audio/sample.wav'), commit: project.commit('audio'), ref: 'audio')
expect(doc.at_css('audio')['src'])
.to eq "/#{project_path}/raw/audio/files/audio/sample.wav"
end
it 'does not modify relative URL with an anchor only' do
doc = filter(link('#section-1'))
expect(doc.at_css('a')['href']).to eq '#section-1'
......
......@@ -60,6 +60,14 @@ describe Banzai::Filter::WikiLinkFilter do
expect(filtered_link.attribute('src').value).to eq("#{wiki.wiki_base_path}/#{repository_upload_folder}/a.mp4")
end
end
context 'with "audio" html tag' do
it 'rewrites links' do
filtered_link = filter("<audio src='#{repository_upload_folder}/a.wav'></audio>", project_wiki: wiki).children[0]
expect(filtered_link.attribute('src').value).to eq("#{wiki.wiki_base_path}/#{repository_upload_folder}/a.wav")
end
end
end
describe "invalid links" do
......
......@@ -260,11 +260,11 @@ describe Banzai::Pipeline::WikiPipeline do
end
end
describe 'videos' do
let(:namespace) { create(:namespace, name: "wiki_link_ns") }
let(:project) { create(:project, :public, name: "wiki_link_project", namespace: namespace) }
let(:project_wiki) { ProjectWiki.new(project, double(:user)) }
let(:page) { build(:wiki_page, wiki: project_wiki, page: OpenStruct.new(url_path: 'nested/twice/start-page')) }
describe 'videos and audio' do
let_it_be(:namespace) { create(:namespace, name: "wiki_link_ns") }
let_it_be(:project) { create(:project, :public, name: "wiki_link_project", namespace: namespace) }
let_it_be(:project_wiki) { ProjectWiki.new(project, double(:user)) }
let_it_be(:page) { build(:wiki_page, wiki: project_wiki, page: OpenStruct.new(url_path: 'nested/twice/start-page')) }
it 'generates video html structure' do
markdown = "![video_file](video_file_name.mp4)"
......@@ -279,5 +279,19 @@ describe Banzai::Pipeline::WikiPipeline do
expect(output).to include('<video src="/wiki_link_ns/wiki_link_project/wikis/nested/twice/video%20file%20name.mp4"')
end
it 'generates audio html structure' do
markdown = "![audio_file](audio_file_name.wav)"
output = described_class.to_html(markdown, project: project, project_wiki: project_wiki, page_slug: page.slug)
expect(output).to include('<audio src="/wiki_link_ns/wiki_link_project/wikis/nested/twice/audio_file_name.wav"')
end
it 'rewrites and replaces audio links names with white spaces to %20' do
markdown = "![audio file](audio file name.wav)"
output = described_class.to_html(markdown, project: project, project_wiki: project_wiki, page_slug: page.slug)
expect(output).to include('<audio src="/wiki_link_ns/wiki_link_project/wikis/nested/twice/audio%20file%20name.wav"')
end
end
end
......@@ -27,19 +27,35 @@ describe Gitlab::FileMarkdownLinkBuilder do
end
end
context 'when file is an image or video' do
let(:filename) { 'dk.png' }
context 'when file is an image' do
let(:filename) { 'my_image.png' }
it 'returns preview markdown link' do
expect(custom_class.markdown_link).to eq '![dk](/uploads/dk.png)'
expect(custom_class.markdown_link).to eq '![my_image](/uploads/my_image.png)'
end
end
context 'when file is not an image or video' do
let(:filename) { 'dk.zip' }
context 'when file is video' do
let(:filename) { 'my_video.mp4' }
it 'returns preview markdown link' do
expect(custom_class.markdown_link).to eq '![my_video](/uploads/my_video.mp4)'
end
end
context 'when file is audio' do
let(:filename) { 'my_audio.wav' }
it 'returns preview markdown link' do
expect(custom_class.markdown_link).to eq '![my_audio](/uploads/my_audio.wav)'
end
end
context 'when file is not embeddable' do
let(:filename) { 'my_zip.zip' }
it 'returns markdown link' do
expect(custom_class.markdown_link).to eq '[dk.zip](/uploads/dk.zip)'
expect(custom_class.markdown_link).to eq '[my_zip.zip](/uploads/my_zip.zip)'
end
end
......@@ -53,19 +69,35 @@ describe Gitlab::FileMarkdownLinkBuilder do
end
describe 'mardown_name' do
context 'when file is an image or video' do
let(:filename) { 'dk.png' }
context 'when file is an image' do
let(:filename) { 'my_image.png' }
it 'retrieves the name without the extension' do
expect(custom_class.markdown_name).to eq 'my_image'
end
end
context 'when file is video' do
let(:filename) { 'my_video.mp4' }
it 'retrieves the name without the extension' do
expect(custom_class.markdown_name).to eq 'my_video'
end
end
context 'when file is audio' do
let(:filename) { 'my_audio.wav' }
it 'retrieves the name without the extension' do
expect(custom_class.markdown_name).to eq 'dk'
expect(custom_class.markdown_name).to eq 'my_audio'
end
end
context 'when file is not an image or video' do
let(:filename) { 'dk.zip' }
context 'when file is not embeddable' do
let(:filename) { 'my_zip.zip' }
it 'retrieves the name with the extesion' do
expect(custom_class.markdown_name).to eq 'dk.zip'
expect(custom_class.markdown_name).to eq 'my_zip.zip'
end
end
......
This diff is collapsed.
......@@ -43,6 +43,11 @@ describe Gitlab::Utils::SanitizeNodeLink do
doc: HTML::Pipeline.parse("<video><source src='#{scheme}alert(1);'></video>"),
attr: "src",
node_to_check: -> (doc) { doc.children.first.children.filter("source").first }
},
audio: {
doc: HTML::Pipeline.parse("<audio><source src='#{scheme}alert(1);'></audio>"),
attr: "src",
node_to_check: -> (doc) { doc.children.first.children.filter("source").first }
}
}
......
......@@ -503,6 +503,8 @@ eos
expect(commit.uri_type('files/html')).to be(:tree)
expect(commit.uri_type('files/images/logo-black.png')).to be(:raw)
expect(commit.uri_type('files/images/wm.svg')).to be(:raw)
expect(project.commit('audio').uri_type('files/audio/clip.mp3')).to be(:raw)
expect(project.commit('audio').uri_type('files/audio/sample.wav')).to be(:raw)
expect(project.commit('video').uri_type('files/videos/intro.mp4')).to be(:raw)
expect(commit.uri_type('files/js/application.js')).to be(:blob)
end
......
......@@ -36,6 +36,7 @@ module TestEnv
'expand-collapse-lines' => '238e82d',
'pages-deploy' => '7897d5b',
'pages-deploy-target' => '7975be0',
'audio' => 'c3c21fd',
'video' => '8879059',
'add-balsamiq-file' => 'b89b56d',
'crlf-diff' => '5938907',
......
......@@ -193,6 +193,17 @@ module MarkdownMatchers
end
end
# AudioLinkFilter
matcher :parse_audio_links do
set_default_markdown_messages
match do |actual|
audio = actual.at_css('audio')
expect(audio['src']).to end_with('/assets/audio/gitlab-demo.wav')
end
end
# ColorFilter
matcher :parse_colors do
set_default_markdown_messages
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment