don't flag urls starting with '/' as not relative
When importing, urls starting with '/' were flagged as not relative. This commit changes the relative url detection to use the URI library. There are also a lot of unit tests added for the ImportedHtmlConverter. Test Plan: * Import a package that has a relative mage url starting with '/' on a wiki page * The image should render when the import is complete closes #6813 Change-Id: I008bb603b1abb53ae8753b44413cac8b9b1f4245 Reviewed-on: https://gerrit.instructure.com/8022 Reviewed-by: Cody Cutrer <cody@instructure.com> Tested-by: Hudson <hudson@instructure.com>
This commit is contained in:
Bracken Mosbacker
parent
7ccca7a827
commit
628e32b18a
|
|
@ -64,8 +64,12 @@ module CC::Importer::Standard
|
|||
attrs.each do |attr|
|
||||
if node[attr]
|
||||
val = URI.unescape(node[attr])
|
||||
if ImportedHtmlConverter.relative_url?(val)
|
||||
node[attr] = URI::escape(get_canvas_att_replacement_url(val))
|
||||
begin
|
||||
if ImportedHtmlConverter.relative_url?(val)
|
||||
node[attr] = URI::escape(get_canvas_att_replacement_url(val))
|
||||
end
|
||||
rescue URI::InvalidURIError
|
||||
Rails.logger.warn "attempting to translate invalid url: #{val}"
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -69,8 +69,15 @@ class ImportedHtmlConverter
|
|||
else
|
||||
node[attr] = replace_relative_file_url(rel_path, context, course_path)
|
||||
end
|
||||
elsif relative_url?(node[attr])
|
||||
node[attr] = replace_relative_file_url(node[attr], context, course_path)
|
||||
else
|
||||
begin
|
||||
if relative_url?(node[attr])
|
||||
node[attr] = replace_relative_file_url(node[attr], context, course_path)
|
||||
end
|
||||
rescue URI::InvalidURIError
|
||||
Rails.logger.warn "attempting to translate invalid url: #{node[attr]}"
|
||||
# leave the url as it was
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
@ -122,7 +129,7 @@ class ImportedHtmlConverter
|
|||
end
|
||||
unless new_url
|
||||
# the rel_path should already be escaped
|
||||
new_url = URI::escape("#{course_path}/file_contents/#{Folder.root_folders(context).first.name}/") + rel_path
|
||||
new_url = File.join(URI::escape("#{course_path}/file_contents/#{Folder.root_folders(context).first.name}"), rel_path)
|
||||
end
|
||||
new_url
|
||||
end
|
||||
|
|
@ -146,7 +153,7 @@ class ImportedHtmlConverter
|
|||
end
|
||||
|
||||
def self.relative_url?(url)
|
||||
(url.match(/[\/#\?]/) || (url.match(/\./) && !url.match(/@/))) && !url.match(/\A\w+:/) && !url.match(/\A\//)
|
||||
URI.parse(url).relative?
|
||||
end
|
||||
|
||||
def self.convert_text(text, context, import_source=:webct)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,121 @@
|
|||
#
|
||||
# Copyright (C) 2012 Instructure, Inc.
|
||||
#
|
||||
# This file is part of Canvas.
|
||||
#
|
||||
# Canvas is free software: you can redistribute it and/or modify it under
|
||||
# the terms of the GNU Affero General Public License as published by the Free
|
||||
# Software Foundation, version 3 of the License.
|
||||
#
|
||||
# Canvas 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 Affero General Public License for more
|
||||
# details.
|
||||
#
|
||||
# You should have received a copy of the GNU Affero General Public License along
|
||||
# with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
#
|
||||
|
||||
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
|
||||
|
||||
describe ImportedHtmlConverter do
|
||||
|
||||
context ".convert" do
|
||||
before(:each) do
|
||||
course
|
||||
@path = "/courses/#{@course.id}/"
|
||||
end
|
||||
|
||||
it "should convert a wiki reference" do
|
||||
test_string = %{<a href="%24WIKI_REFERENCE%24/wiki/test-wiki-page">Test Wiki Page</a>}
|
||||
@course.wiki.wiki_pages.create!(:title => "Test Wiki Page", :body => "stuff")
|
||||
|
||||
ImportedHtmlConverter.convert(test_string, @course).should == %{<a href="#{@path}wiki/test-wiki-page">Test Wiki Page</a>}
|
||||
end
|
||||
|
||||
it "should convert a wiki reference without $ escaped" do
|
||||
test_string = %{<a href="$WIKI_REFERENCE$/wiki/test-wiki-page">Test Wiki Page</a>}
|
||||
@course.wiki.wiki_pages.create!(:title => "Test Wiki Page", :body => "stuff")
|
||||
|
||||
ImportedHtmlConverter.convert(test_string, @course).should == %{<a href="#{@path}wiki/test-wiki-page">Test Wiki Page</a>}
|
||||
end
|
||||
|
||||
it "should convert a wiki reference by migration id" do
|
||||
test_string = %{<a href="wiki_page_migration_id=123456677788">Test Wiki Page</a>}
|
||||
wiki = @course.wiki.wiki_pages.create(:title => "Test Wiki Page", :body => "stuff")
|
||||
wiki.migration_id = "123456677788"
|
||||
wiki.save!
|
||||
|
||||
ImportedHtmlConverter.convert(test_string, @course).should == %{<a href="#{@path}wiki/test-wiki-page">Test Wiki Page</a>}
|
||||
end
|
||||
|
||||
it "should convert a discussion reference by migration id" do
|
||||
test_string = %{<a href="discussion_topic_migration_id=123456677788">Test topic</a>}
|
||||
topic = @course.discussion_topics.create(:title => "Test discussion")
|
||||
topic.migration_id = "123456677788"
|
||||
topic.save!
|
||||
|
||||
ImportedHtmlConverter.convert(test_string, @course).should == %{<a href="#{@path}discussion_topics/#{topic.id}">Test topic</a>}
|
||||
end
|
||||
|
||||
it "should find an attachment by migration id" do
|
||||
att = Attachment.create(:filename => 'test.png', :display_name => "test.png", :uploaded_data => StringIO.new('psych!'), :folder => Folder.unfiled_folder(@course), :context => @course)
|
||||
att.migration_id = "1768525836051"
|
||||
att.save!
|
||||
|
||||
test_string = %{<p>This is an image: <br /><img src="%24CANVAS_OBJECT_REFERENCE%24/attachments/1768525836051" alt=":(" /></p>}
|
||||
ImportedHtmlConverter.convert(test_string, @course).should == %{<p>This is an image: <br><img src="#{@path}files/#{att.id}/preview" alt=":("></p>}
|
||||
end
|
||||
|
||||
it "should find an attachment by path" do
|
||||
att = Attachment.create(:filename => 'test.png', :display_name => "test.png", :uploaded_data => StringIO.new('psych!'), :folder => Folder.unfiled_folder(@course), :context => @course)
|
||||
att.migration_id = "1768525836051"
|
||||
att.save!
|
||||
|
||||
test_string = %{<p>This is an image: <br /><img src="%24IMS_CC_FILEBASE%24/test.png" alt=":(" /></p>}
|
||||
|
||||
# if there isn't a path->migration id map it'll be a relative course file path
|
||||
ImportedHtmlConverter.convert(test_string, @course).should == %{<p>This is an image: <br><img src="#{@path}file_contents/course%20files/test.png" alt=":("></p>}
|
||||
|
||||
@course.attachment_path_id_lookup = {"test.png" => att.migration_id}
|
||||
ImportedHtmlConverter.convert(test_string, @course).should == %{<p>This is an image: <br><img src="#{@path}files/#{att.id}/preview" alt=":("></p>}
|
||||
end
|
||||
|
||||
it "should convert course section urls" do
|
||||
test_string = %{<a href="%24CANVAS_COURSE_REFERENCE%24/discussion_topics">discussions</a>}
|
||||
ImportedHtmlConverter.convert(test_string, @course).should == %{<a href="#{@path}discussion_topics">discussions</a>}
|
||||
end
|
||||
|
||||
it "should leave invalid and absolute urls alone" do
|
||||
test_string = %{<a href="stupid &^%$ url">Linkage</a><br><a href="http://www.example.com/poop">Linkage</a>}
|
||||
ImportedHtmlConverter.convert(test_string, @course).should == %{<a href="stupid%20&%5E%%24%20url">Linkage</a><br><a href="http://www.example.com/poop">Linkage</a>}
|
||||
end
|
||||
|
||||
it "should prepend course files for unrecognized relative urls" do
|
||||
test_string = %{<a href="/relative/path/to/file">Linkage</a>}
|
||||
ImportedHtmlConverter.convert(test_string, @course).should == %{<a href="#{@path}file_contents/course%20files/relative/path/to/file">Linkage</a>}
|
||||
test_string = %{<a href="relative/path/to/file">Linkage</a>}
|
||||
ImportedHtmlConverter.convert(test_string, @course).should == %{<a href="#{@path}file_contents/course%20files/relative/path/to/file">Linkage</a>}
|
||||
test_string = %{<a href="relative/path/to/file%20with%20space.html">Linkage</a>}
|
||||
ImportedHtmlConverter.convert(test_string, @course).should == %{<a href="#{@path}file_contents/course%20files/relative/path/to/file%20with%20space.html">Linkage</a>}
|
||||
end
|
||||
end
|
||||
|
||||
context ".relative_url?" do
|
||||
it "should recognize an absolute url" do
|
||||
ImportedHtmlConverter.relative_url?("http://example.com").should == false
|
||||
end
|
||||
|
||||
it "should recognize relative urls" do
|
||||
ImportedHtmlConverter.relative_url?("/relative/eh").should == true
|
||||
ImportedHtmlConverter.relative_url?("also/relative").should == true
|
||||
ImportedHtmlConverter.relative_url?("watup/nothing.html#anchoritbaby").should == true
|
||||
ImportedHtmlConverter.relative_url?("watup/nothing?absolutely=1").should == true
|
||||
end
|
||||
|
||||
it "should error on invalid urls" do
|
||||
expect { ImportedHtmlConverter.relative_url?("stupid &^%$ url") }.to raise_error(URI::InvalidURIError)
|
||||
end
|
||||
end
|
||||
|
||||
end
|
||||
Loading…
Reference in New Issue