Ipstenu / varnish-http-purge

Proxy Cache Purge
Apache License 2.0
46 stars 48 forks source link

VCL optimizations #93

Closed ThijsFeryn closed 11 months ago

ThijsFeryn commented 3 years ago

Hi @Ipstenu,

I'm Thijs, I'm the technical evangelist at Varnish Software, and I'm looking to build a VCL that works for WordPress and that is kept up-to-date by us.

Your sample VCL files on https://github.com/Ipstenu/varnish-http-purge/wiki/Sample-VCLs look like a solid foundation, but I'd like to introduce some optimizations that would heavily reduce the line count.

But before I present potential changes, I'd like to hear your opinion on WordPress cookies.

I have found the following regex pattern in the main VCL file:

wordpress_(?!test_)[a-zA-Z0-9_]+|wp-postpass|comment_author_[a-zA-Z0-9_]+|woocommerce_cart_hash|woocommerce_items_in_cart|wp_woocommerce_session_[a-zA-Z0-9]+|wordpress_logged_in_|comment_author|PHPSESSID

I was wondering if this pattern captures all relevant cookies in WordPress. Instead of removing individual cookies, I'd like to suggest a more aggressive approach: removing the Cookie header entirely if we get passed the if-statement that checks this pattern

This is the if-statement that is currently used:

    if (
        req.http.Authorization ||
        req.http.Authenticate ||
        req.http.X-Logged-In == "True" ||
        req.http.Cookie ~ "wordpress_(?!test_)[a-zA-Z0-9_]+|wp-postpass|comment_author_[a-zA-Z0-9_]+|woocommerce_cart_hash|woocommerce_items_in_cart|wp_woocommerce_session_[a-zA-Z0-9]+|wordpress_logged_in_|comment_author|PHPSESSID"
    ) {
        return(pass);
    }

If the request gets passed this conditional, I'm assuming cookies are no longer relevant to the page. As a consequence I would replace the regsuball() calls that replace individual cookies with unset req.http.Cookie;.

Please advise on the relevance of cookies when the request gets passed the cookie check in the VCL file.

Thanks Thijs

Ipstenu commented 3 years ago

FWIW part of why it's broken up like that is we actually deploy it with code that means people get slightly different versions :)

I was wondering if this pattern captures all relevant cookies in WordPress. Instead of removing individual cookies, I'd like to suggest a more aggressive approach: removing the Cookie header entirely if we get passed the if-statement that checks this pattern

That's certainly an option. That's what another host does, IIRC. We opted NOT to, since it also removed cookies used by other plugins to intentionally, optionally, not-cache.

Basically you get two main schools of thought:

  1. Remove all cookies except the good ones
  2. Allow cookies to override

We went with option 2 to be more flexible and so we didn't have to update things all the time.

FWIW the structure we use is now totally different, account for readability and management, which is why I just leave this here as a kind of starter :)

ThijsFeryn commented 3 years ago

Regardless of the cookie situation, there's a lot of code in there you don't need. Although some items are debatable, you definitely don't need all that logic in your vcl_backend_response subroutine.

As soon as you perform a return(pass) in vcl_recv, the corresponding HTTP responses will never end up in cache. So all that logic where you do set beresp.uncacheable = true; will never be necessary.

I'd also like to talk about the following if-statement, which brings us back to the cookie debate:

if(req.http.X-Logged-In == "False" && req.method != "POST") {
    unset req.http.Cookie;
}

In a literal sense, the X-Logged-In header really needs to be False in order for this conditional to evaluate to true. The POST part is irrelevant, because all HTTP POST calls have received a "pass" via the following conditional:

if (req.method != "GET" && req.method != "HEAD") {
    return(pass);
}

But if we look at the purpose of this code, it will remove all cookies for users that are not logged in.

Earlier in the VCL file, there was already a check for cookies that match wordpress_logged_in_*. So in a way, logged in users have already received a "pass".

Is it fair to assume that my suggested logic on unconditionally removing all cookies passed the cookie & URL checks has exactly the same effect?

ThijsFeryn commented 3 years ago

By the way, this is my attempt at simplifying your VCL files: https://gist.github.com/ThijsFeryn/26efa7c62190c9187be240e2f23da493

I understand that you have specific needs which justify a lot of the extra code.

My goal is to have an "official" WordPress VCL that is supported by us (Varnish Software). It should be simple enough and work for the majority of the user base.

I would appreciate some feedback, and maybe also some input from other people in the WordPress community.

Ipstenu commented 3 years ago

I think it has the same effect. I'm actually a relative newbie (this is the first VCL I sat down and built -- and a lot was based on work my predecessor had done for Varnish 4, which was ... sketchy at best).

Our current setup has:

  1. A main VCL
  2. Separate VCLs for cloudflare, xforward, purge, bigfiles_pipe, static, woocommerce, edd, and other plugins

I left the content in the backend response because we weren't getting proper cache results when I pulled it out. I don't know if that's due to using an NGINX proxy for SSL or not. TBH I'm loathe to mess with it since we got it working so it doesn't cache things it's not supposed to.

eta: I have this vague memory that we wanted to not cache certain pages, logged in or not, because of how WordPress pulls some ajaxy code from wp-admin, and this stemmed from the need to allow more react based content.

Right now, our UTM stuff looks like this:

    if (req.url ~ "(\?|&)(utm_[^=]|gclid|cx|ie|cof|siteurl)=") {
        set req.url = regsuball(req.url, "&(utm_[^=]|gclid|cx|ie|cof|siteurl)=([A-z0-9_\-\.%25]+)", "");
        set req.url = regsuball(req.url, "\?(utm_[^=]|gclid|cx|ie|cof|siteurl)=([A-z0-9_\-\.%25]+)", "?");
        set req.url = regsub(req.url, "\?&", "?");
        set req.url = regsub(req.url, "\?$", "");
    }

Since we learned Google likes to add weird things.

But. If you want to email me directly (mika.epstein @ dreamhost.com) I can send you the complex VCL templates we're using in full.

dgaastra commented 3 years ago

Hi Thijs,

This is great what you have done with your Wordpress VCL. We are trying it now on our production servers. We have to ask you for one favour since the config is still quite long and complex. Would it be possible to [1] comment each part, and [2], explain what the impact would be of removing such a part. In that way, one could eliminate sections that might not be applicable. Thanks so much. Dennis.

ThijsFeryn commented 3 years ago

Hi Thijs,

This is great what you have done with your Wordpress VCL. We are trying it now on our production servers. We have to ask you for one favour since the config is still quite long and complex. Would it be possible to [1] comment each part, and [2], explain what the impact would be of removing such a part. In that way, one could eliminate sections that might not be applicable. Thanks so much. Dennis.

@dgaastra Sure, I'll try to handle it next week.

ThijsFeryn commented 3 years ago

@dgaastra Here's the official tutorial containing commented VCL code https://www.varnish-software.com/developers/tutorials/configuring-varnish-wordpress/

dgaastra commented 3 years ago

HI Thijs,

Thanks, that looks excellent. Did you just publish this?

This tutorial looks much more detailed and correct (and up-to-date?) than:

https://www.varnish-software.com/wiki/content/tutorials/wordpress/wp_step_by_step.html

where port 80 is still mentioned for the backend.

ThijsFeryn commented 3 years ago

HI Thijs,

Thanks, that looks excellent. Did you just publish this?

This tutorial looks much more detailed and correct (and up-to-date?) than:

https://www.varnish-software.com/wiki/content/tutorials/wordpress/wp_step_by_step.html

where port 80 is still mentioned for the backend.

Indeed, this is a new Developer Portal that contains up-to-date content. See https://info.varnish-software.com/blog/varnish-developer-portal for the announcement.

As a matter of fact, all the content on the wiki you're referring to will be moved and revised. Once every article has been overhauled, we will discontinue the wiki and redirect every one of its pages to the relevant content on the DevPortal.

dgaastra commented 3 years ago

That Portal looks very impressive. Just one little thing: we needed to change "vcl 4.0;" to "vcl 4.1;" to avoid a start-up-error. We are using Debian 11 with mostly Debian released standard packages.

dgaastra commented 3 years ago

Another question: before we used your configuration, we needed to add the following snippet in oder to forward the client's IP Addresses to Apache. Now this is not necessary anymore. Could you explain why? Thanks. Dennis


    # @REF: https://www.varnish-software.com/wiki/content/tutorials/varnish/sample_vclTemplate.html
    if (req.restarts == 0) {
        if (req.http.X-Forwarded-For) {
           set req.http.X-Forwarded-For = req.http.X-Forwarded-For + ", " + client.ip;
       } else {
        set req.http.X-Forwarded-For = client.ip;
       }
    }
ThijsFeryn commented 3 years ago

That Portal looks very impressive. Just one little thing: we needed to change "vcl 4.0;" to "vcl 4.1;" to avoid a start-up-error. We are using Debian 11 with mostly Debian realised standard packages.

VCL syntax 4.1 is only needed if you're using Varnish 6. However since we want to encourage people to install Varnish Cache 6.0 LTS it's actually a good idea to include vcl 4.1; in all our code examples.

I've just pushed these changes and you'll now see vcl 4.1; instead of vcl 4.0; on the developer portal.

ThijsFeryn commented 3 years ago

Another question: before we used your configuration, we needed to add the following snippet in oder to forward the client's IP Addresses to Apache. Now this is not necessary anymore. Could you explain why? Thanks. Dennis


    # @REF: https://www.varnish-software.com/wiki/content/tutorials/varnish/sample_vclTemplate.html
    if (req.restarts == 0) {
        if (req.http.X-Forwarded-For) {
           set req.http.X-Forwarded-For = req.http.X-Forwarded-For + ", " + client.ip;
       } else {
        set req.http.X-Forwarded-For = client.ip;
       }
    }

Varnish now sets the X-Forwarded-For header automatically. If no X-Forwarded-For header was received, Varnish will add one containing the value of client.ip. If there is already an X-Forwarded-For header, Varnish will attach the value of client.ip to the existing header.

The VCL code you pasted is no longer required as Varnish handles this automatically.

dgaastra commented 3 years ago

Excellent. Thanks so much.

dgaastra commented 3 years ago

Sorry, one more thing, as we are struggling with a warning in this Wordpress plugin. It needs "X-Cacheable" to be set in the header in oder to prevent a "Varnish caching service is running but is unable to cache your site." warning. Currently, we do not have any "X-Cacheable" in our headers for our webpages. Is there any way to bring it in?

ThijsFeryn commented 3 years ago

Sorry, one more thing, as we are struggling with a warning in this Wordpress plugin. It needs "X-Cacheable" to be set in the header in oder to prevent a "Varnish caching service is running but is unable to cache your site." warning. Currently, we do not have any "X-Cacheable" in our headers for our webpages. Is there any way to bring it in?

Don't worry too much about the X-Cacheable header: it's mostly cosmetic and is set in https://github.com/Ipstenu/varnish-http-purge/wiki/default.vcl.

I find this process to be a bit too verbose for my taste, so I came up with this VCL code:

sub vcl_recv {
    set req.http.X-Cacheable = "YES";
}

sub vcl_pass {
    set req.http.X-Cacheable = "NO:PASS";
}

sub vcl_deliver {
    if(req.is_hitmiss) {
        set resp.http.X-Cacheable = "NO:HITMISS";
    } elseif(req.is_hitpass) {
        set resp.http.X-Cacheable = "NO:HITPASS";       
    } else {
        set resp.http.X-Cacheable = req.http.X-Cacheable;
    }
}

It's minimal, but does the job:

Let me know if that works for you.

dgaastra commented 3 years ago

Hi Thijs,

Thanks for that great and well documented snippet. The functionality seems to make sense.

However; I don't quite have the understanding about which components in the system would benefit from it, except for removing Mika's plugin's warning. Is it really cosmetic to make this plugin happy or does this extra code actually provide any other benefits? We also run Otto v/d Schaaf's mod_pagespeed in combination with varnish/hitch.

Quetion: How would one include this extra code in your master vcl from your official tutorial? Or rather not?

Would it rather make more sense for Mika to fix the code in his plugin?

Greetings Dennis

ThijsFeryn commented 3 years ago

@Ipstenu Mika what's your take on this? Based on a search query I only see X-Cacheable appear in the debug.php. Would it make sense to update the code and not return a warning when the X-Cacheable header is not set?

Ipstenu commented 3 years ago

Sorry about my delay, it was Yom Kippur and I was offline (y'know, lives and all that).

@ThijsFeryn - Yeah the checks we have are super verbose but I needed them to be readable by a number of people far less familiar with things, so I leaned towards too-verbose.

Would it make sense to update the code and not return a warning when the X-Cacheable header is not set?

No, since it's part of a larger check that ties into how the majority of WordPress hosts set up Varnish. That's why it's a notice, though, and says "We were unable find a caching service active for this domain. This may occur if you use a proxy service (such as CloudFlare or Sucuri) or if you\'re in the middle of a DNS move."

There are different levels of alerts for things specifically to cover those cases. I would rather make the language clearer there, and indicate 'or you did not add the X-Cacheable header...', since it's incredibly helpful for hosts when debugging weird stuff users do.

@dgaastra Just FYI, WPML does a lot of weird stuff that plays poorly with caching in my experience. It's been a minute since I tested it on Varnish, though.

ThijsFeryn commented 3 years ago

@Ipstenu @dgaastra Updated the VCL code in my tutorial and added support for the X-Cacheable header. Tested it on a WordPress setup. Seems to work fine.

See https://www.varnish-software.com/developers/tutorials/configuring-varnish-wordpress/.

At some point I'd like to hear if this VCL is good enough to be considered "the official VCL file for WordPress, compatible with varnish-http-purge"

Eventually this ticket will have to be closed, either with a VCL fix or with a decision that the standard VCL for this module shouldn't change.

Ipstenu commented 3 years ago

I'm place holding replying since I'm in and out and in and out but ... I think that looks good. I'm going to do a full test hopefully Thursday 🤞🏻

dgaastra commented 3 years ago

Hi Thijs and Mika,

Thanks for your comments and updating the tutorial. Super nice work! Like Mika, we will also test it over the next couple of days on our production servers. Greetings, Dennis

dgaastra commented 3 years ago

Hi Thijs and Mika,

Tested the new configuration. AFIK, we only have have 2 issuess left: (The WPML Problem was a mod_pagespeed problem+fix and unrelated to Varnish/Hitch)

[1] We use mod_pagespeed in combination with Varnish/Hitch, and the tutorial's code does not produce 'X-Cacheable' set to 'YES:Forced' ? Can this be added to the tutorial ... or is this resolved with a ModPageSpeed parameter?

Below the code snippet from Mika's Plugin:


        // Mod-PageSpeed.
        if ( isset( $headers['X-Mod-Pagespeed'] ) ) {
            if ( strpos( $headers['X-Cacheable'], 'YES:Forced' ) !== false ) {
                $return['Mod Pagespeed'] = array(
                    'icon'    => 'good',
                    'message' => __( 'Mod Pagespeed is active and configured to work properly with caching services.', 'varnish-http-purge' ),
                );
            } else {
                $return['Mod Pagespeed'] = array(
                    'icon'    => 'bad',
                    'message' => __( 'Mod Pagespeed is active but your caching headers may not be correct. This may be a false negative if other parts of your site are overwriting headers. Fix all other errors listed, then come back to this. If you are still having errors, you will need to look into using .htaccess or Nginx to override the Pagespeed headers.', 'varnish-http-purge' ),
                );
            }
        }

[2] The purge logic from the tutorial:


    # Purge logic to remove objects from the cache. 
    # Tailored to the Proxy Cache Purge WordPress plugin
    # See https://wordpress.org/plugins/varnish-http-purge/
    if(req.method == "PURGE") {
        if(!client.ip ~ purge) {
            return(synth(405,"PURGE not allowed for this IP address"));

            if (req.http.X-Purge-Method == "regex") {
                ban("obj.http.x-url ~ " + req.url + " && obj.http.x-host ~ " + req.http.host);
                return(synth(200, "Purged"));
            }
            return (purge);
        }
    }

Shouldn't there be a closing bracket after issuing the 405 error? It appears the regex and return (purge) statements are at the same level after the IP error...and maybe not reachable. Then again, I am not confident with VCLs to be sure here.

Ipstenu commented 3 years ago

I meant to reply on Friday and I think I need a TARDIS to keep up with life.

The VCL worked in my tests and is a great 'base Level' -- I didn't test against Mod_pagespeed since we no longer use it at work, but IIRC we had a special rule we added in for it to case X:Cachable mostly to ... er lie to Varnish that no, really, this was cached, chill out.

I see what you mean about the 405...

    if(req.method == "PURGE") {
        if(!client.ip ~ purge) {
            return(synth(405,"PURGE not allowed for this IP address"));

At this point, you have the return so it won't do the rest (see below)

            if (req.http.X-Purge-Method == "regex") {
                ban("obj.http.x-url ~ " + req.url + " && obj.http.x-host ~ " + req.http.host);
                return(synth(200, "Purged"));
            }
            return (purge);
        }
    }

So should it be this?

    if(req.method == "PURGE") {
        if(!client.ip ~ purge) {
            return(synth(405,"PURGE not allowed for this IP address"));
        }

        if (req.http.X-Purge-Method == "regex") {
                ban("obj.http.x-url ~ " + req.url + " && obj.http.x-host ~ " + req.http.host);
                return(synth(200, "Purged"));
        }
        return (purge);
    }
dgaastra commented 3 years ago

@thijsferyn @Ipstenu: I agree with the solution from Mika as we have the same fix in place. (Thijs; waar ben jij?) @Ipstenu: Maybe, you could then get rid of that Modpagespeed error/code snippet (pasted before).

Thanks Dennis

thijs commented 3 years ago

Je hebt de verkeerde @ gebruikt. Ik ben niet de thijs die je zoekt.

On Tue, Sep 28, 2021 at 1:03 PM dgaastra @.***> wrote:

@thijs https://github.com/thijs @Ipstenu https://github.com/Ipstenu: I agree with the solution from Mika as we have the same fix in place. (Thijs; waar ben jij?) @Ipstenu https://github.com/Ipstenu: Maybe, you could then get rid of that Modpagespeed error/code snippet (pasted before).

Thanks Dennis

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Ipstenu/varnish-http-purge/issues/93#issuecomment-929084010, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAANVVOLG6ZZZOOAJJ4DC3UEGOIRANCNFSM474ZIXGQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

ThijsFeryn commented 3 years ago

@Ipstenu @dgaastra Meanwhile I fixed the VCL. Have a look: https://www.varnish-software.com/developers/tutorials/configuring-varnish-wordpress/#the-wordpress-vcl-file

I fixed the PURGE issues and made the X-Cacheable header more compatible.

Hope this does the job.

dgaastra commented 2 years ago

@ThijsFeryn: Thanks; we'll try it soon. Best greetings Dennis

dgaastra commented 2 years ago

@ThijsFeryn @Ipstenu: Uploaded new config to our production servers. Mika's Plugin's Mod Pagespeed error is still there with a red thumb down. What could be the solution to get rid of this error?

Ipstenu commented 2 years ago

Honestly this is a place where I'd say it's something you should add to your own VCL rules, but not the default.

If you wanted to just edit things, then on your VCL copy of https://www.varnish-software.com/developers/tutorials/configuring-varnish-wordpress/ on line 187 change it to FORCED... but I thought (and it's been a while, so sorry for the lack of freshness with Pagespeed) that you had to write a downstream handler for this to work, which IIRC was a custom VCL, so you'd put it in there.

https://www.modpagespeed.com/doc/downstream-caching seems to suggest it's still needed, so that would be where to put it in.

dgaastra commented 2 years ago

Thanks. Yes, looked at that article, but it turns out, we only need: set req.http.PS-CapabilityList = "fully general optimizations only"; in sub vcl_recv Changing X-Cacheable it to forced in line 184 makes it forced everywhere. Is that the right approach? @Ipstenu : What do you use instead of mod_pagespeed now?

dgaastra commented 2 years ago

@ThijsFeryn @Ipstenu:

[i] The varnish log responds with "HTTP/2.0" but the Apache2 log only with "HTTP/1.1" even though "Protocols h2 h2c http/1.1" is set in Apache2's VHosts. Is this because SSL is hitched-off by hitch?

ThijsFeryn commented 2 years ago

@dgaastra don't forget that this issue and the thread of comments is about optimizing the WordPress VCL. What you're asking now is out of scope.

Send me an e-mail for other Varnish-related issues: thijs.feryn@varnish-software.com

Ipstenu commented 2 years ago

What do you use instead of mod_pagespeed now?

Nothing, actually. We found that blanket minification resulted in more support than it was worth, plus it was regularly causing server instability. And since Google moved away from it being helpful and more to TTFB as their metrics for speed, it was making users angry that using it didn't magically help them.

If someone needs to minimize, we recommend plugins (WP Rocket is my personal fave, for simplicity and general nice humans behind it, and I use it myself on some sites), but once we got varnish working well, there was nearly no speed improvements with pagespeed, so it was an easy thing to remove and simplify the stack.

shagr4th commented 2 years ago

Not sure if I did something wrong, but I have to add a ban to make it work, when modifying an article ("X-Purge-Method" is at "default", and not "regex" in my varnishlog). Line 50:

ban("obj.http.x-url == " + req.url + " && obj.http.x-host == " + req.http.host);
return (purge); 
shagr4th commented 2 years ago

Scrap my previous comment, I understood why the purge wasn't working. The VCL sample on the varnish blog is not working behind a secured reverse proxy. The cache entry is hashed with the request protocol (line 141), so https in my case, but the PURGE sent by the wordpress plugin is called with an http protocol. Result : Varnish responds ok, this is purged, but the object hash was wrongly calculated, so nothing is really removed.

I don't know if there's a generic solution here. Either remove the vcl_hash subroutine, which creates variations depending on the request protocol, or force every PURGE request to https if you are behind a secured reverse proxy, or ban directly the content like I did in my previous comment

ThijsFeryn commented 2 years ago

@shagr4th Thanks for reporting this issue, it is very relevant.

Your suggestion to use ban("obj.http.x-url == " + req.url + " && obj.http.x-host == " + req.http.host); seems like a good way to tackle the issue.

This would result in the following VCL:

    if(req.method == "PURGE") {
        if(!client.ip ~ purge) {
            return(synth(405,"PURGE not allowed for this IP address"));
        }
        if (req.http.X-Purge-Method == "regex") {
            ban("obj.http.x-url ~ " + req.url + " && obj.http.x-host == " + req.http.host);
            return(synth(200, "Purged"));
        }
        ban("obj.http.x-url == " + req.url + " && obj.http.x-host == " + req.http.host);
        return(synth(200, "Purged"));
    }

I will also make the hash_data(req.http.X-Forwarded-Proto); in vcl_hash conditional, which results in the following VCL:

sub vcl_hash {
    if(req.http.X-Forwarded-Proto) {
        # Create cache variations depending on the request protocol
        hash_data(req.http.X-Forwarded-Proto);
    }
}

Does this make sense to you?

dgaastra commented 2 years ago

Hi Thijs

However, if you only use varnish behind hitch, and have blocked port 80 altogether, is this still necessary?

Thanks so much.

Dennis

On 9. Nov 2021, at 08:36, Thijs Feryn @.***> wrote:

@shagr4th https://github.com/shagr4th Thanks for reporting this issue, it is very relevant.

Your suggestion to use ban("obj.http.x-url == " + req.url + " && obj.http.x-host == " + req.http.host); seems like a good way to tackle the issue.

This would result in the following VCL:

if(req.method == "PURGE") {
    if(!client.ip ~ purge) {
        return(synth(405,"PURGE not allowed for this IP address"));
    }
    if (req.http.X-Purge-Method == "regex") {
        ban("obj.http.x-url ~ " + req.url + " && obj.http.x-host == " + req.http.host);
        return(synth(200, "Purged"));
    }
    ban("obj.http.x-url == " + req.url + " && obj.http.x-host == " + req.http.host);
    return(synth(200, "Purged"));
}

I will also make the hash_data(req.http.X-Forwarded-Proto); in vcl_hash conditional, which results in the following VCL:

sub vcl_hash { if(req.http.X-Forwarded-Proto) {

Create cache variations depending on the request protocol

    hash_data(req.http.X-Forwarded-Proto);
}

} Does this make sense to you?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Ipstenu/varnish-http-purge/issues/93#issuecomment-963886814, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASFZUQ3GB37EQXY7SYIK33ULDFRLANCNFSM474ZIXGQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

ThijsFeryn commented 2 years ago

Hi Thijs However, if you only use varnish behind hitch, and have blocked port 80 altogether, is this still necessary? Thanks so much. Dennis On 9. Nov 2021, at 08:36, Thijs Feryn @.***> wrote: @shagr4th https://github.com/shagr4th Thanks for reporting this issue, it is very relevant. Your suggestion to use ban("obj.http.x-url == " + req.url + " && obj.http.x-host == " + req.http.host); seems like a good way to tackle the issue. This would result in the following VCL: if(req.method == "PURGE") { if(!client.ip ~ purge) { return(synth(405,"PURGE not allowed for this IP address")); } if (req.http.X-Purge-Method == "regex") { ban("obj.http.x-url ~ " + req.url + " && obj.http.x-host == " + req.http.host); return(synth(200, "Purged")); } ban("obj.http.x-url == " + req.url + " && obj.http.x-host == " + req.http.host); return(synth(200, "Purged")); } I will also make the hash_data(req.http.X-Forwarded-Proto); in vcl_hash conditional, which results in the following VCL: sub vcl_hash { if(req.http.X-Forwarded-Proto) { # Create cache variations depending on the request protocol hash_data(req.http.X-Forwarded-Proto); } } Does this make sense to you? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#93 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASFZUQ3GB37EQXY7SYIK33ULDFRLANCNFSM474ZIXGQ. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

It's not so much about any specific use case, my goal is to provide a VCL file that works in any situation. While it might not be necessary in your case, the logic will not get in your way.

shagr4th commented 2 years ago

Hi Thijs, thanks for your reply, yes I think it's the most sensible way (caching needs to differentiate between http and https, imho). I run a small instance so maybe heavy users will chime in ;p