Improve url detection in comments

Signed-off-by: Joas Schilling <coding@schilljs.com>

2 files changed, 58 insertions, 9 deletions

diff --git a/core/js/public/comments.js b/core/js/public/comments.js
index 318d527b13d..9811528e4c1 100644
--- a/core/js/public/comments.js
+++ b/core/js/public/comments.js
@@ -21,8 +21,7 @@
 		 * The downside: anything not ascii is excluded. Not sure how common it is in areas using different
 		 * alphabets… the upside: fake domains with similar looking characters won't be formatted as links
 		 */
-		urlRegex: /((\s|^)(https?:\/\/|([-A-Z0-9+_])*\.([-A-Z])+)[-A-Z0-9+&@#\/%?=~_|!:,.;()]*[-A-Z0-9+&@#\/%=~_|()])/ig,
-		protocolRegex: /^https:\/\//,
+		urlRegex: /(\s|^)(https?:\/\/)?((?:[-A-Z0-9+_]*\.)+[-A-Z]+(?:\/[-A-Z0-9+&@#%?=~_|!:,.;()]*)*)(\s|$)/ig,
 
 		plainToRich: function(content) {
 			content = this.formatLinksRich(content);
@@ -35,15 +34,15 @@
 		},
 
 		formatLinksRich: function(content) {
-			var self = this;
-			return content.replace(this.urlRegex, function(url) {
-				var hasProtocol = (url.indexOf('https://') !== -1) || (url.indexOf('http://') !== -1);
-				if(!hasProtocol) {
-					url = 'https://' + url.trim();
+			return content.replace(this.urlRegex, function(_, leadingSpace, protocol, url, trailingSpace) {
+				var linkText = url;
+				if(!protocol) {
+					protocol = 'https://';
+				} else if (protocol === 'http://'){
+					linkText = protocol + url;
 				}
 
-				var linkText = url.replace(self.protocolRegex, '');
-				return '<a class="external" target="_blank" rel="noopener noreferrer" href="' + url + '">' + linkText + '</a>';
+				return leadingSpace + '<a class="external" target="_blank" rel="noopener noreferrer" href="' + protocol + url + '">' + linkText + '</a>' + trailingSpace;
 			});
 		},
 
diff --git a/core/js/tests/specs/public/commentsSpec.js b/core/js/tests/specs/public/commentsSpec.js
new file mode 100644
index 00000000000..57fd7264d25
--- /dev/null
+++ b/core/js/tests/specs/public/commentsSpec.js
@@ -0,0 +1,50 @@
+/**
+* @copyright 2018 Joas Schilling <nickvergessen@owncloud.com>
+*
+* This library is free software; you can redistribute it and/or
+* modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
+* License as published by the Free Software Foundation; either
+* version 3 of the License, or any later version.
+*
+* This library is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+* GNU AFFERO GENERAL PUBLIC LICENSE for more details.
+*
+* You should have received a copy of the GNU Affero General Public
+* License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+*
+*/
+
+describe('OCP.Comments tests', function() {
+	function dataProvider() {
+		return [
+			{input: 'nextcloud.com', expected: '<a class="external" target="_blank" rel="noopener noreferrer" href="https://nextcloud.com">nextcloud.com</a>'},
+			{input: 'http://nextcloud.com', expected: '<a class="external" target="_blank" rel="noopener noreferrer" href="http://nextcloud.com">http://nextcloud.com</a>'},
+			{input: 'https://nextcloud.com', expected: '<a class="external" target="_blank" rel="noopener noreferrer" href="https://nextcloud.com">nextcloud.com</a>'},
+			{input: 'hi nextcloud.com', expected: 'hi <a class="external" target="_blank" rel="noopener noreferrer" href="https://nextcloud.com">nextcloud.com</a>'},
+			{input: 'hi http://nextcloud.com', expected: 'hi <a class="external" target="_blank" rel="noopener noreferrer" href="http://nextcloud.com">http://nextcloud.com</a>'},
+			{input: 'hi https://nextcloud.com', expected: 'hi <a class="external" target="_blank" rel="noopener noreferrer" href="https://nextcloud.com">nextcloud.com</a>'},
+			{input: 'nextcloud.com foobar', expected: '<a class="external" target="_blank" rel="noopener noreferrer" href="https://nextcloud.com">nextcloud.com</a> foobar'},
+			{input: 'http://nextcloud.com foobar', expected: '<a class="external" target="_blank" rel="noopener noreferrer" href="http://nextcloud.com">http://nextcloud.com</a> foobar'},
+			{input: 'https://nextcloud.com foobar', expected: '<a class="external" target="_blank" rel="noopener noreferrer" href="https://nextcloud.com">nextcloud.com</a> foobar'},
+			{input: 'hi nextcloud.com foobar', expected: 'hi <a class="external" target="_blank" rel="noopener noreferrer" href="https://nextcloud.com">nextcloud.com</a> foobar'},
+			{input: 'hi http://nextcloud.com foobar', expected: 'hi <a class="external" target="_blank" rel="noopener noreferrer" href="http://nextcloud.com">http://nextcloud.com</a> foobar'},
+			{input: 'hi https://nextcloud.com foobar', expected: 'hi <a class="external" target="_blank" rel="noopener noreferrer" href="https://nextcloud.com">nextcloud.com</a> foobar'},
+			{input: 'hi help.nextcloud.com/category/topic foobar', expected: 'hi <a class="external" target="_blank" rel="noopener noreferrer" href="https://help.nextcloud.com/category/topic">help.nextcloud.com/category/topic</a> foobar'},
+			{input: 'hi http://help.nextcloud.com/category/topic foobar', expected: 'hi <a class="external" target="_blank" rel="noopener noreferrer" href="http://help.nextcloud.com/category/topic">http://help.nextcloud.com/category/topic</a> foobar'},
+			{input: 'hi https://help.nextcloud.com/category/topic foobar', expected: 'hi <a class="external" target="_blank" rel="noopener noreferrer" href="https://help.nextcloud.com/category/topic">help.nextcloud.com/category/topic</a> foobar'},
+			{input: 'noreply@nextcloud.com', expected: 'noreply@nextcloud.com'},
+			{input: 'hi noreply@nextcloud.com', expected: 'hi noreply@nextcloud.com'},
+			{input: 'hi <noreply@nextcloud.com>', expected: 'hi <noreply@nextcloud.com>'},
+			{input: 'FirebaseInstanceId.getInstance().deleteInstanceId()', expected: 'FirebaseInstanceId.getInstance().deleteInstanceId()'},
+		];
+	}
+
+	it('should parse URLs only', function () {
+		dataProvider().forEach(function(data) {
+			var result = OCP.Comments.plainToRich(data.input);
+			expect(result).toEqual(data.expected);
+		});
+	});
+});