Audio file validation

[4b11b4]

Audio files are not currently validated. What file types should be accepted? For now, let's assume only mp3s for simplicity.

From some initial reading on file validation with Django:

1. Should this be dealt with using a validator? If so, which directory does the validators.py file go in? Does it go in the potify/potify/potify? or potify/potify/music? Based on the way it is imported in the example below, it must go in the same folder as the models.py that you are trying to create the validator for? https://stackoverflow.com/questions/3648421/only-accept-a-certain-file-type-in-filefield-server-side

2. One person points out that checking for filename extensions is not secure. I could simply rename a binary to .mp3. They suggest that checking for file cotent is more secure. --from Answer #3: https://stackoverflow.com/questions/6460848/how-to-limit-file-types-on-file-uploads-for-modelforms-with-filefields

[zachcalvert]

Great points, particularly the second one I hadn't thought of that

1. Adding a validators.py to potify/potify/music makes the most sense to me. That way if we end up needing additional field validators for any of the other models in the music app (and we probably will), we can define them in that file.

2. I love the idea of doing a more thorough validation :shipit:

[4b11b4]

Pushed a change. I didn't make a new branch for a pull request.

Django 1.11 has a built-in FileExtensionValidator. This can be used when defining the audio object for the music/Song model.

As far as testing... The admin site does prevent me from uploading a PNG.

[4b11b4]

The file extension is checked... now to check the content of the file...

see music/models.py line 33

• Validators are added as parameters to the constructor of the audio object in the Song model.
• The parameter validators is a list. I appended the built-in FileExtensionValidator.
• ...then, we append a second validator which checks the content type?

note:

1. Say we have the FileExtensionValidator run first. The extension is valid. Success. Next, the FileContentValidator runs... but it doesn't know what type of content it should be looking for. The ExtensionValidator needs to tell the ContentValidator which type to validate.

   • so... we should write a custom validator that contains both checks? then, we don't have to pass any information around?

[zachcalvert]

I think you're onto something with a custom validator that contains both checks. We could even write a custom validator that inherits from Django's built-in FileExtensionValidator, and thereby gets that functionality, and also validates the content with our own custom logic.

With this approach, validators.py could have a class like

from django.core.validators import FileExtensionValidator

class SongUploadValidator(FileExtensionValidator):
    def __call__(self, value):
        # call to super does the file extension validation
        self.super(SongUploadValidator, self)(value)

        # then check against content
        # if value.content_type not in 'mp3'  # pseudocode
        #    raise ValidationError("The protocol for this URL must be http:// or https://")

[4b11b4]

Seems like there are a variety of ways to accomplish this... See the link below. This method doesn't create a validator. Instead it creates a custom FileField.

It checks for:

1. extension
2. content
3. file size

http://nemesisdesign.net/blog/coding/django-filefield-content-type-size-validation/

At the bottom, there's a note about Django's default FILE_UPLOAD_MAX_MEMORY_SIZE which is set to 2.5MB.

I'm not sure how this comes into effect -- maybe only from the user view? -- I was able to upload an MP3 through the admin console of 4.8MB.

[4b11b4]

from: https://docs.djangoproject.com/en/2.1/topics/security/#user-uploaded-content-security

No bulletproof technical solution exists at the framework level to safely validate all user uploaded file content

more questions:

1. The validation we are creating is only a screen to prevent the majority of invalid files from being uploaded?
2. To be fully secure, you'd need to have the server inspect each file?
3. This does not happen client-side. At least not with Django?

[zachcalvert]

Dang! This is not a trivial problem, here are my thoughts on these questions

1. Yes
2. I think you're right, to be fully secure I suppose we'd need to inspect the content type as well as the contents of each file. Somehow.
3. True, with only Django at our disposal, this doesn't happen client side. Another tool could perhaps help us.

It's also worth keeping in mind that only site admins can access the form to upload new songs, and we have to trust admins to a certain extent by definition anyway. A regular user of the site won't even have the opportunity to upload unsupported file types. With the info you've gathered, I think fully solving this problem is not worth the cycles.

If it becomes a problem in the future we can re-open, but for now I think the file extension validator does what we need. There's too many other cool things we can build for this site, no need to prematurely optimize for this right now, in my opinion.

[4b11b4]

Premature optimization is the root of all evil.