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

Better rules for Nginx #166

Closed
rosell-dk opened this issue Feb 11, 2019 · 10 comments
Closed

Better rules for Nginx #166

rosell-dk opened this issue Feb 11, 2019 · 10 comments
Assignees

Comments

@rosell-dk
Copy link
Owner

rosell-dk commented Feb 11, 2019

We should provide better rules for Nginx.
The rules should redirect directly to existing webp files, and set Vary: Accept header when it does

Current rule is simply:

if ($http_accept ~* "webp"){
  rewrite ^/(.*).(jpe?g|png)$ /wp-content/plugins/webp-express/wod/webp-on-demand.php?wp-content=wp-content break;
}
@rosell-dk rosell-dk self-assigned this Feb 11, 2019
@rosell-dk
Copy link
Owner Author

rosell-dk commented Feb 11, 2019

After a lot of tweaking, I have come up with a solution.
The code must be inserted in the server context.
In WebP Express settings, you must choose Destination folder: mingled and File extension: Append ".webp"

    # WebP Express rules
    # --------------------
    location ~* ^/wp-content/.*\.(png|jpe?g)$ {
        add_header Vary Accept;
        expires 365d;
    }
    location ~* ^/wp-content/.*\.webp$ {
        expires 365d;
        if ($whattodo = AB) {
            add_header Vary Accept;
        }
    }
    if ($http_accept ~* "webp"){
        set $whattodo A;
    }
    if (-f $request_filename.webp) {
        set $whattodo  "${whattodo}B";
    }
    if ($whattodo = AB) {
        rewrite ^(.*) $1.webp last;
    }
    if ($whattodo = A) {
        rewrite ^/wp-content/.*\.(jpe?g|png)$ /wp-content/plugins/webp-express/wod/webp-on-demand.php?xsource=x$request_filename&wp-content=wp-content break;
    }
    # ------------------- (WebP Express rules ends here)

Beware when copy/pasting: You might get html-encoded characters. Verify that the ampersand before "wp-content" isn't encoded

I'm using this hack for checking multiple if conditions
The "last" flag on the first rewrite causes a new location lookup.

The "expires 365d;" lines set caching to one year. You can remove these lines if you wish.
I have not set any expire on the webp-on-demand.php request. This is not needed, as the script sets this according to what you set up in WebP Express settings. Also, trying to do it would require a new location block matching webp-on-demand.php, but that would override the location block handling php files, and thus break the functionality.

@rosell-dk
Copy link
Owner Author

rosell-dk commented Feb 12, 2019

Here is another way to achieve the same:

In server context insert this:

# WebP Express rules
# --------------------
location ~* ^/?wp-content/.*\.(png|jpe?g)$ {
  add_header Vary Accept;
  # expires 365d;
  if ($http_accept !~* "webp"){
    break;
  }
  try_files
    $uri.webp
    /wp-content/plugins/webp-express/wod/webp-on-demand.php?xsource=x$request_filename&wp-content=wp-content
    ;
}
# ------------------- (WebP Express rules ends here)

Beware when copy/pasting: You might get html-encoded characters. Verify that the ampersand before "wp-content" isn't encoded

Again, in WebP Express settings, you must choose Destination folder: mingled and File extension: Append ".webp"

The solution is based on this recipe. But I have added the converter script in the try_files. Also, in order for the php to be processed, I have removed "$uri =404;" from try_files (got that solution here)

If you only want the rules to apply to the uploads folder, change the first line to:
location ~* ^/?wp-content/uploads/.*\.(png|jpe?g)$ {

Beware that if you haven't enabled png conversion, you should replace "(png|jpe?g)" with "jpe?g".

@rosell-dk
Copy link
Owner Author

rosell-dk commented Feb 13, 2019

Here is a third solution.
It contains if directives inside a location statement, which is "evil".
It however seems to work alright.
The solution is basically a modification of the first, but wrapped inside a location. I got to know that this was possible when viewing the rules presented in this blog post on keycdn)

    # WebP Express rules
    # --------------------
    location ~* ^/wp-content/.*\.(png|jpe?g)$ {
        add_header Vary Accept;
        expires 365d;

        if ($http_accept ~* "webp"){
            set $whattodo A;
        }
        if (-f $request_filename.webp) {
            set $whattodo  "${whattodo}B";
        }
        if ($whattodo = AB) {
            rewrite ^(.*) $1.webp last;
        }
        if ($whattodo = A) {
            rewrite ^/wp-content/.*\.(jpe?g|png)$ /wp-content/plugins/webp-express/wod/webp-on-demand.php?xsource=x$request_filename&wp-content=wp-content last;
        }
    }

    location ~* ^/wp-content/.*\.webp$ {
        expires 365d;
        if ($whattodo = AB) {
            add_header Vary Accept;
        }
    }
    # ------------------- (WebP Express rules ends here)

For reference, the rules presented in the blog post by keycdn looks like this:

location ~ ^(/path/to/your/images.+)\.(jpe?g|png)$ {
    if ( $http_accept ~* webp ) {
        set $webp "A";
    }
    if ( $request_filename ~ (.+)\.(png|jpe?g)$ ) {
        set $file_without_ext $1;
    }
    if ( -f $file_without_ext.webp ) {
        set $webp "${webp}E";
    }

    if ( $webp = AE ) {
        add_header Vary Accept;
        rewrite ^(.+)\.(png|jpe?g)$ $1.webp break;
    }
}

@rosell-dk
Copy link
Owner Author

Here are rules for routing non-existing webp requests to converter:

location ~* ^/?wp-content/.*\.(png|jpe?g)\.webp$ {
    try_files
      $uri
      /wp-content/plugins/webp-express/wod/webp-realizer.php?wp-content=wp-content
      ;
}

I'm getting the hang of it :)

@koekaverna
Copy link

Your config always run php. $uri.webp doesn't exist, because plugin store webp images in different than original image folder.
So I added the path to webp images directory to argument

# WebP Express rules
# --------------------
location ~* ^/?wp-content/.*\.(png|jpe?g)$ {
  add_header Vary Accept;
  # expires 365d;
  if ($http_accept !~* "webp"){
    break;
  }
  try_files
    /wp-content/webp-express/webp-images/doc-root/$uri.webp
    /wp-content/plugins/webp-express/wod/webp-on-demand.php?xsource=x$request_filename&wp-content=wp-content
    ;
}
# ------------------- (WebP Express rules ends here)

PS. I checked it with
try_files { $uri.webp $uri.webp;} got 404
and
try_files { /wp-content/webp-express/webp-images/doc-root/$uri.webp /wp-content/webp-express/webp-images/doc-root/$uri.webp ; } got image

@rosell-dk
Copy link
Owner Author

@koekaverna: You can configure the plugin to store images in the same folder as the originals on the plugin settings screen. However, images outside of the upload folder (ie theme images) are always stored in a separate folder so your setup is better. Thanks

@rosell-dk
Copy link
Owner Author

rosell-dk commented Aug 1, 2019

When destination is set to "mingled", your modified line is needed in addition to the original (for theme images etc).

So that solution becomes:

# WebP Express rules
# --------------------
location ~* ^/?wp-content/.*\.(png|jpe?g)$ {
  add_header Vary Accept;
  # expires 365d;
  if ($http_accept !~* "webp"){
    break;
  }
  try_files
    /wp-content/webp-express/webp-images/doc-root/$uri.webp
    $uri.webp
    /wp-content/plugins/webp-express/wod/webp-on-demand.php?xsource=x$request_filename&wp-content=wp-content
    ;
}
# ------------------- (WebP Express rules ends here)

When configured to store in separate folder, you can remove the line containing "$uri.webp"

rosell-dk added a commit that referenced this issue Aug 1, 2019
@mbt1370
Copy link

mbt1370 commented Nov 14, 2019

i tried above all things but still it shows error for Nginx.

>Bummer. As the "content-type" header reveals, we got the jpeg.
It seems we can conclude that the rewrite did not happen.
In addition, we did not get a content-length header either.It seems we can conclude that the rewrite did not happen.
The test FAILED.

@mbt1370
Copy link

mbt1370 commented Nov 14, 2019

problem is solved but it not generate for lazyload images.

@formido
Copy link

formido commented Feb 12, 2021

problem is solved but it not generate for lazyload images.

How was the problem solved?

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

4 participants