Storing blob when validation fails

[anquinn]

Rails version: 6.0.2.1
Gem version: 0.8.6

Validation:
validates :bill_kit, content_type: { in: ['application/pdf', 'application/msword', 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'], message: 'is not a PDF or Word Document' }

If you upload a file other than a word doc or a PDF the validation fails (as it should) but the blob is still stored in the db. The attachment is not created.

[igorkasyanchuk]

@anquinn can you try to make a fix? or at least add a test that fails and reproduces your problem?

[anquinn]

This is my first time using active_storage, still figuring it all out. Just wanted to report the bug.

[Mate2xo]

I have the same problem on Rails 5.2. I initially made a custom ContentTypeValidator that had a value.purge statement to delete the blob and the file from strorage. The problem was that, apparently, .purge relaunches all validations : this unexpected behavior made it difficult to populate the errors array properly, as the purged record being re-validated is in a different state upon 2nd validation.
So then I came to find your gem (which is awesome, thank you), but I notice the following behavior from ActiveStorage:

• On a new record initialization, assigning a file to a new instance (without saving it!) immediately uploads the file to storage (I believe this is fixed in Rails 6)
• When validation fails, the instance is not saved, but the blob is, and the file remains in storage. So in the end, an invalid file is still uploaded to, and kept in storage.

So for now, I'll manually instruct my controller to .purge on invalidation, as I don't know how to purge inside the validator without messing with this "double validation launch" behavior. Sorry I can't suggest you a fix for this.

I'll try to write a test for this later. In the meantime you should be able to reproduce this behaviour easily in the rails console :

• assign an invalid file to a new record, and you should see the file immediately uploaded.
• save the new record to see validation fail, but the blob and file remain strored

[igorkasyanchuk]

@Mate2xo yes, this is a strange issue, and I don't have a fix.

So a possible solution would be is do create at least some snippet of code that could be used inside gem or app to cleanup attachments. I like your idea of adding a spec for this and it would be a good start to fix this issue. Your contribution is more than welcome!

[Mate2xo]

OK I'll try some things and share the results

[igorkasyanchuk]

Can someone help with the validation of this PR?
And maybe this "purge" should be added to other validators?

[Mate2xo]

Yeah I guess the .purge should be implemented on other validators (except AttachementValidator I suppose). I'll try to write some purge tests for other validators when I find some time

[Mate2xo]

In the meantime, as I am not familiar yet with Matchers syntax, if you could find why the .purge on ContentTypeValidator makes the SizeValidatorMatcher fail, that could guide us into how to call the .purge in a proper way

[kennyevil]

I think I can provide a little more insight on what's happening here, though I can't offer a solution.

I added a custom after_validation method on a model using ActiveStorage and I noticed some weird behaviour, that the file was being persisted even though the validation had failed. The after_validation method was meant to purge the file if the file validations had failed, but the file was still being persisted. I think this was because the model failed its validations, so Rails rolled back the transaction, thus undoing the purge I called. As far as I can tell, it's because the save went in this order:

ActiveStorage begins upload of new file and persists it
Validations are called
Validations fail (eg wrong content type)
Custom after_validation purges the new file
Rails rolls back the transaction, thus rolling back the purge of the invalid file

Ultimately this comes down to an immature product being released and featuring a massive security hole, that files could be uploaded, stored and available for users to download before being checked for content-type, size etc. I'm pretty steamed about this because one of the validations I'm doing is antivirus checking, so a lack of validations isn't acceptable.

I understand that this has been fixed in Rails 6.0, but my product is on Rails 5.2 and we don't currently have the time or capacity to upgrade it to Rails 6.0. We need to find a way to stop ActiveStorage jobs from running until after the validations have passed.

[Mate2xo]

If I understand correctly Rails's documentation, when a validation fails the callback chain is stopped; thus the after_validation callback is never called.

[kennyevil]

I put a binding.pry in the method on my local environment and it was definitely being called.

[Mate2xo]

Ok good to know, thanks

[samvandamme]

Just sharing my "solution" to this.

I have a job running every night to scan for blobs that don't have an attachment associated to them. This can result because of this issue, or when somebody deletes a picture, and the server only processed removing the attachment, but not the blob, or if AWS (in my case) wasn't reachable.

Here's the code

class ScanUnusedActiveStorageBlobs
  extend Resque::Plugins::Heroku

  @queue = :maintenance

  def self.perform
    inactive_for = 1.month
    inactive_blobs = ActiveStorage::Blob.where('id not in (select blob_id from active_storage_attachments)').where('created_at < ?', inactive_for.ago)

    inactive_blobs.each_with_index do |blob, index|
      blob.purge
    end
  end
end