From 89174178c6225fc3b4d9b9d96a88edbbe862f1a1 Mon Sep 17 00:00:00 2001
From: Gustavo De Micheli <gustavo.de.micheli@lunatech.com>
Date: Tue, 25 Jul 2017 13:30:00 +0200
Subject: [PATCH] #310 - Allow for topic/patterns with '#' and end of line
 comments

---
 .../spi/impl/security/ACLFileParser.java      | 38 +++++++++++--------
 .../spi/impl/security/ACLFileParserTest.java  | 36 +++++++++++++++++-
 2 files changed, 56 insertions(+), 18 deletions(-)

diff --git a/broker/src/main/java/io/moquette/spi/impl/security/ACLFileParser.java b/broker/src/main/java/io/moquette/spi/impl/security/ACLFileParser.java
index 2121dbb3..5081af54 100644
--- a/broker/src/main/java/io/moquette/spi/impl/security/ACLFileParser.java
+++ b/broker/src/main/java/io/moquette/spi/impl/security/ACLFileParser.java
@@ -20,6 +20,8 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import java.io.*;
 import java.text.ParseException;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
  * Parses the acl configuration file. If a line starts with # it's comment. Blank lines are skipped.
@@ -83,25 +85,29 @@ public final class ACLFileParser {
         String line;
         AuthorizationsCollector collector = new AuthorizationsCollector();
 
+        Pattern emptyLine = Pattern.compile("^\\s*$");
+        Pattern commentLine = Pattern.compile("^#.*"); // As spec, comment lines should start with '#'
+        Pattern invalidCommentLine = Pattern.compile("^\\s*#.*");
+        Pattern endLineComment = Pattern.compile("^([\\w\\s\\/\\+]+#?)(\\s*#.*)$"); // This pattern has a dependency on filtering `commentLine`.
+        Matcher endLineCommentMatcher;
+
         try {
             while ((line = br.readLine()) != null) {
-                int commentMarker = line.indexOf('#');
-                if (commentMarker != -1) {
-                    if (commentMarker == 0) {
-                        // skip its a comment
-                        continue;
-                    } else {
-                        // it's a malformed comment
-                        throw new ParseException(line, commentMarker);
-                    }
-                } else {
-                    if (line.isEmpty() || line.matches("^\\s*$")) {
-                        // skip it's a black line
-                        continue;
-                    }
-
-                    collector.parse(line);
+                if (line.isEmpty() || emptyLine.matcher(line).matches() || commentLine.matcher(line).matches()) {
+                    // skip it's a black line or comment
+                    continue;
+                } else if (invalidCommentLine.matcher(line).matches()) {
+                    // it's a malformed comment
+                    int commentMarker = line.indexOf('#');
+                    throw new ParseException(line, commentMarker);
                 }
+
+                endLineCommentMatcher = endLineComment.matcher(line);
+                if (endLineCommentMatcher.matches()) {
+                    line = endLineCommentMatcher.group(1);
+                }
+
+                collector.parse(line);
             }
         } catch (IOException ex) {
             throw new ParseException("Failed to read", 1);
diff --git a/broker/src/test/java/io/moquette/spi/impl/security/ACLFileParserTest.java b/broker/src/test/java/io/moquette/spi/impl/security/ACLFileParserTest.java
index b677f744..8185855f 100644
--- a/broker/src/test/java/io/moquette/spi/impl/security/ACLFileParserTest.java
+++ b/broker/src/test/java/io/moquette/spi/impl/security/ACLFileParserTest.java
@@ -44,9 +44,12 @@ public class ACLFileParserTest {
     }
 
     @Test(expected = ParseException.class)
-    public void testParseInvalidComment() throws ParseException {
+    public void testParseInvalidPaddedComment() throws ParseException {
         Reader conf = new StringReader(" #simple comment");
-        ACLFileParser.parse(conf);
+        AuthorizationsCollector authorizations = ACLFileParser.parse(conf);
+
+        // Verify
+        assertTrue(authorizations.isEmpty());
     }
 
     @Test
@@ -59,4 +62,33 @@ public class ACLFileParserTest {
         assertTrue(authorizations.canWrite(new Topic("/weather/italy/anemometer"), "", ""));
     }
 
+    @Test
+    public void testParseValidEndLineComment() throws ParseException {
+        Reader conf = new StringReader("topic /weather/italy/anemometer #simple comment");
+        AuthorizationsCollector authorizations = ACLFileParser.parse(conf);
+
+        // Verify
+        assertTrue(authorizations.canRead(new Topic("/weather/italy/anemometer"), "", ""));
+        assertTrue(authorizations.canWrite(new Topic("/weather/italy/anemometer"), "", ""));
+    }
+
+    @Test
+    public void testParseValidPoundTopicWithEndLineComment() throws ParseException {
+        Reader conf = new StringReader("topic /weather/italy/anemometer/# #simple comment");
+        AuthorizationsCollector authorizations = ACLFileParser.parse(conf);
+
+        // Verify
+        assertTrue(authorizations.canRead(new Topic("/weather/italy/anemometer/#"), "", ""));
+        assertTrue(authorizations.canWrite(new Topic("/weather/italy/anemometer/#"), "", ""));
+    }
+
+    @Test
+    public void testParseValidPlusTopicWithEndLineComment() throws ParseException {
+        Reader conf = new StringReader("topic /weather/+/anemometer #simple comment");
+        AuthorizationsCollector authorizations = ACLFileParser.parse(conf);
+
+        // Verify
+        assertTrue(authorizations.canRead(new Topic("/weather/+/anemometer"), "", ""));
+        assertTrue(authorizations.canWrite(new Topic("/weather/+/anemometer"), "", ""));
+    }
 }