From 5f84b7de8ddc15d3499b41fdd90f0a1ea5335b4a Mon Sep 17 00:00:00 2001 From: Murtaza Aliakbar Date: Thu, 27 Jan 2022 19:37:13 +0530 Subject: [PATCH] make http and local caches store identical values --- src/plugins/cacheutil.js | 22 ++++++++---- src/plugins/dns-operation/cacheResponse.js | 3 +- src/plugins/dns-operation/dnsCache.js | 42 ++++++++++++++-------- src/plugins/dns-operation/dnsResolver.js | 15 ++++---- wrangler.toml | 2 +- 5 files changed, 53 insertions(+), 31 deletions(-) diff --git a/src/plugins/cacheutil.js b/src/plugins/cacheutil.js index 38216346ab..c584211b5c 100644 --- a/src/plugins/cacheutil.js +++ b/src/plugins/cacheutil.js @@ -57,17 +57,22 @@ function makeCacheMetadata(dnsPacket, stamps) { }; } -export function makeCacheValue(packet, metadata) { - // null value allowed for packet +export function makeCacheValue(packet, raw, metadata) { + // null value allowed for packet / raw return { dnsPacket: packet, + dnsBuffer: raw, metadata: metadata, }; } -export function cacheValueOf(packet, stamps) { +export function cacheValueOf(rdnsResponse) { + const stamps = rdnsResponse.stamps; + const packet = rdnsResponse.dnsPacket; + const raw = rdnsResponse.dnsBuffer; + const metadata = makeCacheMetadata(packet, stamps); - return makeCacheValue(packet, metadata); + return makeCacheValue(packet, raw, metadata); } export function updateTtl(packet, end) { @@ -91,9 +96,14 @@ function makeId(packet) { return name + ":" + type; } -export function makeHttpCacheValue(packet, metadata) { - const b = dnsutil.encode(packet); +export function makeLocalCacheValue(b, metadata) { + return { + dnsBuffer: b, + metadata: metadata, + }; +} +export function makeHttpCacheValue(b, metadata) { const headers = { headers: util.concatHeaders( { diff --git a/src/plugins/dns-operation/cacheResponse.js b/src/plugins/dns-operation/cacheResponse.js index 7662dc11bc..cd1cfe9527 100644 --- a/src/plugins/dns-operation/cacheResponse.js +++ b/src/plugins/dns-operation/cacheResponse.js @@ -69,7 +69,6 @@ export class DNSCacheResponder { if (util.emptyObj(cr)) return noAnswer; - const dnsBuffer = dnsutil.encode(cr.dnsPacket); // note: stamps in cr may be out-of-date; for ex, consider a // scenario where v6.example.com AAAA to fda3:: today, // but CNAMEs to v6.test.example.org tomorrow. cr.metadata @@ -77,7 +76,7 @@ export class DNSCacheResponder { // whereas it should be [v6.example.com, example.com // v6.test.example.org, test.example.org, example.org] const stamps = rdnsutil.blockstampFromCache(cr); - const res = rdnsutil.dnsResponse(cr.dnsPacket, dnsBuffer, stamps); + const res = rdnsutil.dnsResponse(cr.dnsPacket, cr.dnsBuffer, stamps); this.makeCacheResponse(rxid, /* out*/ res, blockInfo); diff --git a/src/plugins/dns-operation/dnsCache.js b/src/plugins/dns-operation/dnsCache.js index 3b8f668d56..4791b4d2c6 100644 --- a/src/plugins/dns-operation/dnsCache.js +++ b/src/plugins/dns-operation/dnsCache.js @@ -8,6 +8,7 @@ import { LfuCache } from "@serverless-dns/lfu-cache"; import { CacheApi } from "./cacheApi.js"; +import * as bufutil from "../../commons/bufutil.js"; import * as dnsutil from "../../commons/dnsutil.js"; import * as util from "../../commons/util.js"; import * as cacheutil from "../cacheutil.js"; @@ -53,14 +54,15 @@ export class DnsCache { util.emptyString(url.href) || util.emptyObj(data) || util.emptyObj(data.metadata) || - util.emptyObj(data.dnsPacket) + util.emptyObj(data.dnsPacket) || + bufutil.emptyBuf(data.dnsBuffer) ) { this.log.w("put: empty url/data", url, data); return; } try { - // data -> {dnsPacket, metadata}; dnsPacket may be null + // data: {dnsPacket, dnsBuffer, metadata}; dnsPacket/Buffer may be null this.log.d("put: data in cache", data); // a race where the cache may infact have a fresh answer, @@ -74,7 +76,7 @@ export class DnsCache { return; } // else: override cachedEntry with incoming - this.putLocalCache(url.href, data); + this.putLocalCache(url, data); dispatcher(this.putHttpCache(url, data)); } catch (e) { @@ -82,20 +84,32 @@ export class DnsCache { } } - putLocalCache(k, v) { + putLocalCache(url, data) { + const k = url.href; + const v = cacheutil.makeLocalCacheValue(data.dnsBuffer, data.metadata); + if (!k || !v) return; this.localcache.Put(k, v); } fromLocalCache(key) { - const v = this.localcache.Get(key); - return cacheutil.isValueValid(v) ? v : false; + const res = this.localcache.Get(key); + + if (util.emptyObj(res)) return false; + + const b = res.dnsBuffer; + const p = dnsutil.decode(b); + const m = res.metadata; + + const cr = cacheutil.makeCacheValue(p, b, m); + + return cacheutil.isValueValid(cr) ? cr : false; } async putHttpCache(url, data) { const k = url.href; - const v = cacheutil.makeHttpCacheValue(data.dnsPacket, data.metadata); + const v = cacheutil.makeHttpCacheValue(data.dnsBuffer, data.metadata); if (!k || !v) return; @@ -110,15 +124,15 @@ export class DnsCache { const metadata = cacheutil.extractMetadata(response); this.log.d("http-cache response metadata", metadata); - if (!cacheutil.hasMetadata(metadata)) { - return false; - } - - // 'p' may be null or just a dns question or a dns answer - const p = dnsutil.decode(await response.arrayBuffer()); + // 'b' shouldn't be null; but a dns question or a dns answer + const b = await response.arrayBuffer(); + // when 'b' is less than dns-packet header-size, decode errs out + const p = dnsutil.decode(b); // though 'm' is never empty const m = metadata; - return cacheutil.makeCacheValue(p, m); + const cr = cacheutil.makeCacheValue(p, b, m); + + return cacheutil.isValueValid(cr) ? cr : false; } } diff --git a/src/plugins/dns-operation/dnsResolver.js b/src/plugins/dns-operation/dnsResolver.js index ec56f328c4..14e3b13a84 100644 --- a/src/plugins/dns-operation/dnsResolver.js +++ b/src/plugins/dns-operation/dnsResolver.js @@ -145,7 +145,7 @@ export default class DNSResolver { const dnsPacket = dnsutil.decode(raw); // stamps are empty for domains that are not in any blocklist // but there's no way to know if that was indeed the case as - // stamps are shared by cacheResolver, which may or may not + // stamps are sent here by cacheResolver, which may or may not // have retrieved the stamps in the first-place (in which case // these would be empty, regardless of whether the domain was // in any of the blocklists or not). @@ -168,7 +168,7 @@ export default class DNSResolver { return; } - const v = cacheutil.cacheValueOf(r.dnsPacket, r.stamps); + const v = cacheutil.cacheValueOf(r); this.cache.put(k, v, dispatcher); } @@ -206,6 +206,7 @@ DNSResolver.prototype.resolveDnsUpstream = async function ( this.log.w(rxid, "missing resolver url", rurl); continue; } + const u = new URL(request.url); const upstream = new URL(rurl); u.hostname = upstream.hostname; // default cloudflare-dns.com @@ -244,17 +245,15 @@ DNSResolver.prototype.resolveDnsFromCache = async function (rxid, packet) { if (!k) throw new Error("resolver: no cache-key"); const cr = await this.cache.get(k); - const hasVal = cacheutil.isValueValid(cr); - const hasAns = hasVal && dnsutil.isAnswer(cr.dnsPacket); - const freshAns = hasVal && cacheutil.isAnswerFresh(cr.metadata); - this.log.d(rxid, "cache ans", k.href, hasVal, "fresh?", freshAns); + const hasAns = dnsutil.isAnswer(cr.dnsPacket); + const freshAns = cacheutil.isAnswerFresh(cr.metadata); + this.log.d(rxid, "cache ans", k.href, "ans?", hasAns, "fresh?", freshAns); if (!hasAns || !freshAns) { throw new Error("resolver: cache miss"); } - cacheutil.updatedAnswer(cr.dnsPacket, packet.id, cr.metadata.expiry); - + cacheutil.updatedAnswer(cr.dnsPacket, packet.id); const b = dnsutil.encode(cr.dnsPacket); return new Response(b, { headers: cacheutil.cacheHeaders() }); diff --git a/wrangler.toml b/wrangler.toml index bd0dca389a..72aa1ec10b 100644 --- a/wrangler.toml +++ b/wrangler.toml @@ -4,7 +4,7 @@ type = "webpack" account_id = "" workers_dev = true zone_id = "" -compatibility_date = "2021-09-21" +compatibility_date = "2021-11-10" webpack_config = "webpack.config.cjs" [vars]