pool: stricter rules for custom user agent

integration/test-util.js

@@ -2,6 +2,7 @@
 'use strict';
 
 const {spawn, execSync} = require('child_process');
+const {EventEmitter} = require('events');
 const assert = require('bsert');
 const path = require('path');
 const {FullNode, packets} = require('hsd');
@@ -12,8 +13,10 @@ const {version} = require('./package.json');
 const network = 'regtest';
 const hnsdPath = path.join(__dirname, '..', 'hnsd');
 
-class TestUtil {
+class TestUtil extends EventEmitter {
   constructor() {
+    super();
+
     this.host = '127.0.0.1';
     this.port = 10000;
 
@@ -25,7 +28,8 @@ class TestUtil {
       port: this.port,
       brontidePort: 46888, // avoid hnsd connecting via brontide
       noDns: true,
-      plugins: [require('hsd/lib/wallet/plugin')]
+      plugins: [require('hsd/lib/wallet/plugin')],
+      // consoleLog: true, logLevel: 'spam'
     });
 
     // Packets received by full node from hnsd
@@ -95,10 +99,13 @@ class TestUtil {
 
   async close() {
     await this.node.close();
-    this.closeHNSD();
+    await this.closeHNSD();
   }
 
   async openHNSD() {
+    if (this.hnsd)
+      throw new Error('hnsd already open');
+
     return new Promise((resolve, reject) => {
       this.hnsd = spawn(
         path.join(__dirname, '..', 'hnsd'),
@@ -108,7 +115,10 @@ class TestUtil {
 
       this.hnsd.on('spawn', () => resolve());
       this.hnsd.on('error', () => reject());
-      this.hnsd.on('close', this.crash);
+
+      this.hnsd.on('close', (code) => {
+        this.emit('close', code);
+      });
 
       this.message = '';
       this.hnsd.stderr.on('data', (data) => {
@@ -120,28 +130,41 @@ class TestUtil {
 
         const msg = this.message;
         this.message = '';
-        console.log(msg);
-        throw new Error(msg);
+        console.log(msg);         // print memory leak errors to console
+        this.emit('stderr', msg); // for expected error message tests
       });
     });
   }
 
-  crash(code, signal) {
-    throw new Error(`hnsd crashed with code: ${code} and signal: ${signal}`);
-  }
-
-  closeHNSD() {
+  async closeHNSD() {
     if (!this.hnsd)
       return;
 
-    this.hnsd.removeListener('close', this.crash);
+    if (this.hnsd.exitCode != null) {
+      this.hnsd = null;
+      return;
+    }
+
+    const waiter = new Promise((resolve, reject) => {
+      this.hnsd.once('close', (code, signal) => {
+        this.hnsd = null;
+
+        if (code) {
+          reject(new Error(
+            `hnsd closed with exit code: ${code}, signal: ${signal}`
+          ));
+        }
+
+        resolve();
+      });
+    });
 
     this.hnsd.kill('SIGINT');
-    this.hnsd = null;
+    await waiter;
   }
 
   async restartHNSD(args) {
-    this.closeHNSD();
+    await this.closeHNSD();
 
     if (args) {
       assert(Array.isArray(args));
@@ -182,6 +205,9 @@ class TestUtil {
   }
 
   async resolveHS(name) {
+    if (!this.hnsd || this.hnsd.exitCode != null)
+      throw new Error('hnsd is closed');
+
     const qs = wire.Question.fromJSON({
       name,
       class: 'HS',

integration/test/config-test.js

@@ -0,0 +1,112 @@
+/* eslint-env mocha */
+/* eslint prefer-arrow-callback: "off" */
+/* eslint max-len: "off" */
+/* eslint no-return-assign: "off" */
+
+'use strict';
+
+const assert = require('bsert');
+const {version} = require('../package.json');
+
+const TestUtil = require('../test-util');
+const util = new TestUtil();
+
+describe('Configuration', function() {
+  this.timeout(10000);
+
+  before(async () => {
+    await util.open();
+  });
+
+  after(async () => {
+    await util.close();
+  });
+
+  function waitForStartFailure(expectedMsg, expectedCode) {
+    return new Promise((resolve, reject) => {
+      let gotMsg = false;
+      let gotCode = false;
+      let error = null;
+
+      function maybeResolve() {
+        if (!gotMsg || !gotCode)
+          return;
+
+        if (error)
+          reject(error);
+        else
+          resolve();
+      }
+
+      function fail(actual, expected) {
+        error = new assert.AssertionError({
+          message: 'hnsd start failure with unexpected output',
+          actual,
+          expected,
+          operator: 'equal'
+        });
+      }
+
+      function handleErr(msg) {
+        if (!msg.match(expectedMsg))
+          fail(msg, expectedMsg);
+        gotMsg = true;
+        maybeResolve();
+      }
+
+      function handleClose(code) {
+        if (code !== expectedCode)
+          fail(code, expectedCode);
+        gotCode = true;
+        maybeResolve();
+      }
+
+      util.once('stderr', handleErr);
+      util.once('close', handleClose);
+    });
+  }
+
+  describe('--agent', function () {
+    it('should have default user agent', async () => {
+      await util.generate(1);
+      await util.waitForSync();
+
+      const peer = util.node.pool.peers.head();
+      assert.strictEqual(peer.agent, `/hnsd:${version}/`);
+      await util.closeHNSD();
+    });
+
+    it('should fail to start with too-long agent', async () => {
+      const waiter = waitForStartFailure(
+        /failed adding user agent/,
+        3 // HSK_EBADARGS
+      );
+
+      const agent = 'x'.repeat(255 - `/hnsd:${version}/`.length);
+      await util.restartHNSD(['-a', agent]);
+      await waiter;
+    });
+
+    it('should fail to start with backslash-containing agent', async () => {
+      const waiter = waitForStartFailure(
+        /failed adding user agent/,
+        3 // HSK_EBADARGS
+      );
+
+      const agent = 'beacon/browser/verifies/dane';
+      await util.restartHNSD(['-a', agent]);
+      await waiter;
+    });
+
+    it('should have custom user agent', async () => {
+      const agent = 'x'.repeat(255 - `/hnsd:${version}/`.length - 1);
+      await util.restartHNSD(['-a', agent]);
+      await util.generate(1);
+      await util.waitForSync();
+
+      const peer = util.node.pool.peers.head();
+      assert.strictEqual(peer.agent, `/hnsd:${version}/${agent}/`);
+      await util.closeHNSD();
+    });
+  });
+});

src/pool.c

@@ -302,14 +302,19 @@ hsk_pool_set_agent(hsk_pool_t *pool, const char *user_agent) {
   if (!user_agent)
     return true;
 
+  if (strchr(user_agent, '/'))
+    return false;
+
   size_t len = strlen(pool->user_agent);
   len += strlen(user_agent);
+  len += 1; // terminal "/"
 
   // Agent size in p2p version message is 1 byte
-  if (len > 0xff)
+  if (len > HSK_MAX_AGENT)
     return false;
 
   pool->user_agent = strcat(pool->user_agent, user_agent);
+  pool->user_agent = strcat(pool->user_agent, "/");
 
   return true;
 }

src/pool.h

@@ -27,6 +27,7 @@
 #define HSK_STATE_READING 4
 #define HSK_STATE_HANDSHAKE 5
 #define HSK_STATE_DISCONNECTING 6
+#define HSK_MAX_AGENT 255
 
 /*
  * Types
@@ -59,7 +60,7 @@ typedef struct hsk_peer_s {
   hsk_brontide_t *brontide;
   uint64_t id;
   char host[HSK_MAX_HOST];
-  char agent[255];
+  char agent[HSK_MAX_AGENT];
   hsk_addr_t addr;
   int state;
   uint8_t read_buffer[HSK_BUFFER_SIZE];