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

Handle encrypted (maybe also encoded) files #29

Closed
lukaspieper opened this issue Jun 7, 2023 · 11 comments
Closed

Handle encrypted (maybe also encoded) files #29

lukaspieper opened this issue Jun 7, 2023 · 11 comments

Comments

@lukaspieper
Copy link

Hi,

many thanks for creating and sharing this library. I tried migrating from SSIV (subsampling-scale-image-view) and failed because in my use case the files are encrypted. The access happens thru SAF. In SSIV I could provide a BitmapRegionDecoder that handled the decryption.

I think I could make it work by implementing the interface SubSamplingImageSource that is currently sealed.

In a second step I would like to use ZoomableAsyncImage instead of SubSamplingImage. I pass a custom data type into the ImageRequest.Builder and register a custom Fetcher for Coil that handles decryption. Because of that your ZoomableAsyncImage is already able to display my encrypted images however without subsampling. In the file CoilImageSource.kt the method ImageResult.toSubSamplingImageSource(...) would need to be adjusted to be able to map custom request data to custom SubSamplingImageSource.

Likely this is just one possible solution.

Many thanks in advance. I know that my use case might be quite rare.

@saket
Copy link
Owner

saket commented Jun 8, 2023

Very interesting!

I had to mark SubSamplingImageSource as sealed because I wasn't certain about its API stability, especially if it receives multiplatform support. Will offering a SubSamplingImageSource.source(okio.Source) solve your first step?

In the file CoilImageSource.kt the method ImageResult.toSubSamplingImageSource(...) would need to be adjusted to be able to map custom request data to custom SubSamplingImageSource.

Does this not work for your request data because its Result#diskCacheKey is always null?

@lukaspieper
Copy link
Author

Thanks for your fast reply and interest in my use case.

Will offering a SubSamplingImageSource.source(okio.Source) solve your first step?

I can provide a decrypting InputStream, so okio.Source should also work thanks to InputStream.source().

Does this not work for your request data because its Result#diskCacheKey is always null?

I debugged this and result.diskCacheKey is null. Furthermore I would prefer not to create an unencrypted copy of the file (even in the app's internal storage).

@saket
Copy link
Owner

saket commented Jun 17, 2023

Furthermore I would prefer not to create an unencrypted copy of the file (even in the app's internal storage).

I'm curious what's the concern here? App directories are inaccessible to other apps.

I can't think of good ideas to make ZoomableAsyncImage work with custom data types. Do you want to instead roll out your own ZoomableImageSource? You could wrap ZoomableImageSource.coil() or write a new one from scratch.

ZoomableImage(
  image = ZoomableImageSource.encrypted(…),
)

@lukaspieper
Copy link
Author

Hi Saket,

I'm curious what's the concern here? App directories are inaccessible to other apps.

I just want to keep the "attack surface" small. Android is very fragmented and at least until Android 12 you could utilize adb backup to dump the app's internal storage without root. On the other hand, there should also be a performance impact when copying files (on cache miss).

Do you want to instead roll out your own ZoomableImageSource?

I started digging into ZoomableImageSource and I think this will likely work for my use case. To test it we need to implement SubSamplingImageSource.source(okio.Source) or SubSamplingImageSource.inputStream(InputStream) first.

I'm quite busy the upcoming week but after that I could create a PR for the first step if you like.

@saket
Copy link
Owner

saket commented Jul 12, 2023

Sure, please go ahead!

@saket
Copy link
Owner

saket commented Jul 12, 2023

Alternatively, could you provide a custom disk cache to Coil that is backed by a fake in-memory file system?

@lukaspieper
Copy link
Author

Hi,

forked your repo, added the methods to SubSamplingImageSource and tried them with an encrypted file in my app, however getting an exception that the image format is not supported or cannot be decoded.

The only way I got it working is to provide a BitmapRegionDecoder directly, however that does not make sense to me because I also initialize that with an InputStream. It should be the same code, just split in two methods.

Will need to spend some more time looking into this. Due to a lack of time in the next days, it could take another week or two. But I'm on it, after that I will try ZoomableImageSource and your latest idea with the fake cache.

@saket
Copy link
Owner

saket commented Jul 18, 2023

On a second thought, please disregard that. A fake disk cache won't help because BitmapRegionDecoder will expect the file to exist on the real disk system.

I went ahead and implemented SubSamplingImageSource.rawSource(). Wanna try it out on 0.5.0-SNAPSHOT?

@lukaspieper
Copy link
Author

I went ahead and implemented SubSamplingImageSource.rawSource(). Wanna try it out on 0.5.0-SNAPSHOT?

Many thanks! I gave it a try and it's working fine. Will test the approach with ZoomableImageSource in den next days.

@lukaspieper
Copy link
Author

So basically copied the ZoomableImageSource.coil() implementation, removed everything I do not need and adjusted the work() and toSubSamplingImageSource() methods and it's working fine. Some methods are duplicated, but these are just a few smaller ones.

From my side, the issue would be resolved with the new method. Thanks again!

Maybe one final question, depending on the answer this would be worth a new issue then. Coil offers SubcomposeAsyncImage where I like to use loading, error and success for overlays. Would you consider such a feature?

@saket
Copy link
Owner

saket commented Jul 21, 2023

Glad to hear!

Coil offers SubcomposeAsyncImage where I like to use loading, error and success for overlays. Would you consider such a feature?

Yes please, let's move this to a new discussion so that it's searchable by others.

@saket saket closed this as completed Jul 21, 2023
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

No branches or pull requests

2 participants