Commit 71490b74 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Fix VideoLinkFilter for uppercase file extensions

Also fixed to handle images with blank src
attributes gracefully
parent 9af1b6d2
---
title: Fix inline rendering of videos for uploads with uppercase file extensions
merge_request: 17924
author:
type: fixed
......@@ -8,8 +8,8 @@ module Banzai
# a "Download" link in the case the video cannot be played.
class VideoLinkFilter < HTML::Pipeline::Filter
def call
doc.xpath(query).each do |el|
el.replace(video_node(doc, el))
doc.xpath('descendant-or-self::img[not(ancestor::a)]').each do |el|
el.replace(video_node(doc, el)) if has_video_extension?(el)
end
doc
......@@ -17,22 +17,13 @@ module Banzai
private
def query
@query ||= begin
src_query = UploaderHelper::SAFE_VIDEO_EXT.map do |ext|
"'.#{ext}' = substring(@src, string-length(@src) - #{ext.size})"
end
def has_video_extension?(element)
src = element.attr('data-canonical-src').presence || element.attr('src')
if context[:asset_proxy_enabled].present?
src_query.concat(
UploaderHelper::SAFE_VIDEO_EXT.map do |ext|
"'.#{ext}' = substring(@data-canonical-src, string-length(@data-canonical-src) - #{ext.size})"
end
)
end
return unless src.present?
"descendant-or-self::img[not(ancestor::a) and (#{src_query.join(' or ')})]"
end
src_ext = File.extname(src).sub('.', '').downcase
Gitlab::FileTypeDetection::SAFE_VIDEO_EXT.include?(src_ext)
end
def video_node(doc, element)
......
......@@ -12,52 +12,98 @@ describe Banzai::Filter::VideoLinkFilter do
end
def link_to_image(path)
%(<img src="#{path}" />)
return '<img/>' if path.nil?
%(<img src="#{path}"/>)
end
let(:project) { create(:project, :repository) }
context 'when the element src has a video extension' do
UploaderHelper::SAFE_VIDEO_EXT.each do |ext|
it "replaces the image tag 'path/video.#{ext}' with a video tag" do
container = filter(link_to_image("/path/video.#{ext}")).children.first
shared_examples 'a video element' do
let(:image) { link_to_image(src) }
expect(container.name).to eq 'div'
expect(container['class']).to eq 'video-container'
it 'replaces the image tag with a video tag' do
container = filter(image).children.first
video, paragraph = container.children
expect(container.name).to eq 'div'
expect(container['class']).to eq 'video-container'
expect(video.name).to eq 'video'
expect(video['src']).to eq "/path/video.#{ext}"
video, paragraph = container.children
expect(paragraph.name).to eq 'p'
expect(video.name).to eq 'video'
expect(video['src']).to eq src
link = paragraph.children.first
expect(paragraph.name).to eq 'p'
expect(link.name).to eq 'a'
expect(link['href']).to eq "/path/video.#{ext}"
expect(link['target']).to eq '_blank'
end
link = paragraph.children.first
expect(link.name).to eq 'a'
expect(link['href']).to eq src
expect(link['target']).to eq '_blank'
end
end
context 'when the element src is an image' do
shared_examples 'an unchanged element' do |ext|
it 'leaves the document unchanged' do
element = filter(link_to_image('/path/my_image.jpg')).children.first
element = filter(link_to_image(src)).children.first
expect(element.name).to eq 'img'
expect(element['src']).to eq '/path/my_image.jpg'
expect(element['src']).to eq src
end
end
context 'when asset proxy is enabled' do
it 'uses the correct src' do
stub_asset_proxy_setting(enabled: true)
context 'when the element src has a video extension' do
Gitlab::FileTypeDetection::SAFE_VIDEO_EXT.each do |ext|
it_behaves_like 'a video element' do
let(:src) { "/path/video.#{ext}" }
end
it_behaves_like 'a video element' do
let(:src) { "/path/video.#{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_video.somemp4' }
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 a video' do
let(:src) { '/path/video.mp4' }
it_behaves_like 'a video 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.mp4'
image = %(<img src="#{proxy_src}" data-canonical-src="#{canonical_src}" />)
container = filter(image, asset_proxy_enabled: true).children.first
image = %(<img src="#{proxy_src}" data-canonical-src="#{canonical_src}"/>)
container = filter(image).children.first
expect(container['class']).to eq 'video-container'
......
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