Fix issue where closing square brackets could be included in a URL when the URL was simply wrapped in square brackets

Fixes #228

Author: gregjacobs

src/matcher/url-matcher.ts

@@ -126,32 +126,6 @@ export class UrlMatcher extends Matcher {
 	protected wordCharRegExp = new RegExp( '[' + alphaNumericAndMarksCharsStr + ']' );
 
 
-	/**
-	 * The regular expression to match opening parenthesis in a URL match.
-	 *
-	 * This is to determine if we have unbalanced parenthesis in the URL, and to
-	 * drop the final parenthesis that was matched if so.
-	 *
-	 * Ex: The text "(check out: wikipedia.com/something_(disambiguation))"
-	 * should only autolink the inner "wikipedia.com/something_(disambiguation)"
-	 * part, so if we find that we have unbalanced parenthesis, we will drop the
-	 * last one for the match.
-	 *
-	 * @protected
-	 * @property {RegExp}
-	 */
-	protected openParensRe = /\(/g;
-
-	/**
-	 * The regular expression to match closing parenthesis in a URL match. See
-	 * {@link #openParensRe} for more information.
-	 *
-	 * @protected
-	 * @property {RegExp}
-	 */
-	protected closeParensRe = /\)/g;
-
-
 	/**
 	 * @method constructor
 	 * @param {Object} cfg The configuration properties for the Match instance,
@@ -207,13 +181,16 @@ export class UrlMatcher extends Matcher {
 				continue;
 			}
 
-			if( /\?$/.test(matchStr) ) {
-				matchStr = matchStr.substr(0, matchStr.length-1);
+			// If the URL ends with a question mark, don't include the question
+			// mark as part of the URL. We'll assume the question mark was the
+			// end of a sentence, such as: "Going to google.com?"
+			if( /\?$/.test( matchStr ) ) {
+				matchStr = matchStr.substr( 0, matchStr.length-1 );
 			}
 
-			// Handle a closing parenthesis at the end of the match, and exclude
-			// it if there is not a matching open parenthesis in the match
-			// itself.
+			// Handle a closing parenthesis or square bracket at the end of the 
+			// match, and exclude it if there is not a matching open parenthesis 
+			// or square bracket in the match itself.
 			if( this.matchHasUnbalancedClosingParen( matchStr ) ) {
 				matchStr = matchStr.substr( 0, matchStr.length - 1 );  // remove the trailing ")"
 			} else {
@@ -246,38 +223,66 @@ export class UrlMatcher extends Matcher {
 
 
 	/**
-	 * Determines if a match found has an unmatched closing parenthesis. If so,
-	 * this parenthesis will be removed from the match itself, and appended
-	 * after the generated anchor tag.
+	 * Determines if a match found has an unmatched closing parenthesis or 
+	 * square bracket. If so, the parenthesis or square bracket will be removed 
+	 * from the match itself, and appended after the generated anchor tag.
 	 *
 	 * A match may have an extra closing parenthesis at the end of the match
 	 * because the regular expression must include parenthesis for URLs such as
 	 * "wikipedia.com/something_(disambiguation)", which should be auto-linked.
 	 *
 	 * However, an extra parenthesis *will* be included when the URL itself is
-	 * wrapped in parenthesis, such as in the case of "(wikipedia.com/something_(disambiguation))".
+	 * wrapped in parenthesis, such as in the case of: 
+	 *     "(wikipedia.com/something_(disambiguation))"
 	 * In this case, the last closing parenthesis should *not* be part of the
-	 * URL itself, and this method will return `true`.
+	 * URL itself, and this method will return `true`. 
+	 * 
+	 * For square brackets in URLs such as in PHP arrays, the same behavior as 
+	 * parenthesis discussed above should happen:
+	 *     "[http://www.example.com/foo.php?bar[]=1&bar[]=2&bar[]=3]"
+	 * The closing square bracket should not be part of the URL itself, and this
+	 * method will return `true`.
 	 *
 	 * @protected
 	 * @param {String} matchStr The full match string from the {@link #matcherRegex}.
-	 * @return {Boolean} `true` if there is an unbalanced closing parenthesis at
-	 *   the end of the `matchStr`, `false` otherwise.
+	 * @return {Boolean} `true` if there is an unbalanced closing parenthesis or
+	 *   square bracket at the end of the `matchStr`, `false` otherwise.
 	 */
-	protected matchHasUnbalancedClosingParen( matchStr: string ) {
-		let lastChar = matchStr.charAt( matchStr.length - 1 );
-
-		if( lastChar === ')' ) {
-			let openParensMatch = matchStr.match( this.openParensRe ),
-			    closeParensMatch = matchStr.match( this.closeParensRe ),
-			    numOpenParens = ( openParensMatch && openParensMatch.length ) || 0,
-			    numCloseParens = ( closeParensMatch && closeParensMatch.length ) || 0;
+	protected matchHasUnbalancedClosingParen( matchStr: string ): boolean {
+		let endChar = matchStr.charAt( matchStr.length - 1 );
+		let startChar: string;
+
+		if( endChar === ')' ) {
+			startChar = '(';
+		} else if( endChar === ']' ) {
+			startChar = '[';
+		} else {
+			return false;  // not a close parenthesis or square bracket
+		}
 
-			if( numOpenParens < numCloseParens ) {
-				return true;
+		// Find if there are the same number of open braces as close braces in
+		// the URL string, minus the last character (which we have already 
+		// determined to be either ')' or ']'
+		let numOpenBraces = 0;
+		for( let i = 0, len = matchStr.length - 1; i < len; i++ ) {
+			const char = matchStr.charAt( i );
+
+			if( char === startChar ) {
+				numOpenBraces++;
+			} else if( char === endChar ) {
+				numOpenBraces = Math.max( numOpenBraces - 1, 0 );
 			}
 		}
 
+		// If the number of open braces matches the number of close braces in
+		// the URL minus the last character, then the match has *unbalanced*
+		// braces because of the last character. Example of unbalanced braces
+		// from the regex match:
+		//     "http://example.com?a[]=1]"
+		if( numOpenBraces === 0 ) {
+			return true;
+		}
+
 		return false;
 	}
 

tests/autolinker-mention.spec.ts

@@ -86,7 +86,7 @@ describe( "Autolinker Mention Matching -", () => {
 
 
 			it( `should automatically link ${serviceName} handles surrounded by 
-				 braces`, 
+			     curly brackets`, 
 			() => {
 				let result = autolinker.link( `Joe's ${serviceName} is {@joe_the_man12}` );
 
@@ -95,7 +95,7 @@ describe( "Autolinker Mention Matching -", () => {
 
 
 			it( `should automatically link ${serviceName} handles surrounded by 
-				 brackets`, 
+				 square brackets`, 
 			() => {
 				let result = autolinker.link( `Joe's ${serviceName} is [@joe_the_man12]` );
 

tests/autolinker-url.spec.ts

@@ -679,6 +679,77 @@ describe( "Autolinker Url Matching -", () => {
 	} );
 
 
+	describe( "square bracket handling", () => {
+
+		it( `when the url is surrounded by square brackets, it should not include 
+			 should not include the final closing bracket in the URL`,
+		() => {
+			let result = autolinker.link( "Click here [google.com] for more details" );
+			expect( result ).toBe( 'Click here [<a href="http://google.com">google.com</a>] for more details' );
+		} );
+
+		
+		it( `when the URL starts with a scheme, and is surrounded by square
+			  brackets, should not include the final closing bracket in the URL
+			  (Issue #228)`,
+		() => {
+			let result = autolinker.link( "Click here [http://example.com] for more details" );
+			expect( result ).toBe( 'Click here [<a href="http://example.com">example.com</a>] for more details' );
+		} );
+
+		
+		it( `when the URL ends with a closing square bracket, but there is no 
+			  matching open square bracket, should not include the final closing 
+			  bracket in the URL (Issue #228)`,
+		() => {
+			let result = autolinker.link( "Click here [cat http://example.com] for more details" );
+			expect( result ).toBe( 'Click here [cat <a href="http://example.com">example.com</a>] for more details' );
+		} );
+
+
+		it( "should not include a final closing bracket in the URL when a path exists", function() {
+			let result = autolinker.link( "Click here [google.com/abc] for more details" );
+			expect( result ).toBe( 'Click here [<a href="http://google.com/abc">google.com/abc</a>] for more details' );
+		} );
+
+
+		it( "should not include a final closing bracket in the URL when a query string exists", function() {
+			let result = autolinker.link( "Click here [google.com?abc=1] for more details" );
+			expect( result ).toBe( 'Click here [<a href="http://google.com?abc=1">google.com?abc=1</a>] for more details' );
+		} );
+
+
+		it( "should not include a final closing bracket in the URL when a hash anchor exists", function() {
+			let result = autolinker.link( "Click here [google.com#abc] for more details" );
+			expect( result ).toBe( 'Click here [<a href="http://google.com#abc">google.com#abc</a>] for more details' );
+		} );
+
+
+		it( "should include escaped brackets in the URL", function() {
+			let result = autolinker.link( "Here's an example from CodingHorror: http://en.wikipedia.org/wiki/PC_Tools_%5BCentral_Point_Software%5D" );
+			expect( result ).toBe( 'Here\'s an example from CodingHorror: <a href="http://en.wikipedia.org/wiki/PC_Tools_%5BCentral_Point_Software%5D">en.wikipedia.org/wiki/PC_Tools_[Central_Point_Software]</a>' );
+		} );
+
+
+		it( `should correctly accept square brackets such as PHP array 
+		     representation in query strings`,
+		() => {
+			let result = autolinker.link( "Here's an example: http://www.example.com/foo.php?bar[]=1&bar[]=2&bar[]=3" );
+			expect( result ).toBe( `Here's an example: <a href="http://www.example.com/foo.php?bar[]=1&bar[]=2&bar[]=3">example.com/foo.php?bar[]=1&bar[]=2&bar[]=3</a>` );
+		} );
+
+
+		it( `should correctly accept square brackets such as PHP array 
+			 representation in query strings, when the entire URL is surrounded
+			 by square brackets`,
+		() => {
+			let result = autolinker.link( "Here's an example: [http://www.example.com/foo.php?bar[]=1&bar[]=2&bar[]=3]" );
+			expect( result ).toBe( `Here's an example: [<a href="http://www.example.com/foo.php?bar[]=1&bar[]=2&bar[]=3">example.com/foo.php?bar[]=1&bar[]=2&bar[]=3</a>]` );
+		} );
+
+	} );
+
+
 	describe( "Special character handling", function() {
 
 		it( "should include $ in URLs", function() {