ignore bad file ids

if a file_id is given to show_relative in addition to a file_path, but
that file_id is invalid, treat is just like as if they'd given an
incorrect-but-valid id (i.e. doesn't match the path).

fixes CNVS-6148

test-plan:
  - create a file
  - get a link to the file that includes both the file_id and file_path
  - change the file id to an invalid string
  - should still load the file
  - should not generate a page error

Change-Id: Ifed3fbd9ef9fe2c5a4fd951f2de5facdf38e28d0
Reviewed-on: https://gerrit.instructure.com/21556
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>

app/controllers/files_controller.rb

@@ -345,12 +345,12 @@ class FilesController < ApplicationController
 
   def show_relative
     path = params[:file_path]
+    file_id = params[:file_id]
+    file_id = nil unless file_id.to_s =~ Api::ID_REGEX
 
     #if the relative path matches the given file id use that file
-    if params[:file_id].present? && @attachment = @context.attachments.find_by_id(params[:file_id])
-      if @attachment.matches_full_display_path?(path) || @attachment.matches_full_path?(path)
-        params[:id] = params[:file_id]
-      else
+    if file_id && @attachment = @context.attachments.find_by_id(file_id)
+      unless @attachment.matches_full_display_path?(path) || @attachment.matches_full_path?(path)
         @attachment = nil
       end
     end

lib/api.rb

@@ -91,7 +91,11 @@ module Api
         :scope => 'root_account_id' },
   }.freeze
 
-  ID_REGEX = %r{\A\d+\z}
+  # (digits in 2**63-1) - 1, so that any ID representable in MAX_ID_LENGTH
+  # digits is < 2**63, which is the max signed 64-bit integer, which is what's
+  # used for the DB ids.
+  MAX_ID_LENGTH = 18
+  ID_REGEX = %r{\A\d{1,#{MAX_ID_LENGTH}}\z}
 
   def self.sis_parse_id(id, lookups)
     # returns column_name, column_value

spec/controllers/files_controller_spec.rb

@@ -424,6 +424,15 @@ describe FilesController do
       proc { get "show_relative", :course_id => @course.id, :file_path => @file.full_display_path+"blah" }.should raise_error(ActiveRecord::RecordNotFound)
       proc { get "show_relative", :file_id => @file.id, :course_id => @course.id, :file_path => @file.full_display_path+"blah" }.should raise_error(ActiveRecord::RecordNotFound)
     end
+
+    it "should ignore bad file_ids" do
+      course_with_teacher_logged_in(:active_all => true)
+      file_in_a_module
+      get "show_relative", :file_id => @file.id + 1, :course_id => @course.id, :file_path => @file.full_display_path
+      response.should be_redirect
+      get "show_relative", :file_id => "blah", :course_id => @course.id, :file_path => @file.full_display_path
+      response.should be_redirect
+    end
   end
 
   describe "GET 'new'" do