ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

correctly scope rce urls with host to context 

the logic that sanitizes urls in user content only converts verified
file urls to file urls in the correct scope when the uri does not
contain the host. this is breaking because the rcs is using full
absolute urls for links to files, and we probably don't want to stop
in case content is rendered on a different domain in the future (lti
app in an iframe?).

fixes CNVS-32091

test plan:
- insert a file link from the rcs sidebar
- inspect the html before saveing
  - should include the host and be a verified url without the context
- save the content
- inspect the html
  - link should include the host, context and context id
    (i.e. https://account.instructure.com/course/1/files...)

Change-Id: I22e872370a120aebc2c60b3a11d2b98be14c7771
Reviewed-on: https://gerrit.instructure.com/91099
Reviewed-by: John Corrigan <jcorrigan@instructure.com>
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Tested-by: Jenkins
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Brent Burgoyne <bburgoyne@instructure.com>
This commit is contained in:
Brent Burgoyne 2016-09-22 14:30:21 -06:00
parent d9751edf53
commit 0913a186e9
4 changed files with 24 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/api/html/content.rb
View File

@ -42,7 +42,7 @@ module Api
end
def might_need_modification?
!!(html =~ %r{verifier=|['"]/files|instructure_inline_media_comment})
!!(html =~ %r{verifier=|\D/files|instructure_inline_media_comment})
end
# Take incoming html from a user or similar and modify it for safe storage and display

8
lib/api/html/link.rb
View File

@ -16,6 +16,8 @@
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
require 'uri'
module Api
module Html
class Link
@ -45,9 +47,11 @@ module Api
end
def scope_link_to_context(local_link)
if local_link.start_with?('/files')
uri = URI.parse(local_link)
if uri.path.start_with?('/files')
if attachment && APPLICABLE_CONTEXT_TYPES.include?(attachment.context_type)
return "/#{attachment.context_type.underscore.pluralize}/#{attachment.context_id}" + local_link
uri.path = "/#{attachment.context_type.underscore.pluralize}/#{attachment.context_id}#{uri.path}"
return uri.to_s
end
end

10
spec/lib/api/html/content_spec.rb
View File

@ -37,6 +37,16 @@ module Api
expect(Content.new(string).might_need_modification?).to be(true)
end
it 'is true for a link to files that include the host' do
string = "<body><a href='https://account.instructure.com/files'>link</a></body>"
expect(Content.new(string).might_need_modification?).to be(true)
end
it 'is false for a link to files in a context' do
string = "<body><a href='/courses/1/files'>link</a></body>"
expect(Content.new(string).might_need_modification?).to be(false)
end
it 'is false for garden-variety content' do
string = "<body><a href='http://example.com/123'>link</a></body>"
expect(Content.new(string).might_need_modification?).to be(false)

7
spec/lib/api/html/link_spec.rb
View File

@ -48,6 +48,13 @@ module Api
raw_link = "/files/1/download?verifier=123"
expect(Link.new(raw_link).to_corrected_s).to eq "/courses/1/files/1/download?"
end
it 'scopes to the context even if url includes the host' do
course_attachment = stub(context_type: "Course", context_id: 1)
Attachment.stubs(:where).with(id: "1").returns(stub(first: course_attachment))
raw_link = "https://account.instructure.com/files/1/download?verifier=123"
expect(Link.new(raw_link).to_corrected_s).to eq "https://account.instructure.com/courses/1/files/1/download?"
end
end
end
end