spatie / laravel-glide

Easily convert images with Glide
https://freek.dev/2015/12/easily-convert-images-with-glide/
MIT License
424 stars 51 forks source link

Fix GlideImageController #59

Closed n7olkachev closed 8 years ago

n7olkachev commented 8 years ago

$server->outputImage already sends StreamedResponse and returns $request, so we can't return it from controller.

freekmurze commented 8 years ago

Thanks for your contribution.

Unless their are bugs that are breaking the package, I'm not planning on changing the code base.

n7olkachev commented 8 years ago

@freekmurze Actually it is very serious bug. $request returned from controller is echoing to the response. I found it during fixing bug with Safari doesn't show images served on HTTPS, because of it. So, please, accept it.

freekmurze commented 8 years ago

Ok, I'll verify the PR soon. If everything is ok I'll tag a new version in the next few days.

On Saturday, 16 July 2016, Nick Tolkachev notifications@github.com wrote:

@freekmurze https://github.com/freekmurze Actually it is very serious bug. $request returned from controller is echoing to the response. I found it during fixing bug with Safari doesn't show images served on HTTPS, because of it. So, please, accept it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/spatie/laravel-glide/pull/59#issuecomment-233088988, or mute the thread https://github.com/notifications/unsubscribe-auth/AAdiDf7X3VH5Yykk0gU2IBFcZ7Gz7Rjeks5qWBG-gaJpZM4JNfKf .

Freek Van der Herten https://spatie.be +32 495 84 27 91

n7olkachev commented 8 years ago

Thx a lot!

freekmurze commented 8 years ago

Can you help me understand your fix?

This is Glide's outputImage function:

    public function outputImage()
    {
        $request = (new RequestArgumentsResolver())->getRequest(func_get_args());

        $this->makeImage($request);

        ResponseFactory::create($this->cache, $request, $this->getCachePath($request))->send();

        return $request;
    }

It seems to me that if we don't return the output of that function in our controller, there will be no response at all.

n7olkachev commented 8 years ago
ResponseFactory::create($this->cache, $request, $this->getCachePath($request))->send();

Response is sending on this line (send method), while outputImage returns request, not response.

freekmurze commented 8 years ago

You're right! Thanks!

freekmurze commented 8 years ago

Tagged new version 2.3.4.