Raise if tried to stream

[doits]

When streaming (= a block is passed to download) it only downloads a chunk for the file here and returns nil to the caller:

lockbox/lib/lockbox/active_storage_extensions.rb

Lines 82 to 83 in ef24e9d

	def download
	result = super

This leads to the following error when doing something like ActiveStorage::Attachment.last.download { |chunk| }

TypeError (can't convert NilClass to String):

/gems/lockbox-2.0.1/lib/lockbox/encryptor.rb:72:in 'Lockbox::Encryptor#check_string'
/gems/lockbox-2.0.1/lib/lockbox/encryptor.rb:24:in 'Lockbox::Encryptor#decrypt'
/gems/lockbox-2.0.1/lib/lockbox/utils.rb:113:in 'block in Lockbox::Utils.decrypt_result'
/gems/lockbox-2.0.1/lib/lockbox/utils.rb:112:in 'Lockbox::Utils.decrypt_result'
/gems/lockbox-2.0.1/lib/lockbox/active_storage_extensions.rb:91:in 'Lockbox::ActiveStorageExtensions::Attachment#download'

In addition the block receives the first chunk of encrypted data first (before the exception is raised) which I think is not expected, too.

I think it should raise like it does below, for example do this:

      def download
        raise Lockbox::Error, "Streaming not supported for encrypted files" if block_given?

        result = super

Edit: I guess it should raise for download_chunk, too?

Wdyt?

[ankane]

Hi @doits, thanks for reporting. Addressed this in the two commits above. For download, I think it's better to maximize compatibility like with open. For download_chunk (which should be less common), I think an error makes sense, but want to wait until the next major version since it breaks backwards compatibility.

[doits]

Thanks for the fast changes!

For download, I think it's better to maximize compatibility like with open

Yeah, makes sense.

Just FYI: My usecase for trying to use download with a block and using chunking was to reduce memory usage. The file in question was very big (100mb+), so I tried to send it in chunks to the user and not fully load it into memory. Which doesn't work with encrypted attachments of course, they need to be fully loaded into memory to be decrypted afaik. So there is no benefit in using download with a block, at least not from the memory perspective. Not sure if this is something that should be mentioned in the readme, e.g. chunking has no benefit for memory usage, though there are of course other reasons to use chunking besides that.