instructure / canvas-lms

The open LMS by Instructure, Inc.
https://github.com/instructure/canvas-lms/wiki
GNU Affero General Public License v3.0
5.42k stars 2.42k forks source link

Error When Re-Uploading Submissions Files to S3 without InstFS Enabled #2278

Open imsujinpark opened 8 months ago

imsujinpark commented 8 months ago

Summary:

I am currently utilizing AWS S3 storage in combination with the absence of Instructure File System (InstFS) activation. I've encountered an issue when attempting to re-upload student submissions in Assignments. While the file successfully uploads to S3, the final "create_success" API call results in a "400 Bad Request" error.

Steps to reproduce:

  1. Configure storage settings to use AWS S3 instead of local storage. Disable InstFS if it is currently enabled.
  2. On Canvas LMS website, navigate to the "Assignments" section and select an assignment containing student submissions. Follow this link for instructions until Step 4. with images.
  3. Click 'Download Submissions'.
  4. Choose 'Re-Upload Submissions' and select the previously downloaded zip file.
  5. Open the 'Network' tab in the Developer's tools.
  6. Click 'Upload Files'.

    Expected Behavior:

  7. Upon clicking 'Upload Files', you should observe three API calls in sequence: 1. POST - pending 2. POST - upload_url 3. GET/POST - create_success.
  8. All API calls should return responses with status codes such as 200 OK, 201 Created, or 302 Found, successfully uploading the zip file.

Actual Behavior:

  1. The final API call, 'create_success,' returns a "400 Bad Request" error.
  2. Examination of the database entries for the file reveals workflow_state='unattached_temporary'.
  3. When I inspect the attachment.state using the Rails console, it also displays unattached_temporary.

Additional Notes:

  1. As per the 'setupSubmitHandler' function in 'app/jsx/shared/helpers/reuploadSubmissionsHelper,' the re-upload submissions process is associated with the intent: 'submissions_zip_upload'.
  2. The 'temporary_file' method in 'files_controller.rb' returns true if the intent does not match ['message', 'attach_discussion_file', 'upload'] or ['comment', 'submit'].
  3. However, when the 'api_attachment_preflight' method is invoked during the first API call, the attachment is assigned the state 'unattached_temporary' if it receives the parameter opts[:temporary] == true.
  4. According to the 'api_create_success' method in 'files_controller.rb,' a "Bad Request" error is triggered if the attachment.state is not 'unattached,' while using S3 storage which is checked during the third API call.

Based on the sequence of events, it appears that using S3 storage while disabling InstFS may lead to a "Bad Request" error when re-uploading submissions.

I'm also curious about why we're filtering attachments with a status other than 'unattached' when using S3 storage.

// app/controllers/files_controller.rb
    if Attachment.s3_storage?
      return head(:bad_request) unless @attachment.state == :unattached
      details = @attachment.s3object.data
      @attachment.process_s3_details!(details)
    else
      @attachment.file_state = 'available'
      @attachment.save!
    end

Understanding the reasoning behind this condition would be really helpful for addressing the issue.