From 1596e89cefd8f807cf9b98c2ae1a2710aca257a9 Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Thu, 13 Sep 2018 17:32:11 -0700 Subject: [PATCH] WebLab calls fail fast when filename is too long WebLab put/move/copy operations return 400 BAD REQUEST when the filename is more than 512 characters long. Resolves https://app.honeybadger.io/projects/3240/faults/35530298 The key thing here is we're detecting this situation before we send a bad request to S3. This doesn't address the client behavior in this situation, but since it's already happening occasionally I think that's okay. 512 isn't entirely arbitrary. [From the S3 docs:](https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingMetadata.html) (emphasis added) > When you create an object, you specify the key name, which uniquely identifies the object in the bucket. For example, in the Amazon S3 console (see AWS Management Console), when you highlight a bucket, a list of objects in your bucket appears. These names are the object keys. The name for a key is a sequence of Unicode characters whose UTF-8 encoding is **at most 1024 bytes long.** In the case of weblab the key is not just what the student enters - there's also a prefix we use to organize project data. The whole key looks something like this: ``` files/00000001/00000001/my/cool/project.jpg ^1 ^2 ^3 ^4 ``` 1. The `files` prefix actually indicates we're interacting with the production environment. 1. Storage ID 1. Channel ID 1. Everything else is student-defined directory structure within Web Lab Our prefix isn't 512 characters, but it also means we can't give the student the full 1024. I went with a powers-of-two rule and an intuition that 512 characters is going to be enough for 99%+ of projects. --- shared/middleware/files_api.rb | 1 + shared/middleware/helpers/file_bucket.rb | 1 + .../files/rename_object_filename_too_long.yml | 564 ++++++++++++++++++ shared/test/test_files.rb | 22 + 4 files changed, 588 insertions(+) create mode 100644 shared/test/fixtures/vcr/files/rename_object_filename_too_long.yml diff --git a/shared/middleware/files_api.rb b/shared/middleware/files_api.rb index 6ad090f771de7..87a83f2426136 100644 --- a/shared/middleware/files_api.rb +++ b/shared/middleware/files_api.rb @@ -576,6 +576,7 @@ def files_put_file(encrypted_channel_id, filename, body) unescaped_filename = CGI.unescape(filename) unescaped_filename_downcased = unescaped_filename.downcase bad_request if unescaped_filename_downcased == FileBucket::MANIFEST_FILENAME + bad_request if unescaped_filename_downcased.length > FileBucket::MAXIMUM_FILENAME_LENGTH bucket = FileBucket.new manifest = get_manifest(bucket, encrypted_channel_id) diff --git a/shared/middleware/helpers/file_bucket.rb b/shared/middleware/helpers/file_bucket.rb index 757a00a2c0a93..f87c837526ff7 100644 --- a/shared/middleware/helpers/file_bucket.rb +++ b/shared/middleware/helpers/file_bucket.rb @@ -3,6 +3,7 @@ # class FileBucket < BucketHelper MANIFEST_FILENAME = 'manifest.json'.freeze + MAXIMUM_FILENAME_LENGTH = 512 def initialize super CDO.files_s3_bucket, CDO.files_s3_directory diff --git a/shared/test/fixtures/vcr/files/rename_object_filename_too_long.yml b/shared/test/fixtures/vcr/files/rename_object_filename_too_long.yml new file mode 100644 index 0000000000000..085f54494def9 --- /dev/null +++ b/shared/test/fixtures/vcr/files/rename_object_filename_too_long.yml @@ -0,0 +1,564 @@ +--- +http_interactions: +- request: + method: get + uri: https://cdo-v3-files.s3.amazonaws.com/files_test/1/1/manifest.json + body: + encoding: UTF-8 + string: '' + headers: + Content-Length: + - '0' + response: + status: + code: 404 + message: Not Found + headers: + Content-Type: + - application/xml + Transfer-Encoding: + - chunked + Date: + - Fri, 14 Sep 2018 00:26:14 GMT + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: |- + + NoSuchKeyThe specified key does not exist.files_test/1/1/manifest.json504D13988B6EB8F6hqq77K0eRC/1HJjGM94e5Jb+w5JKEhXvT6ddRFP8BacCUEP9CNhU7EpZZrcCT2jPOpsBXuFU1Mo= + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:14 GMT +- request: + method: get + uri: https://cdo-v3-files.s3.amazonaws.com/?encoding-type=url&prefix=files_test/1/1/old_fileac0a7f8c2faac49775a6.html&versions + body: + encoding: UTF-8 + string: '' + headers: + Content-Length: + - '0' + response: + status: + code: 200 + message: OK + headers: + Date: + - Fri, 14 Sep 2018 00:26:15 GMT + Content-Type: + - application/xml + Transfer-Encoding: + - chunked + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: |- + + cdo-v3-filesfiles_test/1/1/old_fileac0a7f8c2faac49775a6.html1000urlfalsefiles_test/1/1/old_fileac0a7f8c2faac49775a6.htmlXeE4zbyCWcdtFj9zz5BtdkLcVP8jc8aMtrue2018-09-14T00:14:23.000Zcf6bd5437eaccbf2d79d5d40694e94c727ef59eb29caa52acbc32daffeb8e9e4sitefiles_test/1/1/old_fileac0a7f8c2faac49775a6.htmlh0WFzSBtrtKvenikZaXW3_7Br7ZqZwKbfalse2018-09-14T00:14:21.000Z"458b68094909507177f21dc1c238829d"14cf6bd5437eaccbf2d79d5d40694e94c727ef59eb29caa52acbc32daffeb8e9e4siteSTANDARD + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:15 GMT +- request: + method: post + uri: https://cdo-v3-files.s3.amazonaws.com/?delete + body: + encoding: UTF-8 + string: | + + + files_test/1/1/old_fileac0a7f8c2faac49775a6.html + h0WFzSBtrtKvenikZaXW3_7Br7ZqZwKb + + + files_test/1/1/old_fileac0a7f8c2faac49775a6.html + XeE4zbyCWcdtFj9zz5BtdkLcVP8jc8aM + + true + + headers: + Expect: + - 100-continue + Content-Md5: + - lMt9TLBnKIbs5pPD4Lr0nA== + Content-Length: + - '383' + response: + status: + code: 200 + message: OK + headers: + Date: + - Fri, 14 Sep 2018 00:26:16 GMT + Connection: + - close + Content-Type: + - application/xml + Transfer-Encoding: + - chunked + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: |- + + + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:15 GMT +- request: + method: get + uri: https://cdo-v3-files.s3.amazonaws.com/?encoding-type=url&prefix=files_test/1/1/old_fileac0a7f8c2faac49775a6.html&versions + body: + encoding: UTF-8 + string: '' + headers: + Content-Length: + - '0' + response: + status: + code: 200 + message: OK + headers: + Date: + - Fri, 14 Sep 2018 00:26:16 GMT + Content-Type: + - application/xml + Transfer-Encoding: + - chunked + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: |- + + cdo-v3-filesfiles_test/1/1/old_fileac0a7f8c2faac49775a6.html1000urlfalse + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:15 GMT +- request: + method: get + uri: https://cdo-v3-files.s3.amazonaws.com/?encoding-type=url&prefix=files_test/1/1/manifest.json&versions + body: + encoding: UTF-8 + string: '' + headers: + Content-Length: + - '0' + response: + status: + code: 200 + message: OK + headers: + Date: + - Fri, 14 Sep 2018 00:26:17 GMT + Content-Type: + - application/xml + Transfer-Encoding: + - chunked + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: |- + + cdo-v3-filesfiles_test/1/1/manifest.json1000urlfalse + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:16 GMT +- request: + method: get + uri: https://cdo-v3-files.s3.amazonaws.com/files_test/1/1/manifest.json + body: + encoding: UTF-8 + string: '' + headers: + Content-Length: + - '0' + response: + status: + code: 404 + message: Not Found + headers: + Content-Type: + - application/xml + Transfer-Encoding: + - chunked + Date: + - Fri, 14 Sep 2018 00:26:15 GMT + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: |- + + NoSuchKeyThe specified key does not exist.files_test/1/1/manifest.jsonC0499F195E516993OcrFO9qiRruGCavqnkn/reCeeHvqK+3sJdU1vXH1UAn1lEwY/G/kNgkThI/YDNKRoD0N19wIIEY= + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:16 GMT +- request: + method: get + uri: https://cdo-v3-files.s3.amazonaws.com/?encoding-type=url&prefix=files_test/1/1/ + body: + encoding: UTF-8 + string: '' + headers: + Content-Length: + - '0' + response: + status: + code: 200 + message: OK + headers: + Date: + - Fri, 14 Sep 2018 00:26:17 GMT + X-Amz-Bucket-Region: + - us-east-1 + Content-Type: + - application/xml + Transfer-Encoding: + - chunked + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: |- + + cdo-v3-filesfiles_test/1/1/1000urlfalse + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:16 GMT +- request: + method: put + uri: https://cdo-v3-files.s3.amazonaws.com/files_test/1/1/old_fileac0a7f8c2faac49775a6.html + body: + encoding: ASCII-8BIT + string: fake-file-data + headers: + X-Amz-Meta-Abuse-Score: + - '0' + Expect: + - 100-continue + Content-Md5: + - RYtoCUkJUHF38h3BwjiCnQ== + Content-Length: + - '14' + response: + status: + code: 200 + message: OK + headers: + Date: + - Fri, 14 Sep 2018 00:26:18 GMT + X-Amz-Version-Id: + - VUk3mTnPAGEVDx1XdZu0FRMc4lKeR_zl + Etag: + - '"458b68094909507177f21dc1c238829d"' + Content-Length: + - '0' + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: '' + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:17 GMT +- request: + method: put + uri: https://cdo-v3-files.s3.amazonaws.com/files_test/1/1/manifest.json + body: + encoding: UTF-8 + string: '[{"filename":"old_fileac0a7f8c2faac49775a6.html","category":"text","size":14,"versionId":"VUk3mTnPAGEVDx1XdZu0FRMc4lKeR_zl","timestamp":"2018-09-13T17:26:17.195-07:00"}]' + headers: + X-Amz-Meta-Abuse-Score: + - '0' + Expect: + - 100-continue + Content-Md5: + - EDBgyU/DSDMh7U519WGRug== + Content-Length: + - '169' + response: + status: + code: 200 + message: OK + headers: + Date: + - Fri, 14 Sep 2018 00:26:18 GMT + X-Amz-Version-Id: + - IyObkRYt1T4pyT9d..t1W7v4os95OQFp + Etag: + - '"103060c94fc3483321ed4e75f56191ba"' + Content-Length: + - '0' + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: '' + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:17 GMT +- request: + method: get + uri: https://cdo-v3-files.s3.amazonaws.com/files_test/1/1/long_filename________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________.html + body: + encoding: UTF-8 + string: '' + headers: + Content-Length: + - '0' + response: + status: + code: 404 + message: Not Found + headers: + Content-Type: + - application/xml + Transfer-Encoding: + - chunked + Date: + - Fri, 14 Sep 2018 00:26:17 GMT + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: |- + + NoSuchKeyThe specified key does not exist.files_test/1/1/long_filename________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________.htmlDE215D49B9285792FPtaxnEbusX9xGLqXNDt3GsVgA2NZ/b2wPbdD2jisXk7bbMyMSc03PE6hVE0NovSFOPRkMi7JWQ= + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:17 GMT +- request: + method: get + uri: https://cdo-v3-files.s3.amazonaws.com/files_test/1/1/old_fileac0a7f8c2faac49775a6.html + body: + encoding: UTF-8 + string: '' + headers: + Content-Length: + - '0' + response: + status: + code: 200 + message: OK + headers: + Date: + - Fri, 14 Sep 2018 00:26:19 GMT + Last-Modified: + - Fri, 14 Sep 2018 00:26:18 GMT + Etag: + - '"458b68094909507177f21dc1c238829d"' + X-Amz-Meta-Abuse-Score: + - '0' + X-Amz-Version-Id: + - VUk3mTnPAGEVDx1XdZu0FRMc4lKeR_zl + Accept-Ranges: + - bytes + Content-Type: + - '' + Content-Length: + - '14' + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: fake-file-data + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:18 GMT +- request: + method: get + uri: https://cdo-v3-files.s3.amazonaws.com/files_test/1/1/manifest.json + body: + encoding: UTF-8 + string: '' + headers: + Content-Length: + - '0' + response: + status: + code: 200 + message: OK + headers: + Date: + - Fri, 14 Sep 2018 00:26:19 GMT + Last-Modified: + - Fri, 14 Sep 2018 00:26:18 GMT + Etag: + - '"103060c94fc3483321ed4e75f56191ba"' + X-Amz-Meta-Abuse-Score: + - '0' + X-Amz-Version-Id: + - IyObkRYt1T4pyT9d..t1W7v4os95OQFp + Accept-Ranges: + - bytes + Content-Type: + - '' + Content-Length: + - '169' + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: '[{"filename":"old_fileac0a7f8c2faac49775a6.html","category":"text","size":14,"versionId":"VUk3mTnPAGEVDx1XdZu0FRMc4lKeR_zl","timestamp":"2018-09-13T17:26:17.195-07:00"}]' + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:18 GMT +- request: + method: put + uri: https://cdo-v3-files.s3.amazonaws.com/files_test/1/1/manifest.json + body: + encoding: UTF-8 + string: "[]" + headers: + X-Amz-Meta-Abuse-Score: + - '0' + Expect: + - 100-continue + Content-Md5: + - 11FxOYiYfpMxmANj4kGJzg== + Content-Length: + - '2' + response: + status: + code: 200 + message: OK + headers: + Date: + - Fri, 14 Sep 2018 00:26:19 GMT + X-Amz-Version-Id: + - ASNMdK39C.YS645GxfT9WF8v1n.BfUfN + Etag: + - '"d751713988987e9331980363e24189ce"' + Content-Length: + - '0' + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: '' + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:19 GMT +- request: + method: delete + uri: https://cdo-v3-files.s3.amazonaws.com/files_test/1/1/old_fileac0a7f8c2faac49775a6.html + body: + encoding: UTF-8 + string: '' + headers: + Content-Length: + - '0' + response: + status: + code: 204 + message: No Content + headers: + Date: + - Fri, 14 Sep 2018 00:26:20 GMT + X-Amz-Version-Id: + - oWKsjuSOYr3rlZlIcuWBkgggNFbaKwh1 + X-Amz-Delete-Marker: + - 'true' + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: '' + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:19 GMT +- request: + method: get + uri: https://cdo-v3-files.s3.amazonaws.com/?encoding-type=url&prefix=files_test/1/1/manifest.json&versions + body: + encoding: UTF-8 + string: '' + headers: + Content-Length: + - '0' + response: + status: + code: 200 + message: OK + headers: + Date: + - Fri, 14 Sep 2018 00:26:20 GMT + Content-Type: + - application/xml + Transfer-Encoding: + - chunked + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: |- + + cdo-v3-filesfiles_test/1/1/manifest.json1000urlfalsefiles_test/1/1/manifest.jsonASNMdK39C.YS645GxfT9WF8v1n.BfUfNtrue2018-09-14T00:26:19.000Z"d751713988987e9331980363e24189ce"2cf6bd5437eaccbf2d79d5d40694e94c727ef59eb29caa52acbc32daffeb8e9e4siteSTANDARDfiles_test/1/1/manifest.jsonIyObkRYt1T4pyT9d..t1W7v4os95OQFpfalse2018-09-14T00:26:18.000Z"103060c94fc3483321ed4e75f56191ba"169cf6bd5437eaccbf2d79d5d40694e94c727ef59eb29caa52acbc32daffeb8e9e4siteSTANDARD + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:19 GMT +- request: + method: post + uri: https://cdo-v3-files.s3.amazonaws.com/?delete + body: + encoding: UTF-8 + string: | + + + files_test/1/1/manifest.json + ASNMdK39C.YS645GxfT9WF8v1n.BfUfN + + + files_test/1/1/manifest.json + IyObkRYt1T4pyT9d..t1W7v4os95OQFp + + true + + headers: + Expect: + - 100-continue + Content-Md5: + - Vo5w2669bW4d3UsiVwdXCw== + Content-Length: + - '343' + response: + status: + code: 200 + message: OK + headers: + Date: + - Fri, 14 Sep 2018 00:26:20 GMT + Connection: + - close + Content-Type: + - application/xml + Transfer-Encoding: + - chunked + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: |- + + + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:20 GMT +- request: + method: get + uri: https://cdo-v3-files.s3.amazonaws.com/files_test/1/1/manifest.json + body: + encoding: UTF-8 + string: '' + headers: + Content-Length: + - '0' + response: + status: + code: 404 + message: Not Found + headers: + Content-Type: + - application/xml + Transfer-Encoding: + - chunked + Date: + - Fri, 14 Sep 2018 00:26:19 GMT + Server: + - AmazonS3 + body: + encoding: UTF-8 + string: |- + + NoSuchKeyThe specified key does not exist.files_test/1/1/manifest.json04142AAD8D4791A6NycKPklMz0wPSO1qsRXVKQswCrkGe9H9STRMhPGoOY3iUIGFxqSI/24pVmfxKxBvl2XRWXGirgE= + http_version: + recorded_at: Fri, 14 Sep 2018 00:26:20 GMT +recorded_with: VCR 3.0.3 diff --git a/shared/test/test_files.rb b/shared/test/test_files.rb index ced70e77e7ccf..b955c26cfe621 100644 --- a/shared/test/test_files.rb +++ b/shared/test/test_files.rb @@ -68,6 +68,28 @@ def test_rename_object delete_all_manifest_versions end + def test_rename_object_filename_too_long + file_data = 'fake-file-data' + old_filename = @api.randomize_filename 'old_file.html' + new_filename = "long_filename#{'_' * 512}.html" + delete_all_file_versions old_filename, URI.escape(old_filename) + delete_all_manifest_versions + post_file_data @api, old_filename, file_data, 'test/html' + + @api.rename_object old_filename, new_filename + assert bad_request? + @api.get_object(new_filename) + assert not_found? + @api.get_object(old_filename) + assert successful? + assert_equal file_data, last_response.body + + @api.delete_object(old_filename) + assert successful? + + delete_all_manifest_versions + end + def test_upload_files dog_image_filename = @api.randomize_filename('dog.png') dog_image_body = 'stub-dog-contents'