From 314b495f560a2da9713b6f4a440ee3b99749263a Mon Sep 17 00:00:00 2001
From: Matt Broadstone <mbroadst@gmail.com>
Date: Wed, 24 Aug 2016 13:10:32 -0400
Subject: [PATCH] fix(defaultSubjects): throw error on invalid subjects

Its possible (and perhaps very likely) for default subjects to be
specified using a variable which might be undefined, and therefore
coerced to a string value as such. As a result, the user will
likely be using an unexpected link unknowingly. Instead, we will
now throw an error on these invalid cases.
---
 lib/amqp_client.js                   |  8 ++++++++
 lib/errors.js                        | 13 +++++++++++++
 test/integration/qpid/client.test.js | 24 ++++++++++++++++++++----
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/lib/amqp_client.js b/lib/amqp_client.js
index a1fa285..69dba0c 100644
--- a/lib/amqp_client.js
+++ b/lib/amqp_client.js
@@ -176,6 +176,10 @@ AMQPClient.prototype.createSender = function(address, policyOverrides) {
       }, policyOverrides, this.policy.senderLink);
 
   if (!!address.subject && this.policy.defaultSubjects) {
+    if (address.subject === 'undefined' || address.subject === 'null') {
+      throw new errors.InvalidSubjectError(address.subject);
+    }
+
     linkPolicy.defaultSubject = address.subject;
   }
 
@@ -241,6 +245,10 @@ AMQPClient.prototype.createReceiver = function(address, policyOverrides) {
   // if the policy supports the defaultSubjects feature, and a subject has been
   // provided then automatically set up a filter to match on that subject.
   if (this.policy.defaultSubjects && !!address.subject) {
+    if (address.subject === 'undefined' || address.subject === 'null') {
+      throw new errors.InvalidSubjectError(address.subject);
+    }
+
     var filterSymbol = (address.subject.indexOf('*') || address.subject.indexOf('#')) ?
       'apache.org:legacy-amqp-topic-binding:string' :
       'apache.org:legacy-amqp-direct-binding:string';
diff --git a/lib/errors.js b/lib/errors.js
index 29f93f1..6041678 100644
--- a/lib/errors.js
+++ b/lib/errors.js
@@ -176,3 +176,16 @@ errors.VersionError = function(msg) {
   this.name = 'AmqpVersionError';
 };
 util.inherits(errors.VersionError, errors.BaseError);
+
+/**
+ * Invalid subject specified for receiver or sender link creation.
+ *
+ * @param subject the subject specified
+ * @extends BaseError
+ * @constructor
+ */
+errors.InvalidSubjectError = function(subject) {
+  errors.BaseError.call(this, 'Invalid subject: ' + subject);
+  this.name = 'AmqpInvalidSubjectError';
+};
+util.inherits(errors.InvalidSubjectError, errors.BaseError);
diff --git a/test/integration/qpid/client.test.js b/test/integration/qpid/client.test.js
index 2ec869f..51a2982 100644
--- a/test/integration/qpid/client.test.js
+++ b/test/integration/qpid/client.test.js
@@ -1,13 +1,15 @@
 'use strict';
 var Promise = require('bluebird'),
-    AMQPClient = require('../../..').Client,
-    Policy = require('../../..').Policy,
+    amqp = require('../../..'),
+    AMQPClient = amqp.Client,
+    Policy = amqp.Policy,
+    Errors = amqp.Errors,
     config = require('./config'),
-    expect = require('chai').expect;
+    chai = require('chai'),
+    expect = chai.expect;
 
 var test = {};
 describe('QPID', function() {
-
 describe('Client', function() {
   beforeEach(function() {
     if (!!test.client) test.client = undefined;
@@ -246,5 +248,19 @@ describe('Client', function() {
       });
   });
 
+  it('should throw an error when `undefined` or `null` are used for default subjects', function() {
+    return test.client.connect(config.address)
+      .then(function() {
+        [ null, undefined ].forEach(function(value) {
+          expect(function() {
+            test.client.createReceiver(config.defaultLink + '/' + value);
+          }).to.throw(Errors.InvalidSubjectError);
+          expect(function() {
+            test.client.createSender(config.defaultLink + '/' + value);
+          }).to.throw(Errors.InvalidSubjectError);
+        });
+      });
+  });
+
 });
 });