BBlack has submitted this change and it was merged. Change subject: Revert "Revert "Set domain to TLD on GeoIP cookie"" ......................................................................
Revert "Revert "Set domain to TLD on GeoIP cookie"" Apparently not causing current prod issue, un-reverting. This reverts commit bcc85b3806aba03c354d266a7715c52de21c6d43. Change-Id: Ibcb07f4d513863d9970fa76eede5dce48f8e5f4e --- M templates/varnish/geoip.inc.vcl.erb 1 file changed, 58 insertions(+), 6 deletions(-) Approvals: BBlack: Verified; Looks good to me, approved diff --git a/templates/varnish/geoip.inc.vcl.erb b/templates/varnish/geoip.inc.vcl.erb index b9e5d71..3cbb2b2 100644 --- a/templates/varnish/geoip.inc.vcl.erb +++ b/templates/varnish/geoip.inc.vcl.erb @@ -22,6 +22,7 @@ char * geo_get_xff_ip (const struct sess *sp); char * geo_sanitize_for_cookie (char *string); void geo_set_cache_control (const struct sess *sp); + const char * geo_get_top_cookie_domain (const char *host); void geo_init() { @@ -93,6 +94,51 @@ VRT_SetHdr(sp, HDR_OBJ, "\016Last-Modified:", now, vrt_magic_string_end); VRT_SetHdr(sp, HDR_OBJ, "\016Cache-Control:", "private, max-age=86400, s-maxage=0", vrt_magic_string_end); } + + + /* + * Extract the topmost part of the domain name for which a cookie may be set. + * This consists of the public suffix (e.g., 'org') plus one more level. + * + * In Wikimedia's case, this is always the top two parts of the name (for example, + * 'wikipedia.org' for 'en.m.wikipedia.org'. But we handle other common cases correctly too, + * like 'news.bbc.co.uk' (which may set cookies for bbc.co.uk, but not the entire co.uk public + * suffix), by assuming that if either of the top two levels is less than three characters + * long, then the public suffix contains two parts. A fully comprehensive and correct solution + * would require checking against a public suffix database like <https://publicsuffix.org/>. + */ + const char * + geo_get_top_cookie_domain(const char *host) { + const char *last, *second_last, *third_last, *pos, *top_cookie_domain; + + if (host == NULL) { + return NULL; + } + + last = second_last = third_last = host; + for (pos = host; *pos != '\0'; pos++) { + if (*pos == '.') { + third_last = second_last; + second_last = last; + last = pos; + } + } + + /* If either the second- or top-level domain is less than three characters long, */ + /* assume that the domain uses a two-part public suffix (like '.co.uk') and include */ + /* one additional level in the result. */ + if ((pos - last) <= 3 || (last - second_last) <= 3) { + top_cookie_domain = third_last; + } else { + top_cookie_domain = second_last; + } + + if (*top_cookie_domain == '.') { + top_cookie_domain++; + } + + return top_cookie_domain; + } }C sub geoip_lookup { @@ -141,7 +187,8 @@ const char *cookie_out = NULL; char cookie_buf[255]; - char *ip = geo_get_xff_ip(sp); + const char *host = VRT_GetHdr(sp, HDR_REQ, "\005host:"); + const char *ip = geo_get_xff_ip(sp); int af = geo_get_addr_family(ip); if (af == -1) { ip = VRT_IP_string(sp, VRT_r_client_ip(sp)); @@ -151,21 +198,26 @@ geo_init(); record = GeoIP_record_by_addr(gi, ip); + int snp_len; if (record) { /* Set-Cookie: GeoIP=US:San_Francisco:37.7749:-122.4194:v4; path=/ */ - int snp_len = snprintf(cookie_buf, sizeof(cookie_buf), "GeoIP=%s:%s:%.4f:%.4f:%s; path=/", + snp_len = snprintf(cookie_buf, sizeof(cookie_buf), "GeoIP=%s:%s:%.4f:%.4f:%s; Path=/; Domain=.%s", record->country_code ? geo_sanitize_for_cookie(record->country_code) : "", record->city ? geo_sanitize_for_cookie(record->city) : "", record->latitude, record->longitude, - (af == AF_INET6) ? "v6" : "v4" + (af == AF_INET6) ? "v6" : "v4", + geo_get_top_cookie_domain(host) ); - if (snp_len < sizeof(cookie_buf)) /* don't use truncated output */ - cookie_out = cookie_buf; GeoIPRecord_delete(record); } else { - cookie_out = (af == AF_INET6) ? "GeoIP=::::v6; path=/" : "GeoIP=::::v4; path=/"; + snp_len = snprintf(cookie_buf, sizeof(cookie_buf), "GeoIP=::::%s; Path=/; Domain=.%s", + (af == AF_INET6) ? "v6" : "v4", + geo_get_top_cookie_domain(host) + ); } + if (snp_len < sizeof(cookie_buf)) /* don't use truncated output */ + cookie_out = cookie_buf; if (cookie_out) { /* Use libvmod-header to ensure the Set-Cookie header we are adding does not -- To view, visit https://gerrit.wikimedia.org/r/130342 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibcb07f4d513863d9970fa76eede5dce48f8e5f4e Gerrit-PatchSet: 2 Gerrit-Project: operations/puppet Gerrit-Branch: production Gerrit-Owner: BBlack <bbl...@wikimedia.org> Gerrit-Reviewer: BBlack <bbl...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits