Handle & report server request errors as 'requestError' events

README.md

@@ -145,6 +145,10 @@ server.on('request', (request, response, rinfo) => {
   console.log(request.header.id, request.questions[0]);
 });
 
+server.on('requestError', (error) => {
+  console.log('Client sent an invalid request', error);
+});
+
 server.on('listening', () => {
   console.log(server.address());
 });

server/dns.js

@@ -35,8 +35,10 @@ class DNSServer extends EventEmitter {
     });
 
     const emitRequest = (request, send, client) => this.emit('request', request, send, client);
+    const emitRequestError = (error) => this.emit('requestError', error);
     for (const server of servers) {
       server.on('request', emitRequest);
+      server.on('requestError', emitRequestError);
     }
 
     if (options.handle) {

server/doh.js

@@ -43,69 +43,74 @@ class Server extends EventEmitter {
   }
 
   async handleRequest(client, res) {
-    const { method, url, headers } = client;
-    const { pathname, searchParams: query } = new URL(url, 'http://unused/');
-    const { cors } = this;
-    if (cors === true) {
-      res.setHeader('Access-Control-Allow-Origin', '*');
-    } else if (typeof cors === 'string') {
-      res.setHeader('Access-Control-Allow-Origin', cors);
-      res.setHeader('Vary', 'Origin');
-    } else if (typeof cors === 'function') {
-      const isAllowed = cors(headers.origin);
-      res.setHeader('Access-Control-Allow-Origin', isAllowed ? headers.origin : 'false');
-      res.setHeader('Vary', 'Origin');
-    }
-    // debug
-    debug('request', method, url);
-    // We are only handling get and post as reqired by rfc
-    if ((method !== 'GET' && method !== 'POST')) {
-      res.writeHead(405, { 'Content-Type': 'text/plain' });
-      res.write('405 Method not allowed\n');
-      res.end();
-      return;
-    }
-    // Check so the uri is correct
-    if (pathname !== '/dns-query') {
-      res.writeHead(404, { 'Content-Type': 'text/plain' });
-      res.write('404 Not Found\n');
-      res.end();
-      return;
-    }
-    // Make sure the requestee is requesting the correct content type
-    const contentType = headers.accept;
-    if (contentType !== 'application/dns-message') {
-      res.writeHead(400, { 'Content-Type': 'text/plain' });
-      res.write('400 Bad Request: Illegal content type\n');
-      res.end();
-      return;
-    }
-    let queryData;
-    if (method === 'GET') {
-      // Parse query string for the request data
-      const dns = query.get('dns');
-      if (!dns) {
-        res.writeHead(400, { 'Content-Type': 'text/plain' });
-        res.write('400 Bad Request: No query defined\n');
+    try {
+      const { method, url, headers } = client;
+      const { pathname, searchParams: query } = new URL(url, 'http://unused/');
+      const { cors } = this;
+      if (cors === true) {
+        res.setHeader('Access-Control-Allow-Origin', '*');
+      } else if (typeof cors === 'string') {
+        res.setHeader('Access-Control-Allow-Origin', cors);
+        res.setHeader('Vary', 'Origin');
+      } else if (typeof cors === 'function') {
+        const isAllowed = cors(headers.origin);
+        res.setHeader('Access-Control-Allow-Origin', isAllowed ? headers.origin : 'false');
+        res.setHeader('Vary', 'Origin');
+      }
+      // debug
+      debug('request', method, url);
+      // We are only handling get and post as reqired by rfc
+      if ((method !== 'GET' && method !== 'POST')) {
+        res.writeHead(405, { 'Content-Type': 'text/plain' });
+        res.write('405 Method not allowed\n');
         res.end();
         return;
       }
-      // Decode from Base64Url Encoding
-      const base64 = decodeBase64URL(dns);
-      if (!base64) {
+      // Check so the uri is correct
+      if (pathname !== '/dns-query') {
+        res.writeHead(404, { 'Content-Type': 'text/plain' });
+        res.write('404 Not Found\n');
+        res.end();
+        return;
+      }
+      // Make sure the requestee is requesting the correct content type
+      const contentType = headers.accept;
+      if (contentType !== 'application/dns-message') {
         res.writeHead(400, { 'Content-Type': 'text/plain' });
-        res.write('400 Bad Request: Invalid query data\n');
+        res.write('400 Bad Request: Illegal content type\n');
         res.end();
         return;
       }
-      // Decode Base64 to buffer
-      queryData = Buffer.from(base64, 'base64');
-    } else if (method === 'POST') {
-      queryData = await readStream(client);
+      let queryData;
+      if (method === 'GET') {
+        // Parse query string for the request data
+        const dns = query.get('dns');
+        if (!dns) {
+          res.writeHead(400, { 'Content-Type': 'text/plain' });
+          res.write('400 Bad Request: No query defined\n');
+          res.end();
+          return;
+        }
+        // Decode from Base64Url Encoding
+        const base64 = decodeBase64URL(dns);
+        if (!base64) {
+          res.writeHead(400, { 'Content-Type': 'text/plain' });
+          res.write('400 Bad Request: Invalid query data\n');
+          res.end();
+          return;
+        }
+        // Decode Base64 to buffer
+        queryData = Buffer.from(base64, 'base64');
+      } else if (method === 'POST') {
+        queryData = await readStream(client);
+      }
+      // Parse DNS query and Raise event.
+      const message = Packet.parse(queryData);
+      this.emit('request', message, this.response.bind(this, res), client);
+    } catch (e) {
+      this.emit('requestError', e);
+      res.destroy();
     }
-    // Parse DNS query and Raise event.
-    const message = Packet.parse(queryData);
-    this.emit('request', message, this.response.bind(this, res), client);
   }
 
   /**

server/tcp.js

@@ -11,9 +11,14 @@ class Server extends tcp.Server {
   }
 
   async handle(client) {
-    const data = await Packet.readStream(client);
-    const message = Packet.parse(data);
-    this.emit('request', message, this.response.bind(this, client), client);
+    try {
+      const data = await Packet.readStream(client);
+      const message = Packet.parse(data);
+      this.emit('request', message, this.response.bind(this, client), client);
+    } catch (e) {
+      this.emit('requestError', e);
+      client.destroy();
+    }
   }
 
   response(client, message) {

server/udp.js

@@ -20,8 +20,12 @@ class Server extends udp.Socket {
   }
 
   handle(data, rinfo) {
-    const message = Packet.parse(data);
-    this.emit('request', message, this.response.bind(this, rinfo), rinfo);
+    try {
+      const message = Packet.parse(data);
+      this.emit('request', message, this.response.bind(this, rinfo), rinfo);
+    } catch (e) {
+      this.emit('requestError', e);
+    }
   }
 
   response(rinfo, message) {

test/index.js

@@ -2,6 +2,8 @@ const assert = require('assert');
 const test = require('./test');
 const { Packet, createDOHServer, createServer, TCPClient, DOHClient, UDPClient } = require('..');
 const http = require('http');
+const tcp = require('net');
+const udp = require('dgram');
 
 /* TODO: below is unused, either delete or use
 const request = Buffer.from([
@@ -335,6 +337,44 @@ test('server/all#simple-request', async() => {
   await server.close();
 });
 
+test('server/all#invalid-request', async() => {
+  const server = createServer({
+    doh    : true,
+    tcp    : true,
+    udp    : true,
+    handle : () => {},
+  });
+  const servers = await server.listen();
+  assert.ok(servers.udp.port > 1000);
+  assert.ok(servers.tcp.port > 1000);
+  assert.ok(servers.doh.port > 1000);
+
+  const errors = [];
+  server.on('requestError', (e) => {
+    errors.push(e);
+  });
+
+  const tcpSocket = tcp.connect({ port: servers.tcp.port, host: '127.0.0.1' });
+  tcpSocket.on('connect', () => tcpSocket.end('INVALID'));
+
+  const udpSocket = udp.createSocket('udp4');
+  udpSocket.send('INVALID', servers.udp.port, '127.0.0.1', () => udpSocket.close());
+
+  const dohConn = http.get(`http://127.0.0.1:${servers.doh.port}/dns-query?dns=INVALID`, {
+    headers: { accept: 'application/dns-message' },
+  }).on('error', () => {});
+
+  await Promise.all([
+    new Promise((resolve) => tcpSocket.on('close', resolve)),
+    new Promise((resolve) => udpSocket.on('close', resolve)),
+    new Promise((resolve) => dohConn.on('close', resolve)),
+  ]);
+
+  assert.equal(errors.length, 3);
+
+  await server.close();
+});
+
 function get(url, options) {
   return new Promise((resolve, reject) => {
     try {