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

Throws exception when content-type is not image #4

Closed
FastNinja opened this issue Dec 23, 2017 · 5 comments
Closed

Throws exception when content-type is not image #4

FastNinja opened this issue Dec 23, 2017 · 5 comments

Comments

@FastNinja
Copy link

Hi there,
Great idea - I almost implemented my own cache manager and then found your package.

However I am stuck with the following problem:

  • My images do not always have correct content type.
  • if content type is not what you expect - the plugin throw exception and does not do anything.

I found the root cause:

 if (headers.containsKey("content-type")) {
      var type = headers["content-type"].split("/");
      if (type[0] == "image") {

I do not see why would you want to check the content-type at all. I would assume that people who pass URL have to make sure it is image....

Would you accept pull request where check if (type[0] == "image") is removed?

P.S. image was uploaded to Firebase Storage using Flutter Firebase plugin that autodetects the content type and guesses it wrong 50% of the time.. I will look into fixing that plugin too - but it will require porting native code and also approval from official flutter team later..

Look forward.

@renefloor
Copy link
Collaborator

renefloor commented Dec 23, 2017

Hi FastNinja,

Thanks for your response. When I made this I was already unsure how to make this. For now I made this just for images and expected the http header to give the right extension. There is not really a need to have this only working for images though. What I was not sure about is whether I need to save the file with an extension. I think I can just ignore that all completely and just save the file with a random string as filename.

If I am correctly the ImageWidget also ignores that and just retrieves the codec from the databytes. I would propose to just remove the whole content-type header check and save the file without an extension. Especially because it is just for caching web data and not really for any user interaction further on. However, I am not sure how happy Android and iOS are with files without an extension.

What do you think?

@FastNinja
Copy link
Author

Yes, @renefloor I took you files, commented out all the lines related to content-type and all images worked!

   //if (headers.containsKey("content-type")) {
    // var type = headers["content-type"].split("/");
    // if (type[0] == "image") {
//    if (filePath == null || !filePath.endsWith(type[1])) {
    if (filePath == null) {
      Directory directory = await getTemporaryDirectory();
      var folder = new Directory("${directory.path}/imagecache");
      if (!(await folder.exists())) {
        folder.createSync();
      }
      //var fileName = "${new Uuid().v1()}.${type[1]}";
      var fileName = "${new Uuid().v1()}";
      _map["path"] = "${folder.path}/${fileName}";
    }
    //  }
    // }

However...I found that you NetworkImage file does not really work with placeholder.
but what people really need - is ability to show placeholder while image is loading.

So I did one extra step - I took another package https://github.com/FaisalAbid/pluto and changed it a bit.
I used their PlutoImage file with placeholder and injected your CachedNetworkImageProvider. it worked like a charm!

@renefloor
Copy link
Collaborator

Thanks for testing @FastNinja.
I was already thinking about removing CachedNetworkImage and only support the ImageProvider as the placeholder is not really core to this package. Did you fork the pluto library or can you directly set an ImageProvider?

I will take the header issue to #5

@FastNinja
Copy link
Author

@renefloor Pluto did not accept ImageProvider in the constructor - so I had to extract Pluto and create new constructor that accepts Image Provider. then I pass your image provider and it all worked...
I am thinking you can take the full Pluto Image concept and just reimplement/steal it..

@renefloor
Copy link
Collaborator

You could also make a pull request for Pluto that it accepts an imageprovider. Maybe I'll do that myself. Creating your own version of Pluto also means both have to be maintained.

@expilu expilu mentioned this issue Jul 23, 2020
2 tasks
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