Unescaped filenames were causing issues in multipart form uploads.
Filenames that had unmatched quotes were corrupting the payload and
causing direct uploads to inst-fs to fail (anything uploaded from
the app server directly). This was not an issue for files uploaded
from browsers.
Escape the names before injecting them into the multipart payload.
Test plan: specs
refs SAS-1400
Change-Id: Ib842652ed7cf0bd3cab88c81e69586620eced679
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/225242
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
QA-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
Product-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
sequenced_stream becomes really confused when we attempt to post a file
that has non-ascii characters in the name. Add explicit conversion of
individual buffers to UTF-8 on read to prevent this issue.
Test Plan:
* Configure local inst-fs with canvas (see readme in repo for
instructions)
* Open a canvas rails console
`bundle exec rails c`
* Run a manual inst-fs upload of a binary file (like a jpg) with
non-ascii characters in the filename
```
InstFS.direct_upload(
file_name: "祝贺.jpg",
file_object: File.open("congrats.jpg")
)
```
* Upload should complete without throwing
closes SAS-312
Change-Id: I724cf77701171074cfb5317c362c963d60ecc15f
Reviewed-on: https://gerrit.instructure.com/172075
Reviewed-by: Andrew Huff <ahuff@instructure.com>
Tested-by: Jenkins
QA-Review: Andrew Huff <ahuff@instructure.com>
Product-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
fixes RECNVS-583, RECNVS-594, RECNVS-595
reverts commit efdb13e9a9, restoring
3da400a75e, but adds fixes for the bugs
that caused the original revert. in particular:
* refactor to avoid `eof?` (RECNVS-594)
in some situations, the file contents stream doesn't implement `eof?`.
the only required interface for `IO.copy_stream` is `size` and `read`,
so we'll restrict our interface to that and only expect no more than
that interface of the parts.
* keep trying the same stream on partial read (RECNVS-595)
the IO#read docs indicate that if the read result has fewer than the
bytes requested, it's because the stream is at EOF. the
SequencedStream output sticks to that behavior. but in some
situations, the file contents stream will return a partial read before
EOF. so we need to keep asking for the remainder until we either get
as much as we wanted or an explicit EOF (nil returned).
test-plan:
- ensure the following all still work with inst-fs enabled:
- content exports, particularly of larger courses. don't just verify
the file is created and can be downloaded from inst-fs, but the
generated file should also be a valid zip file (file extension is
imscc, but format is zip) that can be extracted into the expected
course contents. exercises RECNVS-595
- content migrations, particularly of larger courses, where you choose
a source Canvas course when executing "Import Course Content" in a
destination Canvas course. also exercises RECNVS-595.
- SIS imports via API
- create a simple courses.csv with a new course defined in it a la
https://canvas.instructure.com/doc/api/file.sis_csv.html
- POST this CSV to the SIS imports API documented at
https://canvas.instructure.com/doc/api/sis_imports.html#method.sis_imports_api.create
use the mechanism described under the text "If you decide to do a raw post,
you can skip the 'attachment' argument..." in the description of
the attachment parameter. exercises RECNVS-594.
Change-Id: Ie7e7fecff491ac2344a48d97f1c6b27eb24885c8
Reviewed-on: https://gerrit.instructure.com/162584
Reviewed-by: Xander Moffatt <xmoffatt@instructure.com>
Tested-by: Jenkins
Product-Review: Xander Moffatt <xmoffatt@instructure.com>
QA-Review: Xander Moffatt <xmoffatt@instructure.com>
This reverts commit 3da400a75e.
closes RECNVS-592
Change-Id: I51fb152a1ceef45a51b4434575afd1b25a955aa5
Reviewed-on: https://gerrit.instructure.com/162120
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
Tested-by: Jenkins
Product-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
QA-Review: Jonathan Featherstone <jfeatherstone@instructure.com>
fixes RECNVS-547
generally, in Multipart::Post, you can get a stream representation of
the body instead of a string, and in CanvasHttp.post you can request
that stream be supplied to the underlying request's body_stream instead
of the string to the underlying body. inst-fs then supplies the
streaming flag during direct upload.
this avoids an error when the body as a single string is too large to
POST.
test-plan:
- confirm general upload-from-canvas functionality is preserved by
triggering uploads from canvas to another server
- e.g. inst-fs enabled course export
- e.g. inst-fs enabled course export
- confirm specific upload-from-canvas functionality with streamed larger
files
- i.e. inst-fs enabled course export where the export size is larger
than 2GB
- note that there may be a new HTTP 504 error when the upload size
approaches 3GB. this is a separate issue not related to this commit
Change-Id: I15e22e6688233a77783d1cd86960bbaaa64eb0dc
Reviewed-on: https://gerrit.instructure.com/160105
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
QA-Review: Michael Jasper <mjasper@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
closes RECNVS-324
test plan
- use the `InstFS.direct_upload` method to upload files to canvas through the
the rails console
- an example of a rails console command is documented in the method
- verify that uploads from canvas are available in inst-fs
Change-Id: I20e5d2544a0853fd913ebcba5229e9977825f482
Reviewed-on: https://gerrit.instructure.com/142411
Tested-by: Jenkins
QA-Review: Collin Parrish <cparrish@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>
Product-Review: Jacob Fugal <jacob@instructure.com>
as of this commit, all canvas gems should be on rspec 3.5, and pass
without deprecation warnings.
closes CNVS-34040
test plan: specs should pass without deprecation warnings
Change-Id: I556b1a4a5aeb791c6ddd50ee35b51c513e025019
Reviewed-on: https://gerrit.instructure.com/98414
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Landon Wilkins <lwilkins@instructure.com>
QA-Review: Landon Wilkins <lwilkins@instructure.com>
Tested-by: Jenkins
rely on test_all_gems.sh to output header and trailer,
and use `set -e` in each test.sh to simplify passing
through errors
Change-Id: I3ba724ad2539ddfe31195394c43f646acfc73920
Reviewed-on: https://gerrit.instructure.com/70469
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
refs CNVS-13987
what was called CanvasUuid was *not* generating UUIDs. it was generating
slugs. by default, its generate method only creates 4 character slugs.
these should obviously not be used as UUIDs. the misnomer already caused
a bug in EventStream where it used these slugs as UUIDs, causing
collisions. to fix:
(1) rename canvas_uuid gem to canvas_slug, and rename it's primary
class CanvasUuid to CanvasSlug
(2) create new canvas_uuid gem, with class CanvasUUID, extracted from
lib/uuid_singleton for actual UUID generation
(3) fix event stream use CanvasUUID, rather than following the rename
of CanvasUuid to CanvasSlug
test-plan:
- have cassandra set up for audit logs
- create an audit log entry (e.g. change a grade)
- look at the generated audit log entry's id field; it should be a UUID
value, not a 4 character slug
Change-Id: I19758fff4433cd6cb2e21219217dced19ee05c5a
Reviewed-on: https://gerrit.instructure.com/37506
Reviewed-by: Rob Orton <rob@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: August Thornton <august@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Brian Palmer <brianp@instructure.com>
they're likely already installed, and it will save a ton of trips
to rubygems.org
Change-Id: I9ccf2194619a6e8f97d7f21b4e232dac7ff20d3c
Reviewed-on: https://gerrit.instructure.com/35694
Reviewed-by: Bryan Madsen <bryan@instructure.com>
QA-Review: Bryan Madsen <bryan@instructure.com>
Product-Review: Bryan Madsen <bryan@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Change-Id: I0dad5345aae75e552af97f5b54ded10bbfebbe37
Reviewed-on: https://gerrit.instructure.com/33925
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Mike Nomitch <mnomitch@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
This reverts commit e3148346df
This reverts commit 16f518a130
This partially reverts 9d34baed66
fixes CNVS-12021
test plan (e3148346df)
- rails 3 specs should pass
test plan (9d34baed66)
- import content including a video file into a course
(such as the QTI file attached to CNVS-11602)
- the import should complete
test plan (16f518a130)
- submit a document to turnitin
- it should successfully receive a score
Change-Id: I0622d9eceee4b94e488f103db68290f1b641b555
Reviewed-on: https://gerrit.instructure.com/32333
Reviewed-by: Brian Palmer <brianp@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: Amber Taniuchi <amber@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
Jonathan Featherstone