Ema has submitted this change and it was merged. Change subject: Text VCL forward-port to Varnish 4 ......................................................................
Text VCL forward-port to Varnish 4 Bug: T131503 Change-Id: I6789d1129aee77f5eb8b7816afeaac2c1a7a1a41 --- M modules/varnish/templates/geoip.inc.vcl.erb M modules/varnish/templates/initscripts/varnish.systemd.erb M modules/varnish/templates/normalize_path.inc.vcl.erb M modules/varnish/templates/text-backend.inc.vcl.erb M modules/varnish/templates/text-common.inc.vcl.erb M modules/varnish/templates/text-frontend.inc.vcl.erb 6 files changed, 145 insertions(+), 33 deletions(-) Approvals: Ema: Looks good to me, approved BBlack: Looks good to me, but someone else must approve jenkins-bot: Verified diff --git a/modules/varnish/templates/geoip.inc.vcl.erb b/modules/varnish/templates/geoip.inc.vcl.erb index c8d97f2..72c7914 100644 --- a/modules/varnish/templates/geoip.inc.vcl.erb +++ b/modules/varnish/templates/geoip.inc.vcl.erb @@ -54,8 +54,14 @@ * return value is the zero for success, non-zero for failure (input IP is * invalid or the encoding of it is too long). */ +<% if @varnish_version4 -%> + static int geo_get_xcip(const struct vrt_ctx *ctx, char* ip) { + const struct gethdr_s hdr = { HDR_REQ, "\014X-Client-IP:" }; + const char* client_ip = VRT_GetHdr(ctx, &hdr); +<% else -%> static int geo_get_xcip(const struct sess* sp, char* ip) { const char* client_ip = VRT_GetHdr(sp, HDR_REQ, "\014X-Client-IP:"); +<% end -%> size_t len = 0; int rv = -1; @@ -141,13 +147,22 @@ _GEO_IDX_SIZE = 5, } geo_idx_t; +<% if @varnish_version4 -%> + static void geo_out_cookie(const struct vrt_ctx *ctx, char** geo) { +<% else -%> static void geo_out_cookie(struct sess* sp, char** geo) { +<% end -%> char host_safe[50]; // We can't set a cookie if we don't know the valid top domain, so this // is the case where we emit no Cookie output at all (as before) with // the two possible bare "return;" below +<% if @varnish_version4 -%> + const struct gethdr_s hdr = { HDR_REQ, "\005host:" }; + const char* host = VRT_GetHdr(ctx, &hdr); +<% else -%> const char* host = VRT_GetHdr(sp, HDR_REQ, "\005host:"); +<% end -%> if (!host) return; const char* top_dom = geo_get_top_cookie_domain(host); @@ -173,9 +188,16 @@ // Use libvmod-header to ensure the Set-Cookie header we are adding // does not clobber or manipulate existing cookie headers (if any). +<% if @varnish_version4 -%> + const struct gethdr_s hdr_set_cookie = { HDR_RESP, "\013Set-Cookie:" }; + Vmod_header_Func.append(ctx, &hdr_set_cookie, + out, "; Path=/; secure; Domain=.", + host_safe, vrt_magic_string_end); +<% else -%> Vmod_Func_header.append(sp, HDR_RESP, "\013Set-Cookie:", out, "; Path=/; secure; Domain=.", host_safe, vrt_magic_string_end); +<% end -%> } static const char* mm_path[_GEO_IDX_SIZE][4] = { @@ -186,7 +208,11 @@ {"location", "longitude", NULL, NULL}, }; +<% if @varnish_version4 -%> + static void geo_xcip_output(const struct vrt_ctx *ctx) { +<% else -%> static void geo_xcip_output(struct sess* sp) { +<% end -%> int gai_error, mmdb_error; char ip[INET6_ADDRSTRLEN]; char* geo[_GEO_IDX_SIZE]; @@ -199,7 +225,11 @@ if (!mmdb) goto out; +<% if @varnish_version4 -%> + if (geo_get_xcip(ctx, ip)) +<% else -%> if (geo_get_xcip(sp, ip)) +<% end -%> goto out; MMDB_lookup_result_s result = MMDB_lookup_string(mmdb, ip, &gai_error, &mmdb_error); @@ -232,11 +262,19 @@ } out: +<% if @varnish_version4 -%> + geo_out_cookie(ctx, geo); +<% else -%> geo_out_cookie(sp, geo); +<% end -%> } }C // Emits a Set-Cookie sub geoip_cookie { +<% if @varnish_version4 -%> + C{geo_xcip_output(ctx);}C +<% else -%> C{geo_xcip_output(sp);}C +<% end -%> } diff --git a/modules/varnish/templates/initscripts/varnish.systemd.erb b/modules/varnish/templates/initscripts/varnish.systemd.erb index 3d2414d..c3397f6 100644 --- a/modules/varnish/templates/initscripts/varnish.systemd.erb +++ b/modules/varnish/templates/initscripts/varnish.systemd.erb @@ -27,6 +27,7 @@ <% if @varnish_version4 -%> -p thread_pool_min=250 -p thread_pool_max=<%= @processorcount.to_i * 250 -%> -p thread_pool_timeout=120 \ -p vsl_reclen=2048 \ +-p vcc_allow_inline_c=true \ <% else -%> -w 250,<%= @processorcount.to_i * 250 -%>,120 \ -p shm_reclen=2048 \ diff --git a/modules/varnish/templates/normalize_path.inc.vcl.erb b/modules/varnish/templates/normalize_path.inc.vcl.erb index e145508..dfe5ed5 100644 --- a/modules/varnish/templates/normalize_path.inc.vcl.erb +++ b/modules/varnish/templates/normalize_path.inc.vcl.erb @@ -12,7 +12,11 @@ #define NP_IS_HEX(c) (NP_HEX_DIGIT(c) != -1) #define NP_HEXCHAR(c1, c2) (char)( (NP_HEX_DIGIT(c1) << 4) | NP_HEX_DIGIT(c2) ) +<% if @varnish_version4 -%> +void raw_normalize_path(const struct vrt_ctx *ctx, const int doslash) { +<% else -%> void raw_normalize_path(struct sess *sp, const int doslash) { +<% end -%> /* Rewrite the path part of the URL, replacing unnecessarily escaped * punctuation with the actual characters. The character list is from * MediaWiki's wfUrlencode(), so the URLs produced here will be the same as @@ -20,7 +24,11 @@ * cache fragmentation and fixes T29935, i.e. stale cache entries due to * MediaWiki purging only the wfUrlencode'd version of the URL. */ +<% if @varnish_version4 -%> + const char * url = VRT_r_req_url(ctx); +<% else -%> const char * url = VRT_r_req_url(sp); +<% end -%> size_t i, outPos; const size_t urlLength = strlen(url); // index for the last position %XX can start at: @@ -72,7 +80,11 @@ * vrt_magic_string_end marks the end of the list. */ if (dirty) { +<% if @varnish_version4 -%> + VRT_l_req_url(ctx, destBuffer, vrt_magic_string_end); +<% else -%> VRT_l_req_url(sp, destBuffer, vrt_magic_string_end); +<% end -%> } } } @@ -84,9 +96,17 @@ }C sub normalize_mediawiki_path { +<% if @varnish_version4 -%> + C{ raw_normalize_path(ctx, 1); }C +<% else -%> C{ raw_normalize_path(sp, 1); }C +<% end -%> } sub normalize_rest_path { +<% if @varnish_version4 -%> + C{ raw_normalize_path(ctx, 0); }C +<% else -%> C{ raw_normalize_path(sp, 0); }C +<% end -%> } diff --git a/modules/varnish/templates/text-backend.inc.vcl.erb b/modules/varnish/templates/text-backend.inc.vcl.erb index 5ef7d09..2c3f0d2 100644 --- a/modules/varnish/templates/text-backend.inc.vcl.erb +++ b/modules/varnish/templates/text-backend.inc.vcl.erb @@ -3,39 +3,71 @@ include "text-common.inc.vcl"; sub cluster_be_recv_pre_purge { - if (req.request == "PURGE") { + if (<%= @req_method %> == "PURGE") { call text_normalize_path; } } sub cluster_be_recv_applayer_backend { if (req.http.Host == "cxserver.wikimedia.org" ) { # LEGACY: to be removed eventually + <%- if @varnish_version4 -%> + set req.backend_hint = cxserver_backend.backend(); + <%- else -%> set req.backend = cxserver_backend; + <%- end -%> } else if (req.http.Host == "citoid.wikimedia.org" ) { # LEGACY: to be removed eventually + <%- if @varnish_version4 -%> + set req.backend_hint = citoid_backend.backend(); + <%- else -%> set req.backend = citoid_backend; + <%- end -%> } else { // default for all other hostnames if (req.url ~ "^/api/rest_v1/") { + <%- if @varnish_version4 -%> + set req.backend_hint = restbase_backend.backend(); + <%- else -%> set req.backend = restbase_backend; + <%- end -%> } else if (req.url ~ "^/w/api\.php") { + <%- if @varnish_version4 -%> + set req.backend_hint = api.backend(); + <%- else -%> set req.backend = api; + <%- end -%> + set req.http.X-Backend-is-Mediawiki = 1; } else if (req.url ~ "^/w/thumb(_handler)?\.php") { + <%- if @varnish_version4 -%> + set req.backend_hint = rendering.backend(); + <%- else -%> set req.backend = rendering; + <%- end -%> + set req.http.X-Backend-is-Mediawiki = 1; } else { + <%- if @varnish_version4 -%> + set req.backend_hint = appservers.backend(); + <%- else -%> set req.backend = appservers; + <%- end -%> + set req.http.X-Backend-is-Mediawiki = 1; } } - if (req.http.X-Wikimedia-Debug && ( - req.backend == appservers - || req.backend == rendering - || req.backend == api - )) { + if (req.http.X-Wikimedia-Debug && req.http.X-Backend-is-Mediawiki) { + <%- if @varnish_version4 -%> + set req.backend_hint = appservers_debug.backend(); + <%- else -%> set req.backend = appservers_debug; + <%- end -%> + unset req.http.X-Backend-is-Mediawiki; } <% if @varnish_directors.include?('security_audit') && !@varnish_directors['security_audit']['backends'].empty? %> if (req.http.X-Wikimedia-Security-Audit == "1") { + <%- if @varnish_version4 -%> + set req.backend_hint = security_audit.backend(); + <%- else -%> set req.backend = security_audit; + <%- end -%> } <% end %> } @@ -73,29 +105,41 @@ sub cluster_be_hit { } sub cluster_be_miss { + <%- if not @varnish_version4 -%> call misspass_mangle; call text_common_misspass_restore_cookie; + <%- end -%> } sub cluster_be_pass { + <%- if not @varnish_version4 -%> call misspass_mangle; call text_common_misspass_restore_cookie; + <%- end -%> } <% if @varnish_version4 -%> -sub cluster_be_backend_fetch { } +sub cluster_be_backend_fetch { + call misspass_mangle; + call text_common_misspass_restore_cookie; +} <% end -%> sub cluster_be_backend_response { // Make sure Set-Cookie responses are not cacheable, and log violations // FIXME: exceptions are ugly; maybe get rid of the whole thing? if (beresp.ttl > 0s && beresp.http.Set-Cookie && - req.url !~ "^/wiki/Special:HideBanners") { - std.log("Cacheable object with Set-Cookie found. req.url: " + req.url + " Cache-Control: " + beresp.http.Cache-Control + " Set-Cookie: " + beresp.http.Set-Cookie); + <%= @bereq_req %>.url !~ "^/wiki/Special:HideBanners") { + std.log("Cacheable object with Set-Cookie found. <%= @bereq_req %>.url: " + <%= @bereq_req %>.url + " Cache-Control: " + beresp.http.Cache-Control + " Set-Cookie: " + beresp.http.Set-Cookie); set beresp.http.Cache-Control = "private, max-age=0, s-maxage=0"; set beresp.ttl = 0s; set beresp.http.X-CDIS = "pass"; + <%- if @varnish_version4 -%> + set beresp.uncacheable = true; + return (deliver); + <%- else -%> return (hit_for_pass); + <%- end -%> } // FIXME: Fix up missing Vary headers on Apache redirects diff --git a/modules/varnish/templates/text-common.inc.vcl.erb b/modules/varnish/templates/text-common.inc.vcl.erb index 2831af7..b018093 100644 --- a/modules/varnish/templates/text-common.inc.vcl.erb +++ b/modules/varnish/templates/text-common.inc.vcl.erb @@ -77,7 +77,7 @@ // fe+be common recv code sub text_common_recv { - if (req.request != "GET" && req.request != "HEAD") { + if (<%= @req_method %> != "GET" && <%= @req_method %> != "HEAD") { return (pass); } @@ -121,7 +121,7 @@ // fe+be common fetch code sub text_common_backend_response { - if (req.http.X-Subdomain && (req.url ~ "mobileaction=" || req.url ~ "useformat=")) { + if (<%= @bereq_req %>.http.X-Subdomain && (<%= @bereq_req %>.url ~ "mobileaction=" || <%= @bereq_req %>.url ~ "useformat=")) { set beresp.ttl = 60 s; } @@ -146,12 +146,17 @@ && beresp.status < 500 && (!beresp.http.X-Cache-Int || beresp.http.X-Cache-Int !~ " hit") ) || ( - req.http.Cookie == "Token=1" + <%= @bereq_req %>.http.Cookie == "Token=1" && beresp.http.Vary ~ "(?i)(^|,)\s*Cookie\s*(,|$)" ) ) { set beresp.ttl = 601s; set beresp.http.X-CDIS = "pass"; + <%- if @varnish_version4 -%> + set beresp.uncacheable = true; + return (deliver); + <%- else -%> return (hit_for_pass); + <%- end -%> } } diff --git a/modules/varnish/templates/text-frontend.inc.vcl.erb b/modules/varnish/templates/text-frontend.inc.vcl.erb index 1919693..28fb15b 100644 --- a/modules/varnish/templates/text-frontend.inc.vcl.erb +++ b/modules/varnish/templates/text-frontend.inc.vcl.erb @@ -17,7 +17,7 @@ // the example of traffic sourced directly by satellite or something. sub mobile_redirect { - if (!req.http.X-Subdomain && (req.request == "GET" || req.request == "HEAD") + if (!req.http.X-Subdomain && (<%= @req_method %> == "GET" || <%= @req_method %> == "HEAD") && (req.http.User-Agent ~ "(?i)(mobi|240x240|240x320|320x320|alcatel|android|audiovox|bada|benq|blackberry|cdm-|compal-|docomo|ericsson|hiptop|htc[-_]|huawei|ipod|kddi-|kindle|meego|midp|mitsu|mmp\/|mot-|motor|ngm_|nintendo|opera.m|palm|panasonic|philips|phone|playstation|portalmmm|sagem-|samsung|sanyo|sec-|semc-browser|sendo|sharp|silk|softbank|symbian|teleca|up.browser|vodafone|webos)" || req.http.User-Agent ~ "^(?i)(lge?|sie|nec|sgh|pg)-" || req.http.Accept ~ "vnd.wap.wml") && req.http.Cookie !~ "(stopMobileRedirect=true|mf_useformat=desktop)" @@ -41,7 +41,7 @@ } else { set req.http.Location = "http://" + req.http.MobileHost + req.url; } - error 666 "Found"; + <%= error_synth(666, "Found") -%> } unset req.http.MobileHost; } @@ -50,15 +50,15 @@ sub cluster_fe_recv_pre_purge { // Forged UAs on zerodot. This largely handles lazywebtools below, incidentally. if (req.http.host ~ "zero\.wikipedia\.org" && req.http.User-Agent && req.http.User-Agent ~ "Facebookbot|Googlebot") { - error 403 "Noise"; + <%= error_synth(403, "Noise") -%> } if (req.http.referer && req.http.referer ~ "^http://(www\.(keeprefreshing|refreshthis|refresh-page|urlreload)\.com|tuneshub\.blogspot\.com|itunes24x7\.blogspot\.com|autoreload\.net|www\.lazywebtools\.co\.uk)/") { - error 403 "Noise"; + <%= error_synth(403, "Noise") -%> } - if (req.request == "POST" && req.url ~ "index\.php\?option=com_jce&task=plugin&plugin=imgmanager&file=imgmanager&method=form&cid=") { - error 403 "Noise"; + if (<%= @req_method %> == "POST" && req.url ~ "index\.php\?option=com_jce&task=plugin&plugin=imgmanager&file=imgmanager&method=form&cid=") { + <%= error_synth(403, "Noise") -%> } // FIXME: we're seeing an issue with Range requests and gzip/gunzip. @@ -182,7 +182,7 @@ // randomly. We may increase that a bit in the future and/or widen the // UA filter, but this is a good starting point. if (req.http.X-Connection-Properties ~ "DES-CBC3" - && req.http.User-Agent ~ "Windows NT" && req.request == "GET" + && req.http.User-Agent ~ "Windows NT" && <%= @req_method %> == "GET" && req.url ~ "^/wiki/" && req.http.host !~ "\.m\." && req.http.Cookie !~ "Browser-Security=Awful" && std.random(0,100) < 2.0) { @@ -246,7 +246,11 @@ if (resp.status == 302) { unset resp.http.Location; set resp.status = 200; + <%- if @varnish_version4 -%> + set resp.reason = "OK"; + <%- else -%> set resp.response = "OK"; + <%- end -%> } elsif (resp.status == 301) { // preserve the original client redirect preference on title redirects if (resp.http.Location ~ "[?]") { @@ -287,29 +291,29 @@ sub cluster_fe_err_synth { // Support mobile redirects - if (obj.status == 666) { - set obj.http.Location = req.http.Location; - set obj.status = 302; - set obj.http.Connection = "keep-alive"; - set obj.http.Content-Length = "0"; // BZ #62245 + if (<%= @resp_obj %>.status == 666) { + set <%= @resp_obj %>.http.Location = req.http.Location; + set <%= @resp_obj %>.status = 302; + set <%= @resp_obj %>.http.Connection = "keep-alive"; + set <%= @resp_obj %>.http.Content-Length = "0"; // BZ #62245 return (deliver); } // Chrome/41-on-Windows: T141786 - if (obj.status == 741) { - set obj.status = 401; - set obj.http.WWW-Authenticate = {"Basic realm="Buggy request, please report at https://phabricator.wikimedia.org/T141786""}; + if (<%= @resp_obj %>.status == 741) { + set <%= @resp_obj %>.status = 401; + set <%= @resp_obj %>.http.WWW-Authenticate = {"Basic realm="Buggy request, please report at https://phabricator.wikimedia.org/T141786""}; return (deliver); } // Browser sec redirect, with a cookie to prevent repeats in the same session // Note the cookie is is per-site for simplicity - if (obj.status == 787) { - set obj.status = 302; - set obj.http.Connection = "keep-alive"; - set obj.http.Content-Length = "0"; // BZ #62245 - set obj.http.Set-Cookie = "Browser-Security=Awful; Path=/; secure"; - set obj.http.Location = "https://wikitech.wikimedia.org/wiki/HTTPS:_Browser_Recommendations"; + if (<%= @resp_obj %>.status == 787) { + set <%= @resp_obj %>.status = 302; + set <%= @resp_obj %>.http.Connection = "keep-alive"; + set <%= @resp_obj %>.http.Content-Length = "0"; // BZ #62245 + set <%= @resp_obj %>.http.Set-Cookie = "Browser-Security=Awful; Path=/; secure"; + set <%= @resp_obj %>.http.Location = "https://wikitech.wikimedia.org/wiki/HTTPS:_Browser_Recommendations"; return (deliver); } } -- To view, visit https://gerrit.wikimedia.org/r/314716 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I6789d1129aee77f5eb8b7816afeaac2c1a7a1a41 Gerrit-PatchSet: 5 Gerrit-Project: operations/puppet Gerrit-Branch: production Gerrit-Owner: Ema <e...@wikimedia.org> Gerrit-Reviewer: BBlack <bbl...@wikimedia.org> Gerrit-Reviewer: Ema <e...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits