Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebLab calls fail fast when filename is too long #24792

Merged
merged 1 commit into from Sep 14, 2018

Conversation

islemaster
Copy link
Contributor

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: (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.
  2. Storage ID
  3. Channel ID
  4. 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.

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.
Copy link
Contributor

@cpirich cpirich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Is it worth having a test case for an initial create as well (and not just a rename)? I believe they currently end up going through the same code path, so it probably doesn't matter much at the moment, but it would be good to ensure that we prevent the creation in both cases.

@islemaster
Copy link
Contributor Author

I'm having trouble testing the initial create case because I'm unable to actually create a file with a name > 512 characters on linux. Linux has a maximum filename length of 255 characters for most filesystems. and our unit tests actually create a temporary file and upload it to check initial create. Of course, rename doesn't require you to actually upload a file, so it's easier to create a name that's too long that way.

@islemaster islemaster merged commit 5ef369c into staging Sep 14, 2018
@islemaster islemaster deleted the weblab-catch-filename-too-long branch September 14, 2018 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants