fail gracefully on a bad tool launch url

closes FOO-1464
flag=none

TEST PLAN:
  1) scrub a launch by forcing a silly url
  2) you get a flash error rather than an app-level
     500

Change-Id: I38ac55d85f505e62fc7368da6026e19cf4573a06
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/256673
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Ethan Vizitei <evizitei@instructure.com>
Product-Review: Ethan Vizitei <evizitei@instructure.com>

app/controllers/external_tools_controller.rb

@@ -490,6 +490,7 @@ class ExternalToolsController < ApplicationController
     render_unauthorized_action
     nil
   rescue Lti::Errors::UnsupportedExportTypeError,
+         Lti::Errors::InvalidLaunchUrlError,
          Lti::Errors::InvalidMediaTypeError,
          Lti::Errors::UnsupportedPlacement => e
     Canvas::Errors.capture_exception(:lti_launch, e, :info)

app/models/lti/lti_outbound_adapter.rb

@@ -18,6 +18,7 @@
 # with this program. If not, see <http://www.gnu.org/licenses/>.
 
 module Lti
+
   class LtiOutboundAdapter
     cattr_writer :consumer_instance_class
 
@@ -82,13 +83,17 @@ module Lti
       hash = @tool_launch.generate(@overrides)
       hash[:ext_lti_assignment_id] = assignment&.lti_context_id if assignment&.lti_context_id.present?
       hash[:ext_lti_student_id] = student_id if student_id
-      Lti::Security.signed_post_params(
+      begin
+        return Lti::Security.signed_post_params(
           hash,
           @tool_launch.url,
           @tool.consumer_key,
           @tool.shared_secret,
           disable_post_only?
         )
+      rescue URI::InvalidURIError
+        raise ::Lti::Errors::InvalidLaunchUrlError, "Invalid launch url: #{@tool_launch.url}"
+      end
     end
 
     def generate_post_payload_for_assignment(assignment, outcome_service_url, legacy_outcome_service_url, lti_turnitin_outcomes_placement_url)

lib/lti/errors.rb

@@ -25,5 +25,6 @@ module Lti
     class UnsupportedPlacement < StandardError; end
     class InvalidMediaTypeError < StandardError; end
     class UnauthorizedToolError < StandardError; end
+    class InvalidLaunchUrlError < StandardError; end
   end
 end

spec/controllers/external_tools_controller_spec.rb

@@ -171,6 +171,15 @@ describe ExternalToolsController do
         end
       end
 
+      context "with a bad launch url" do
+        it "fails gracefully" do
+          user_session(@teacher)
+          allow(controller).to receive(:basic_lti_launch_request).and_raise(Lti::Errors::InvalidLaunchUrlError)
+          get :show, params: {:course_id => @course.id, id: tool.id}
+          expect(response).to be_redirect
+        end
+      end
+
       context 'current user is a student view user' do
         before do
           user_session(@course.student_view_student)

spec/models/lti/lti_outbound_adapter_spec.rb

@@ -216,6 +216,15 @@ describe Lti::LtiOutboundAdapter do
       adapter.generate_post_payload
     end
 
+    it "errors if the url is ridiculous" do
+      tool_launch = double('tool launch')
+      expect(tool_launch).to receive_messages(generate: {})
+      allow(tool_launch).to receive_messages(url: "wat;no-way")
+      allow(LtiOutbound::ToolLaunch).to receive(:new).and_return(tool_launch)
+      adapter.prepare_tool_launch(return_url, variable_expander)
+      expect{ adapter.generate_post_payload }.to raise_error(::Lti::Errors::InvalidLaunchUrlError)
+    end
+
     it "does not copy query params to the post body if oauth_compliant tool setting is enabled" do
       allow(account).to receive(:all_account_users_for).with(user).and_return([])
       tool.settings = {oauth_compliant: true}