transpose cross-shard links in api_user_content
fixes CORE-763 test plan: * set up two accounts, on different shards, and a user associated with both * create a course on shard 2 and create a wiki page for the front page. * create a module in that course * as the content of the front page, embed an image from the course, and link to the module * using the domain of shard 1, go to /api/v1/courses/<id of shard 2>~<id the course>/front_page * inspect the links in the returned body element. they should be using the first account's domain, but should contain (short) global ids. there should 4 - the 'regular' URL for the image and the link, and a data-api-endpoint for each. * exercise all 4 URLs. the HTML ones should redirect to account 2's domain, and the API URLs should return a result directly Change-Id: I10aa0fc1dc003a781d04ec5b230ede6aeba64fb9 Reviewed-on: https://gerrit.instructure.com/141664 Reviewed-by: Rob Orton <rob@instructure.com> Tested-by: Jenkins QA-Review: Jeremy Putnam <jeremyp@instructure.com> Product-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer
parent
2093398f70
commit
8b47dfcd8f
|
|
@ -484,6 +484,7 @@ module Api
|
|||
# otherwise let HostUrl figure out what host is appropriate
|
||||
if self.respond_to?(:request)
|
||||
host, protocol = get_host_and_protocol_from_request
|
||||
target_shard = Shard.current
|
||||
elsif self.respond_to?(:use_placeholder_host?) && use_placeholder_host?
|
||||
host = PLACEHOLDER_HOST
|
||||
protocol = PLACEHOLDER_PROTOCOL
|
||||
|
|
@ -505,7 +506,11 @@ module Api
|
|||
end
|
||||
html = rewriter.translate_content(html)
|
||||
|
||||
url_helper = Html::UrlProxy.new(self, context, host, protocol)
|
||||
url_helper = Html::UrlProxy.new(self,
|
||||
context,
|
||||
host,
|
||||
protocol,
|
||||
target_shard: target_shard)
|
||||
account = Context.get_account(context) || @domain_root_account
|
||||
include_mobile = !(respond_to?(:in_app?, true) && in_app?)
|
||||
Html::Content.rewrite_outgoing(html, account, url_helper, include_mobile: include_mobile)
|
||||
|
|
|
|||
|
|
@ -75,12 +75,13 @@ module Api
|
|||
}.freeze
|
||||
|
||||
class UrlProxy
|
||||
attr_reader :proxy, :context, :host, :protocol
|
||||
def initialize(helper, context, host, protocol)
|
||||
attr_reader :proxy, :context, :host, :protocol, :target_shard
|
||||
def initialize(helper, context, host, protocol, target_shard: nil)
|
||||
@proxy = helper
|
||||
@context = context
|
||||
@host = host
|
||||
@protocol = protocol
|
||||
@target_shard = target_shard || context.shard
|
||||
end
|
||||
|
||||
def media_object_thumbnail_url(media_id)
|
||||
|
|
@ -119,7 +120,15 @@ module Api
|
|||
# otherwise if it starts with a slash, it's a path that needs to be
|
||||
# made absolute with the canvas hostname prepended
|
||||
if !url.host && url_str[0] == '/'[0]
|
||||
element[attribute] = "#{protocol}://#{host}#{url_str}"
|
||||
# transpose IDs in the URL
|
||||
if context.shard != target_shard && (args = recognize_path(url_str))
|
||||
transpose_ids(args)
|
||||
args[:host] = host
|
||||
args[:protocol] = protocol
|
||||
element[attribute] = proxy.url_for(args)
|
||||
else
|
||||
element[attribute] = "#{protocol}://#{host}#{url_str}"
|
||||
end
|
||||
api_endpoint_info(url_str).each do |att, val|
|
||||
element[att] = val
|
||||
end
|
||||
|
|
@ -138,6 +147,8 @@ module Api
|
|||
helper = api_route[1]
|
||||
args = { :protocol => protocol, :host => host }
|
||||
args.merge! Hash[api_route.slice(2, match.captures.size).zip match.captures]
|
||||
# transpose IDs in the URL
|
||||
transpose_ids(args) if context.shard != target_shard
|
||||
if args[:url] && (return_type == 'SessionlessLaunchUrl' || (return_type == "Page" && url.include?("titleize=0")))
|
||||
args[:url] = URI.unescape(args[:url])
|
||||
end
|
||||
|
|
@ -146,6 +157,44 @@ module Api
|
|||
end
|
||||
{}
|
||||
end
|
||||
|
||||
# based on ActionDispatch::Routing::RouteSet#recognize_path, but returning all params,
|
||||
# not just path_params. and failures return nil, not raise an exception
|
||||
def recognize_path(path)
|
||||
path = ActionDispatch::Journey::Router::Utils.normalize_path(path)
|
||||
|
||||
begin
|
||||
env = Rack::MockRequest.env_for(path, method: "GET")
|
||||
rescue URI::InvalidURIError
|
||||
return nil
|
||||
end
|
||||
|
||||
req = Rails.application.routes.send(:make_request, env)
|
||||
Rails.application.routes.router.recognize(req) do |route, params|
|
||||
params.each do |key, value|
|
||||
if value.is_a?(String)
|
||||
value = value.dup.force_encoding(Encoding::BINARY)
|
||||
params[key] = URI.parser.unescape(value)
|
||||
end
|
||||
end
|
||||
req.path_parameters = params
|
||||
app = route.app
|
||||
if app.matches?(req) && app.dispatcher?
|
||||
return req.params
|
||||
end
|
||||
end
|
||||
|
||||
nil
|
||||
end
|
||||
|
||||
def transpose_ids(args)
|
||||
args.each_key do |key|
|
||||
if (key.to_s == 'id' || key.to_s.end_with?('_id')) &&
|
||||
(new_id = Switchman::Shard.relative_id_for(args[key], context.shard, target_shard))
|
||||
args[key] = Switchman::Shard.short_id_for(new_id)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -97,7 +97,7 @@ module Api
|
|||
|
||||
it "re-writes root-relative urls to be absolute" do
|
||||
string = "<p><a href=\"/blah\"></a></p>"
|
||||
url_helper = UrlProxy.new(double, double, "example.com", "https")
|
||||
url_helper = UrlProxy.new(double, double(shard: nil), "example.com", "https")
|
||||
html = Content.new(string).rewritten_html(url_helper)
|
||||
expect(html).to eq("<p><a href=\"https://example.com/blah\"></a></p>")
|
||||
end
|
||||
|
|
|
|||
|
|
@ -743,6 +743,35 @@ describe Api do
|
|||
expect(res).to eq "<p>a</p><p>b</p>"
|
||||
end
|
||||
end
|
||||
|
||||
context "sharding" do
|
||||
specs_require_sharding
|
||||
|
||||
it 'transposes ids in urls' do
|
||||
@shard1.activate do
|
||||
a = Account.create!
|
||||
student_in_course(account: a)
|
||||
end
|
||||
html = <<-HTML
|
||||
<img src="/courses/34/files/2082/download?wrap=1" data-api-returntype="File" data-api-endpoint="https://canvas.vanity.edu/api/v1/courses/34/files/2082">
|
||||
<a href="/courses/34/pages/module-1" data-api-returntype="Page" data-api-endpoint="https://canvas.vanity.edu/api/v1/courses/34/pages/module-1">link</a>
|
||||
HTML
|
||||
|
||||
@k = klass.new
|
||||
@k.instance_variable_set(:@domain_root_account, Account.default)
|
||||
@k.extend Rails.application.routes.url_helpers
|
||||
@k.extend ActionDispatch::Routing::UrlFor
|
||||
allow(@k).to receive(:request).and_return(nil)
|
||||
allow(@k).to receive(:get_host_and_protocol_from_request).and_return(['school.instructure.com', 'https'])
|
||||
allow(@k).to receive(:url_options).and_return({})
|
||||
|
||||
res = @k.api_user_content(html, @course, @student)
|
||||
expect(res).to eq <<-HTML
|
||||
<img src="https://school.instructure.com/courses/#{@shard1.id}~34/files/#{@shard1.id}~2082/download?wrap=1" data-api-returntype="File" data-api-endpoint="https://school.instructure.com/api/v1/courses/#{@shard1.id}~34/files/#{@shard1.id}~2082">
|
||||
<a href="https://school.instructure.com/courses/#{@shard1.id}~34/pages/module-1" data-api-returntype="Page" data-api-endpoint="https://school.instructure.com/api/v1/courses/#{@shard1.id}~34/pages/module-1">link</a>
|
||||
HTML
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context ".process_incoming_html_content" do
|
||||
|
|
|
|||
Loading…
Reference in New Issue