getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.32k stars 170 forks source link

Asset media urls are broken #2314

Closed qwerdee closed 3 years ago

qwerdee commented 5 years ago

Describe the bug
The media url for assets are broken. The assets are not copied into the media folder. The same happens with file versions of an asset.

To Reproduce

  1. Try to access an asset with it's media url e.g.:
    <img src="<?= asset('assets/test.png')->mediaUrl() ?>">
    // https://mysite.test/media/assets/assets/702945454-1574246480/test.png
    // produces 404

    or

    <img src="<?= asset('assets/test.png')->resize(100)->url() ?>">
    // https://mysite.test/media/assets/assets/702945454-1574246480/test-100x.png"
    // produces 404

Expected behavior
I would expect that the asset would be copied to the media folder as soon as it is requested for the first time via it's media url.

Kirby Version
3.3.0

Additional context
This seems to have the same cause as #1728

SinisaG commented 5 years ago

EDIT

We actually caught an error on our side. Somehow an empty line sneaked in to our code - at the top of the file - which was appended to beginning of each file. The file was actually included in our config.php.

I leave the issue description under, as it might be useful to someone that stumbles upon the same issue.

Also, I think charset=UTF-8 should not be appended to content-type when image is returned. Or I am missing the idea behind it?


Original issue

Hey, we are experiencing similar issue. So if you call asset url directly within the browser, you always get 404, if asset does not yet exist in media folder. This is if you try to access image url directly.

If you try to load the page referencing assets, the image is created in media folder, but returned wrongly (treated as text file), which break the image display. Second request works, as image is already in media folder.

Any idea what could be going on here?

More info about the setup - we use NGINX with PHP7.2-FPM.

I've tried to tone down nginx to minimum and here's how the configuration looks like.

server {
  listen 80;

  server_name mykirby.de;

  root /var/www/kirby/public;
  index index.php;

  location / {
    try_files $uri $uri/ /index.php?$uri&$args;
  }

  location ~ \b(?!\bhtml\b)\w+\b\.php$ {
    include snippets/fastcgi-php.conf;
    fastcgi_pass unix:/var/run/php7.2-kirby3.sock;
  }
}

Additional: base is set to public folder (that's why root is pointing to subfolder).

Returned image headers:

content-type: image/jpeg; charset=UTF-8
date: Mon, 25 Nov 2019 10:43:36 GMT
server: nginx
status: 200

Kirby version: 3.2.2 and 3.2.5

steirico commented 4 years ago

Thanks @SinisaG for the hint with empty line.

Sorry for hijacking this issue, but IMHO my experience is hardly related and the solution might be useful for others, too.

I experienced that medias served for the very first time were "polluted" with an preceding newline character. Amongst others, this lead to broken image files served with a 200 HTTP Code. Due to caching headers sent by my provider's server, these files we not reloaded for some time and the broken images were used instead.

The superfluous newline character was introduced by one of my plugin's files starting with a newline followed by the <?php tag. Removing the newline solved the issue.

distantnative commented 4 years ago

@qwerdee I am closing this for now, assuming that your issue might have been caused by the same reasons outlined by @SinisaG and @steirico. Please let us know if this it not the case.

qwerdee commented 4 years ago

@distantnative Sorry for getting back to this issue that late. I just checked whether the issue still persists in Kirby 3.3.4 and it does (as @rzschoch kind of reopened it in #2467 )

The issue is not caused by misconfigured server, the issue of @SinisaG is not related to this.

Here a little test case:

This generates the following:

<!-- 1. -->
<img src="http://starterkit.test/media/assets/assets/1353177920-1583233049/cheesy-autumn.jpg">
<!-- 2. -->
<img src="">
<!-- 3. -->
<img src="http://starterkit.test/media/assets/assets/1353177920-1583233049/cheesy-autumn-150x.jpg">
<!-- 4. -->
<img src="http://starterkit.test/assets/cheesy-autumn.jpg">
<!-- 5. -->
<img src="http://starterkit.test/media/assets/assets/1353177920-1583233049/cheesy-autumn-200x.jpg">

And in the media folder

Bildschirmfoto 2020-03-03 um 12 25 31

I think for consistency it would be best to make sure, the asset is available in the media folder when the mediaUrl() is requested. It is very likely that one uses this url then.

It is also interesting that case 2 does create a resize job but mediaUrl() is empty and the actual media url is accessible via url(). In this case, both methods should return the same mediaUrl.

  1. & 5. works as expected
qwerdee commented 4 years ago

Oh and one weird thing, why are there two nested assets folders?

.../media/assets/assets/1353177920-1583233049/...

distantnative commented 3 years ago

Closing this for the newer https://github.com/getkirby/kirby/issues/2467 with current discussion

distantnative commented 3 years ago

@qwerdee Finally being able to tackle this issue, we have been wondering, why you are using ->mediaUrl() in some of the examples and not ->url(). Any particular reason?

Oh and one weird thing, why are there two nested assets folders?

media/assets is the media path folder for everything generated via the assets() helper. What follows is the path to the file you passed to the helper - since that one also includes assets, you end up with that double assets path segment.

qwerdee commented 3 years ago

Hi @distantnative, nice to see this tackled

Yes there was a reason, I wanted to be able to safely apply cache headers to our assets by taking advantage of the hash segment in the -> mediaUrl() for correct invalidation.