Make util.emptyObj handle objs with inherited keys

util.emptyObj did not work for 'new URL(str)', 'new Date()', 'new Response()'
because even if these objects did not contain own properties, they did inherit
from their prototypes (something which the util.emptyObj impl did not account
for). This resulted in all sorts of wrong empty object detection, that is, an
obj (with prototypal properties) would be marked empty, even when it was not.

src/commons/util.js

@@ -91,18 +91,6 @@ export function copyHeaders(request) {
   return headers;
 }
 
-export function copyNonPseudoHeaders(req) {
-  const headers = {};
-  if (!req || !req.headers) return headers;
-
-  // drop http/2 pseudo-headers
-  for (const name in req.headers) {
-    if (name.startsWith(":")) continue;
-    headers[name] = req.headers[name];
-  }
-  return headers;
-}
-
 /**
  * Promise that resolves after `ms`
  * @param {number} ms - Milliseconds to sleep
@@ -157,7 +145,7 @@ export function timedSafeAsyncOp(promisedOp, ms, defaultOp) {
   // constructs (async, await, catch, then etc) within a
   // "new Promise()" is an anti-pattern and hard to get right:
   // stackoverflow.com/a/23803744 and stackoverflow.com/a/25569299
-  return new Promise((resolve, _) => {
+  return new Promise((resolve, reject) => {
     let timedout = false;
 
     const defferedOp = () => {
@@ -271,9 +259,10 @@ export function emptyResponse() {
 }
 
 export function errResponse(id, err) {
+  const st = emptyObj(err) || !err.stack ? "no-stacktrace" : err.stack;
   return {
     isException: true,
-    exceptionStack: err.stack,
+    exceptionStack: st,
     exceptionFrom: id,
     data: {},
   };
@@ -305,9 +294,13 @@ export function concatObj(...args) {
   return Object.assign(...args);
 }
 
+// stackoverflow.com/a/32108184/402375
 export function emptyObj(x) {
   // note: Object.keys type-errors when x is null / undefined
-  return !x || Object.keys(x).length <= 0;
+  if (!x) return true;
+  return (
+    Object.keys(x).length === 0 && Object.getPrototypeOf(x) === Object.prototype
+  );
 }
 
 export function emptyMap(m) {

src/core/currentRequest.js

@@ -69,7 +69,10 @@ export default class CurrentRequest {
   }
 
   hResponse(r) {
-    if (util.emptyObj(r)) return;
+    if (util.emptyObj(r)) {
+      this.log.w("no http-res to set, empty obj?", r);
+      return;
+    }
 
     this.httpResponse = r;
     this.stopProcessing = true;

src/core/doh.js

@@ -33,8 +33,6 @@ async function proxyRequest(event) {
     errorResponse(cr, err);
   }
 
-  // TODO: cors-headers are also set in server-node.js
-  // centralize setting these in just one place, if possible
   const ua = event.request.headers.get("User-Agent");
   if (util.fromBrowser(ua)) cr.setCorsHeadersIfNeeded();
 
@@ -46,6 +44,6 @@ function optionsRequest(request) {
 }
 
 function errorResponse(currentRequest, err = null) {
-  const eres = util.emptyObj(err) ? null : util.errResponse("doh.js", err);
+  const eres = util.errResponse("doh.js", err);
   currentRequest.dnsExceptionResponse(eres);
 }

src/core/node/util.js

@@ -35,6 +35,15 @@ export function getTLSfromEnv(TLS_CRT_KEY) {
  * @return {Object}
  */
 export function copyNonPseudoHeaders(headers) {
+  // nodejs req headers may be of form
+  // ':authority': 'localhost:8080'
+  // ':method': 'GET'
+  // ':path': '/1:AAIAgA==?dns=AAABAAABAAAAAAAACnJldGhpbmtkbnMDY29tAAABAAE'
+  // ':scheme': 'https'
+  // accept: 'application/dns-message'
+  // 'user-agent': 'Go-http-client/2.0'
+  // [Symbol(nodejs.http2.sensitiveHeaders)]: []
+
   const out = {};
 
   if (!headers) return out;

src/core/plugin.js

@@ -172,6 +172,7 @@ export default class RethinkPlugin {
     this.log.d(rxid, "command-control response");
 
     if (!util.emptyObj(r) && r.stopProcessing) {
+      this.log.d(rxid, "command-control reply", r);
       currentRequest.hResponse(r.httpResponse);
     }
   }
@@ -268,7 +269,10 @@ export default class RethinkPlugin {
       // throw away any request that is not a dns-msg since cc.js
       // processes non-dns msgs only via GET, while rest of the
       // plugins process only dns-msgs via GET and POST.
-      if (!util.isGetRequest(request)) setInvalidResponse(currentRequest);
+      if (!util.isGetRequest(request)) {
+        this.log.i(rxid, "not a dns-msg, not a GET req either", request);
+        setInvalidResponse(currentRequest);
+      }
       return;
     }
 

src/plugins/command-control/cc.js

@@ -7,11 +7,11 @@
  */
 import * as envutil from "../../commons/envutil.js";
 import * as util from "../../commons/util.js";
-import * as dnsBlockUtil from "../dnsblockutil.js";
+import * as rdnsutil from "../dnsblockutil.js";
 
 export class CommandControl {
   constructor() {
-    this.latestTimestamp = "";
+    this.latestTimestamp = envutil.timestamp();
     this.log = log.withTags("CommandControl");
   }
 
@@ -24,8 +24,6 @@ export class CommandControl {
    * @returns
    */
   async RethinkModule(param) {
-    this.latestTimestamp = envutil.timestamp();
-
     // process only GET requests, ignore all others
     if (util.isGetRequest(param.request)) {
       return this.commandOperation(
@@ -80,6 +78,7 @@ export class CommandControl {
       const queryString = reqUrl.searchParams;
       const pathSplit = reqUrl.pathname.split("/");
 
+      // FIXME: isDohGetRequest is redundant, simply trust isDnsMsg as-is
       const isDnsCmd = isDnsMsg || this.isDohGetRequest(queryString);
 
       if (isDnsCmd) {
@@ -215,8 +214,8 @@ function b64ToList(queryString, blocklistFilter) {
     listDetail: {},
   };
 
-  const stamp = dnsBlockUtil.unstamp(b64);
-  if (dnsBlockUtil.hasBlockstamp(stamp)) {
+  const stamp = rdnsutil.unstamp(b64);
+  if (rdnsutil.hasBlockstamp(stamp)) {
     return jsonResponse(r);
   }
 

src/plugins/dns-operation/dnsResolver.js

@@ -86,11 +86,11 @@ export default class DNSResolver {
   async resolveDns(param) {
     const rxid = param.rxid;
     const blInfo = param.userBlocklistInfo;
-    const packet = param.requestBodyBuffer;
+    const rawpacket = param.requestBodyBuffer;
     const blf = param.blocklistFilter;
     const dispatcher = param.dispatcher;
 
-    const q = await this.makeRdnsResponse(rxid, packet, blf);
+    const q = await this.makeRdnsResponse(rxid, rawpacket, blf);
 
     this.blocker.blockQuestion(rxid, /* out*/ q, blInfo);
     this.log.d(rxid, blInfo, "question blocked?", q.isBlocked);
@@ -129,7 +129,7 @@ export default class DNSResolver {
   }
 
   async makeRdnsResponse(rxid, raw, blf) {
-    if (!raw) throw new Error(rxid + "mk-res no upstream result");
+    if (!raw) throw new Error(rxid + " mk-res no upstream result");
 
     const dnsPacket = dnsutil.decode(raw);
     const stamps = rdnsutil.blockstampFromBlocklistFilter(dnsPacket, blf);

src/server-deno.ts

@@ -207,7 +207,6 @@ async function resolveQuery(q: Uint8Array) {
 }
 
 function mkFetchEvent(r: Request, ...fns: Function[]) {
-  // just like with URL objs, util.emptyObj does not work for Request
   if (!r) throw new Error("missing request");
 
   // deno.land/manual/runtime/http_server_apis#http-requests-and-responses

src/server-node.js

@@ -15,6 +15,7 @@ import { handleRequest } from "./core/doh.js";
 import * as bufutil from "./commons/bufutil.js";
 import * as dnsutil from "./commons/dnsutil.js";
 import * as envutil from "./commons/envutil.js";
+import * as nodeutil from "./core/node/util.js";
 import * as util from "./commons/util.js";
 import "./core/node/config.js";
 
@@ -471,7 +472,7 @@ async function handleHTTPRequest(b, req, res) {
       ...req,
       headers: util.concatHeaders(
         util.rxidHeader(rxid),
-        util.copyNonPseudoHeaders(req.headers)
+        nodeutil.copyNonPseudoHeaders(req.headers)
       ),
       method: req.method,
       body: req.method === "POST" ? b : null,