tenancy / multi-tenant

Run multiple websites using the same Laravel installation while keeping tenant specific data separated for fully independent multi-domain setups, previously github.com/hyn/multi-tenant
https://tenancy.dev
MIT License
2.56k stars 394 forks source link

MediaController - Images not working #767

Closed xalunda closed 5 years ago

xalunda commented 5 years ago

Good afternoon,

I'm trying to implement the media route for my tenants. Here are my configurations:


routes/tenants.php:

// MEDIA
Route::get('/media/{path}', '\Hyn\Tenancy\Controllers\MediaController')
->where('path', '.+')
->name('tenant.media');

storage:

/storage/app/public/tenancy/d3442a527d7b4b08a8053755b87151f1/media/files/

where d3442a527d7b4b08a8053755b87151f1 is my tenant's uuid


My Issue:

The /media route doesn't seem to work with images. I tested .png, .jpeg, .jpg and .svg. However, it works fine with .pdf files.


Image Test: I have an image example.jpeg

Here are the response headers received from https://tenant.domain.local/media/files/example.jpeg:

Cache-Control: no-cache, private
Connection: Keep-Alive
Content-Type: image/jpeg
Date: Wed, 27 Mar 2019 16:34:42 GMT
Keep-Alive: timeout=5, max=100
Server: Apache
Transfer-Encoding: chunked
X-Powered-By: PHP/7.2.1

This method sends a broken image, whatever the image is: image

Here are the response headers accessing the file directly https://tenant.domain.local/storage/tenancy/d3442a527d7b4b08a8053755b87151f1/media/files/example.jpeg:

Accept-Ranges: bytes
Connection: Keep-Alive
Content-Length: 4960688
Content-Type: image/jpeg
Date: Wed, 27 Mar 2019 16:36:06 GMT
ETag: "2018c7a9f-4bb1b0-5572d22e2ff80"
Keep-Alive: timeout=5, max=100
Last-Modified: Sun, 20 Aug 2017 10:50:54 GMT
Server: Apache
Vary: User-Agent

This sends correctly the image.

I also tried to modify MediaController so that it includes the missing headers. No luck.

public function __invoke(string $path)
{
    $path = "media/$path";
    if ($this->directory->exists($path)) {
        $file = $this->directory->get($path);
        return response($file)
            ->header('Content-Type', Storage::disk('tenant')->mimeType($path))
            ->header('Accept-Ranges', 'bytes')
            ->header('Content-Length', strlen($file));
    }
    return abort(404);
}

PDF Test: I have a PDF example.pdf

Both methods shows the pdf.


Information

Did I miss something? Thanks

open-collective-bot[bot] commented 5 years ago

Hey :wave:,

Thank you for using our package.

We firmly believe that open sourcing our code improves the developer experience. In a pursuit to continue our work, help us by donating to our collective! :heart:

Issues opened by backers of our Open Collective will automatically labelled with the "backer" tag for priority response and resolve times.

https://opencollective.com/tenancy

xalunda commented 5 years ago

After more tests using Laravel's FileSystem->download(); and with a cleared mind this morning, found out that my output was ruined by Laravel itself, which added some whitespaces.

MediaController can make sure that this does not matter to it, by adding an ob_end_clean() right before its return.

Can be closed with PR #769 - MediaController - Images not working

luceos commented 5 years ago

ob_end_clean() might solve it, but I'm curious where laravel is introducing the white space. Using the given function seems like a rude hack to forcefully fix something which should not even happen in the first place..

xalunda commented 5 years ago

Hi @luceos , I also agree. I'm trying to pinpoint where this appends as we speak. Feel free to reject my PR

Miguel-Serejo commented 5 years ago

This may be related to https://github.com/laravel/framework/issues/27996

luceos commented 5 years ago

@36864 we know about this issue, the MediaController doesn't use a view, maybe the Response object does behind the curtains, but I doubt it.

xalunda commented 5 years ago

Found it. I had a new line on a config file before the <?php tag.. I really feel bad now.

luceos commented 5 years ago

No need to feel bad. This stuff happens to the best of us. Glad I forced you to investigate further ✌️