make http and local caches store identical values

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(
       {

src/plugins/dns-operation/cacheResponse.js

@@ -69,15 +69,14 @@ 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
     // would contain stamps for [v6.example.com, example.com]
     // 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);
 

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,28 +76,40 @@ 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) {
       this.log.e("put", url.href, data, e.stack);
     }
   }
 
-  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;
   }
 }

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() });

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]