don't use Kernel#open

fixes CNVS-19301

especially with user supplied urls!

prefixing the path with | executes it in a subprocess.
with open-uri, it follows redirects forever.

use File.open and CanvasHttp.get instead

Change-Id: I94dd7483da44ccd717b66d0e343375601cd0850c
Reviewed-on: https://gerrit.instructure.com/50442
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Clare Strong <clare@instructure.com>
Product-Review: Joel Hough <joel@instructure.com>

gems/plugins/qti_exporter/lib/qti.rb

@@ -41,7 +41,7 @@ module Qti
       sorted_paths = []
     end
     questions = []
-    doc = Nokogiri::XML(open(manifest_path))
+    doc = Nokogiri::XML(File.open(manifest_path))
     doc.css('manifest resources resource[type^=imsqti_item_xmlv2p]').each do |item|
       q = AssessmentItemConverter::create_instructure_question(opts.merge(:manifest_node=>item, :base_dir=>File.dirname(manifest_path), :sorted_file_paths => sorted_paths))
       questions << q if q
@@ -51,7 +51,7 @@ module Qti
 
   def self.convert_assessments(manifest_path, opts={})
     assessments = []
-    doc = Nokogiri::XML(open(manifest_path))
+    doc = Nokogiri::XML(File.open(manifest_path))
     doc.css('manifest resources resource[type=imsqti_assessment_xmlv2p1], manifest resources resource[type=imsqti_test_xmlv2p1]').each do |item|
       a = AssessmentTestConverter.new(item, File.dirname(manifest_path), opts).create_instructure_quiz
       assessments << a if a
@@ -84,7 +84,7 @@ module Qti
 
   def self.convert_files(manifest_path)
     attachments = []
-    doc = Nokogiri::XML(open(manifest_path))
+    doc = Nokogiri::XML(File.open(manifest_path))
     resource_nodes = doc.css('resource')
     doc.css('file').each do |file|
       # skip resource nodes, which are things like xml metadata and other sorts

gems/plugins/qti_exporter/lib/qti/assessment_item_converter.rb

@@ -52,7 +52,7 @@ class AssessmentItemConverter
 
   def create_xml_doc
     if @manifest_node
-      @doc = Nokogiri::XML(open(@href))
+      @doc = Nokogiri::XML(File.open(@href))
     else
       @doc = Nokogiri::XML(@qti_data)
     end

gems/plugins/qti_exporter/lib/qti/assessment_test_converter.rb

@@ -49,7 +49,7 @@ class AssessmentTestConverter
       end
 
       # Get the actual assessment file
-      doc = Nokogiri::XML(open(@href))
+      doc = Nokogiri::XML(File.open(@href))
       parse_quiz_data(doc)
       parse_instructure_metadata(doc)
       

gems/plugins/qti_exporter/lib/qti/converter.rb

@@ -139,7 +139,7 @@ class Converter < Canvas::Migration::Migrator
   def apply_respondus_settings
     settings_path = File.join(@unzipped_file_path, 'settings.xml')
     if File.file?(settings_path)
-      doc = Nokogiri::XML(open(settings_path))
+      doc = Nokogiri::XML(File.open(settings_path))
     end
     if doc
       respondus_settings = Qti::RespondusSettings.new(doc)

lib/canvas/migration/archive.rb

@@ -41,8 +41,14 @@ module Canvas::Migration
       if @settings[:export_archive_path]
         File.open(@settings[:export_archive_path], 'rb')
       elsif @settings[:course_archive_download_url].present?
-        # open-uri downloads the http response to a tempfile
-        open(@settings[:course_archive_download_url])
+        _, uri = CanvasHttp.validate_url(@settings[:course_archive_download_url])
+        CanvasHttp.get(@settings[:course_archive_download_url]) do |http_response|
+          raise CanvasHttp::InvalidResponseCodeError.new(http_response.code.to_i) unless http_response.code.to_i == 200
+          tmpfile = CanvasHttp.tempfile_for_uri(uri)
+          http_response.read_body(tmpfile)
+          tmpfile.rewind
+          return tmpfile
+        end
       elsif @settings[:attachment_id]
         att = Attachment.find(@settings[:attachment_id])
         att.open(:temp_folder => config[:data_folder], :need_local_file => true)

lib/canvas/migration/migrator.rb

@@ -16,7 +16,6 @@
 # with this program. If not, see <http://www.gnu.org/licenses/>.
 #
 
-require 'open-uri'
 module Canvas::Migration
 class Migrator
   include MigratorHelper

lib/canvas/migration/xml_helper.rb

@@ -86,11 +86,11 @@ module XMLHelper
   end
   
   def open_file(path)
-    File.exists?(path) ? ::Nokogiri::HTML(open(path)) : nil
+    File.exists?(path) ? ::Nokogiri::HTML(File.open(path)) : nil
   end
 
   def open_file_xml(path)
-    File.exists?(path) ? create_xml_doc(open(path)) : nil
+    File.exists?(path) ? create_xml_doc(File.open(path)) : nil
   end
 
   def create_xml_doc(string_or_io)

lib/cc/importer/standard/weblink_converter.rb

@@ -24,7 +24,7 @@ module CC::Importer::Standard
       if resource[:files] && resource[:files].first
         path = get_full_path(resource[:files].first[:href])
         if File.exists?(path)
-          xml = open(path).read
+          xml = File.open(path).read
           # because of some sadness from certain vendors clear empty namespace declarations
           xml.gsub!(/xmlns=""/, '')
           doc = create_xml_doc(xml)

lib/cc/web_resources.rb

@@ -222,9 +222,11 @@ module CC
 
           path = File.join(CCHelper::WEB_RESOURCES_FOLDER, CCHelper::MEDIA_OBJECTS_FOLDER, info[:filename])
 
-          remote_stream = open(url)
-          @zip_file.get_output_stream(path) do |stream|
-            FileUtils.copy_stream(remote_stream, stream)
+          CanvasHttp.get(url) do |http_response|
+            raise CanvasHttp::InvalidResponseCodeError.new(http_response.code.to_i) unless http_response.code.to_i == 200
+            @zip_file.get_output_stream(path) do |stream|
+              http_response.read_body(stream)
+            end
           end
 
           @resources.resource(

spec/lib/cc/cc_exporter_spec.rb

@@ -419,7 +419,8 @@ describe "Common Cartridge exporting" do
     it "should export media tracks" do
       stub_kaltura
       CanvasKaltura::ClientV3.any_instance.stubs(:startSession)
-      CanvasKaltura::ClientV3.any_instance.stubs(:flavorAssetGetPlaylistUrl).returns(Tempfile.new('blah.flv'))
+      CanvasKaltura::ClientV3.any_instance.stubs(:flavorAssetGetPlaylistUrl).returns('http://www.example.com/blah.flv')
+      stub_request(:get, 'http://www.example.com/blah.flv').to_return(body: Tempfile.new('blah.flv'), status: 200)
       CC::CCHelper.stubs(:media_object_info).returns({asset: {id: 1, status: '2'}, filename: 'blah.flv'})
       obj = @course.media_objects.create! media_id: '0_deadbeef'
       track = obj.media_tracks.create! kind: 'subtitles', locale: 'tlh', content: "Hab SoSlI' Quch!"

spec/lib/cc/cc_spec_helper.rb

@@ -17,6 +17,7 @@
 #
 
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
+require 'webmock/rspec'
 
 CC_XML_EXPORT_DIR = File.dirname(__FILE__) + '/../../fixtures/cc/cc_export'