refactor: ip blacklist.test

also dont call ipaddr.parse if cidr rules is empty
add a test for cidr

src/meta/blacklist.js

@@ -44,48 +44,49 @@ Blacklist.get = async function () {
 };
 
 Blacklist.test = async function (clientIp) {
-	// Some handy test addresses
-	// clientIp = '2001:db8:85a3:0:0:8a2e:370:7334'; // IPv6
-	// clientIp = '127.0.15.1'; // IPv4
-	// clientIp = '127.0.15.1:3443'; // IPv4 with port strip port to not fail
 	if (!clientIp) {
 		return;
 	}
 	clientIp = clientIp.split(':').length === 2 ? clientIp.split(':')[0] : clientIp;
 
+	const rules = Blacklist._rules;
+	function checkCidrRange(clientIP) {
+		if (!rules.cidr.length) {
+			return false;
+		}
 		let addr;
 		try {
-		addr = ipaddr.parse(clientIp);
+			addr = ipaddr.parse(clientIP);
 		} catch (err) {
 			winston.error(`[meta/blacklist] Error parsing client IP : ${clientIp}`);
 			throw err;
 		}
-
+		return rules.cidr.some((subnet) => {
-	if (
-		!Blacklist._rules.ipv4.includes(clientIp) && // not explicitly specified in ipv4 list
-		!Blacklist._rules.ipv6.includes(clientIp) && // not explicitly specified in ipv6 list
-		!Blacklist._rules.cidr.some((subnet) => {
 			const cidr = ipaddr.parseCIDR(subnet);
 			if (addr.kind() !== cidr[0].kind()) {
 				return false;
 			}
 			return addr.match(cidr);
-		}) // not in a blacklisted IPv4 or IPv6 cidr range
+		});
-	) {
-		try {
-			// To return test failure, pass back an error in callback
-			await plugins.hooks.fire('filter:blacklist.test', { ip: clientIp });
-		} catch (err) {
-			analytics.increment('blacklist');
-			throw err;
 	}
-	} else {
+
+	if (rules.ipv4.includes(clientIp) ||
+		rules.ipv6.includes(clientIp) ||
+		checkCidrRange(clientIp)) {
 		const err = new Error('[[error:blacklisted-ip]]');
 		err.code = 'blacklisted-ip';
 
 		analytics.increment('blacklist');
 		throw err;
 	}
+
+	try {
+		// To return test failure, throw an error in hook
+		await plugins.hooks.fire('filter:blacklist.test', { ip: clientIp });
+	} catch (err) {
+		analytics.increment('blacklist');
+		throw err;
+	}
 };
 
 Blacklist.validate = function (rules) {

test/blacklist.js

@@ -59,10 +59,24 @@ describe('blacklist', () => {
 		});
 	});
 
+	it('should fail ip test against blacklist with port', (done) => {
+		blacklist.test('1.1.1.1:4567', (err) => {
+			assert.equal(err.message, '[[error:blacklisted-ip]]');
+			done();
+		});
+	});
+
 	it('should pass ip test and not crash with ipv6 address', (done) => {
 		blacklist.test('2001:db8:85a3:0:0:8a2e:370:7334', (err) => {
 			assert.ifError(err);
 			done();
 		});
 	});
+
+	it('should fail ip test due to cidr', (done) => {
+		blacklist.test('192.168.100.1', (err) => {
+			assert.equal(err.message, '[[error:blacklisted-ip]]');
+			done();
+		});
+	});
 });