From d2e9e47b673f272772a8c2c0ca6736eca083050c Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sat, 29 Feb 2020 08:45:56 +0100 Subject: [PATCH 1/6] refactor switch into nested if/else, to improve readability - no functional change --- js/privatebin.js | 78 ++++++++++++++++++++++++----------------------- tpl/bootstrap.php | 2 +- tpl/page.php | 2 +- 3 files changed, 42 insertions(+), 40 deletions(-) diff --git a/js/privatebin.js b/js/privatebin.js index e76bf98c..d2b85448 100644 --- a/js/privatebin.js +++ b/js/privatebin.js @@ -2422,52 +2422,54 @@ jQuery.PrivateBin = (function($, RawDeflate) { return; } - // escape HTML entities, link URLs, sanitize - const escapedLinkedText = Helper.urls2links(text), - sanitizedLinkedText = DOMPurify.sanitize( - escapedLinkedText, { - ALLOWED_TAGS: ['a'], - ALLOWED_ATTR: ['href', 'rel'] - } - ); - $plainText.html(sanitizedLinkedText); - $prettyPrint.html(sanitizedLinkedText); + if (format === 'markdown') { + const converter = new showdown.Converter({ + strikethrough: true, + tables: true, + tablesHeaderId: true, + simplifiedAutoLink: true, + excludeTrailingPunctuationFromURLs: true + }); + // let showdown convert the HTML and sanitize HTML *afterwards*! + $plainText.html( + DOMPurify.sanitize( + converter.makeHtml(text) + ) + ); + // add table classes from bootstrap css + $plainText.find('table').addClass('table-condensed table-bordered'); + } else { + // escape HTML entities, link URLs, sanitize + const escapedLinkedText = Helper.urls2links(text); + let sanitizeLinkedText = '', + sanitizerConfiguration = {}; - switch (format) { - case 'markdown': - const converter = new showdown.Converter({ - strikethrough: true, - tables: true, - tablesHeaderId: true, - simplifiedAutoLink: true, - excludeTrailingPunctuationFromURLs: true - }); - // let showdown convert the HTML and sanitize HTML *afterwards*! - $plainText.html( - DOMPurify.sanitize( - converter.makeHtml(text) - ) - ); - // add table classes from bootstrap css - $plainText.find('table').addClass('table-condensed table-bordered'); - break; - case 'syntaxhighlighting': + if (format === 'syntaxhighlighting') { // yes, this is really needed to initialize the environment if (typeof prettyPrint === 'function') { prettyPrint(); } - $prettyPrint.html( - DOMPurify.sanitize( - prettyPrintOne(escapedLinkedText, null, true) - ) + sanitizeLinkedText = prettyPrintOne( + escapedLinkedText, null, true ); - // fall through, as the rest is the same - default: // = 'plaintext' - $prettyPrint.css('white-space', 'pre-wrap'); - $prettyPrint.css('word-break', 'normal'); - $prettyPrint.removeClass('prettyprint'); + } else { + // = 'plaintext' + sanitizeLinkedText = escapedLinkedText; + sanitizerConfiguration = { + ALLOWED_TAGS: ['a'], + ALLOWED_ATTR: ['href', 'rel'] + }; + } + $prettyPrint.html( + DOMPurify.sanitize( + sanitizeLinkedText, sanitizerConfiguration + ) + ); + $prettyPrint.css('white-space', 'pre-wrap'); + $prettyPrint.css('word-break', 'normal'); + $prettyPrint.removeClass('prettyprint'); } } diff --git a/tpl/bootstrap.php b/tpl/bootstrap.php index faaa9779..843dc6a7 100644 --- a/tpl/bootstrap.php +++ b/tpl/bootstrap.php @@ -72,7 +72,7 @@ endif; ?> - + diff --git a/tpl/page.php b/tpl/page.php index 8dc9c0d9..c976ce2f 100644 --- a/tpl/page.php +++ b/tpl/page.php @@ -50,7 +50,7 @@ endif; ?> - + From 5340f417e07fdfdab2f91f5171f4f992bc763a73 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Sat, 29 Feb 2020 09:37:54 +0100 Subject: [PATCH 2/6] in Helper.urls2links(), encode HTML entities, find and insert links, partially decoding only the href property of it --- js/privatebin.js | 23 +++++++++++++++++++---- js/test/Helper.js | 11 ++++------- tpl/bootstrap.php | 2 +- tpl/page.php | 2 +- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/js/privatebin.js b/js/privatebin.js index d2b85448..9e2b0ee2 100644 --- a/js/privatebin.js +++ b/js/privatebin.js @@ -297,10 +297,25 @@ jQuery.PrivateBin = (function($, RawDeflate) { */ me.urls2links = function(html) { - return html.replace( - /(((https?|ftp):\/\/[\w?!=&.\/-;#@~%+*-]+(?![\w\s?!&.\/;#~%"=-]*>))|((magnet):[\w?=&.\/-;#@~%+*-]+))/ig, - '$1' - ); + let reverseEntityMap = {}; + for (let entity of ['&', '"', '/', '=']) { + reverseEntityMap[entityMap[entity]] = entity; + } + const entityRegex = new RegExp(Object.keys(reverseEntityMap).join('|'), 'g'); + + // encode HTML entities, find and insert links, partially decoding only the href property of it + return me.htmlEntities(html) + .replace( + /(((https?|ftp)://[\w?!&.-;#@~%+*-]+(?![\w\s?!&.;#~%-]*>))|((magnet):[\w?&.-;#@~%+*-]+))/ig, + function(encodedUrl) { + let decodedUrl = encodedUrl.replace( + entityRegex, function(entity) { + return reverseEntityMap[entity]; + } + ); + return '' + encodedUrl + ''; + } + ) }; /** diff --git a/js/test/Helper.js b/js/test/Helper.js index dd38e3c4..a884eee2 100644 --- a/js/test/Helper.js +++ b/js/test/Helper.js @@ -81,7 +81,7 @@ describe('Helper', function () { 'ignores non-URL content', 'string', function (content) { - return content === $.PrivateBin.Helper.urls2links(content); + return $.PrivateBin.Helper.htmlEntities(content) === $.PrivateBin.Helper.urls2links(content); } ); jsc.property( @@ -95,8 +95,7 @@ describe('Helper', function () { function (prefix, schema, address, query, fragment, postfix) { query = query.join(''); fragment = fragment.join(''); - prefix = $.PrivateBin.Helper.htmlEntities(prefix); - postfix = ' ' + $.PrivateBin.Helper.htmlEntities(postfix); + postfix = ' ' + postfix; let url = schema + '://' + address.join('') + '/?' + query + '#' + fragment; // special cases: When the query string and fragment imply the beginning of an HTML entity, eg. � or &#x @@ -109,7 +108,7 @@ describe('Helper', function () { postfix = ''; } - return prefix + '' + url + '' + postfix === $.PrivateBin.Helper.urls2links(prefix + url + postfix); + return $.PrivateBin.Helper.htmlEntities(prefix) + '' + $.PrivateBin.Helper.htmlEntities(url) + '' + $.PrivateBin.Helper.htmlEntities(postfix) === $.PrivateBin.Helper.urls2links(prefix + url + postfix); } ); jsc.property( @@ -118,10 +117,8 @@ describe('Helper', function () { jsc.array(common.jscQueryString()), 'string', function (prefix, query, postfix) { - prefix = $.PrivateBin.Helper.htmlEntities(prefix); - postfix = $.PrivateBin.Helper.htmlEntities(postfix); let url = 'magnet:?' + query.join('').replace(/^&+|&+$/gm,''); - return prefix + '' + url + ' ' + postfix === $.PrivateBin.Helper.urls2links(prefix + url + ' ' + postfix); + return $.PrivateBin.Helper.htmlEntities(prefix) + '' + $.PrivateBin.Helper.htmlEntities(url) + ' ' + $.PrivateBin.Helper.htmlEntities(postfix) === $.PrivateBin.Helper.urls2links(prefix + url + ' ' + postfix); } ); }); diff --git a/tpl/bootstrap.php b/tpl/bootstrap.php index 843dc6a7..ba47c27a 100644 --- a/tpl/bootstrap.php +++ b/tpl/bootstrap.php @@ -72,7 +72,7 @@ endif; ?> - + diff --git a/tpl/page.php b/tpl/page.php index c976ce2f..9d66a861 100644 --- a/tpl/page.php +++ b/tpl/page.php @@ -50,7 +50,7 @@ endif; ?> - + From 8a6dcf910a17c9458e458d26f266cea82b9b45f7 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Fri, 6 Mar 2020 20:57:15 +0100 Subject: [PATCH 3/6] Revert "in Helper.urls2links(), encode HTML entities, find and insert links, partially decoding only the href property of it" This reverts commit 5340f417e07fdfdab2f91f5171f4f992bc763a73. --- js/privatebin.js | 23 ++++------------------- js/test/Helper.js | 11 +++++++---- tpl/bootstrap.php | 2 +- tpl/page.php | 2 +- 4 files changed, 13 insertions(+), 25 deletions(-) diff --git a/js/privatebin.js b/js/privatebin.js index 9e2b0ee2..d2b85448 100644 --- a/js/privatebin.js +++ b/js/privatebin.js @@ -297,25 +297,10 @@ jQuery.PrivateBin = (function($, RawDeflate) { */ me.urls2links = function(html) { - let reverseEntityMap = {}; - for (let entity of ['&', '"', '/', '=']) { - reverseEntityMap[entityMap[entity]] = entity; - } - const entityRegex = new RegExp(Object.keys(reverseEntityMap).join('|'), 'g'); - - // encode HTML entities, find and insert links, partially decoding only the href property of it - return me.htmlEntities(html) - .replace( - /(((https?|ftp)://[\w?!&.-;#@~%+*-]+(?![\w\s?!&.;#~%-]*>))|((magnet):[\w?&.-;#@~%+*-]+))/ig, - function(encodedUrl) { - let decodedUrl = encodedUrl.replace( - entityRegex, function(entity) { - return reverseEntityMap[entity]; - } - ); - return '' + encodedUrl + ''; - } - ) + return html.replace( + /(((https?|ftp):\/\/[\w?!=&.\/-;#@~%+*-]+(?![\w\s?!&.\/;#~%"=-]*>))|((magnet):[\w?=&.\/-;#@~%+*-]+))/ig, + '$1' + ); }; /** diff --git a/js/test/Helper.js b/js/test/Helper.js index a884eee2..dd38e3c4 100644 --- a/js/test/Helper.js +++ b/js/test/Helper.js @@ -81,7 +81,7 @@ describe('Helper', function () { 'ignores non-URL content', 'string', function (content) { - return $.PrivateBin.Helper.htmlEntities(content) === $.PrivateBin.Helper.urls2links(content); + return content === $.PrivateBin.Helper.urls2links(content); } ); jsc.property( @@ -95,7 +95,8 @@ describe('Helper', function () { function (prefix, schema, address, query, fragment, postfix) { query = query.join(''); fragment = fragment.join(''); - postfix = ' ' + postfix; + prefix = $.PrivateBin.Helper.htmlEntities(prefix); + postfix = ' ' + $.PrivateBin.Helper.htmlEntities(postfix); let url = schema + '://' + address.join('') + '/?' + query + '#' + fragment; // special cases: When the query string and fragment imply the beginning of an HTML entity, eg. � or &#x @@ -108,7 +109,7 @@ describe('Helper', function () { postfix = ''; } - return $.PrivateBin.Helper.htmlEntities(prefix) + '' + $.PrivateBin.Helper.htmlEntities(url) + '' + $.PrivateBin.Helper.htmlEntities(postfix) === $.PrivateBin.Helper.urls2links(prefix + url + postfix); + return prefix + '' + url + '' + postfix === $.PrivateBin.Helper.urls2links(prefix + url + postfix); } ); jsc.property( @@ -117,8 +118,10 @@ describe('Helper', function () { jsc.array(common.jscQueryString()), 'string', function (prefix, query, postfix) { + prefix = $.PrivateBin.Helper.htmlEntities(prefix); + postfix = $.PrivateBin.Helper.htmlEntities(postfix); let url = 'magnet:?' + query.join('').replace(/^&+|&+$/gm,''); - return $.PrivateBin.Helper.htmlEntities(prefix) + '' + $.PrivateBin.Helper.htmlEntities(url) + ' ' + $.PrivateBin.Helper.htmlEntities(postfix) === $.PrivateBin.Helper.urls2links(prefix + url + ' ' + postfix); + return prefix + '' + url + ' ' + postfix === $.PrivateBin.Helper.urls2links(prefix + url + ' ' + postfix); } ); }); diff --git a/tpl/bootstrap.php b/tpl/bootstrap.php index ba47c27a..843dc6a7 100644 --- a/tpl/bootstrap.php +++ b/tpl/bootstrap.php @@ -72,7 +72,7 @@ endif; ?> - + diff --git a/tpl/page.php b/tpl/page.php index 9d66a861..c976ce2f 100644 --- a/tpl/page.php +++ b/tpl/page.php @@ -50,7 +50,7 @@ endif; ?> - + From c11dc8e17effd30d047b0101b19742c9bd34fc28 Mon Sep 17 00:00:00 2001 From: El RIDO Date: Fri, 6 Mar 2020 22:18:38 +0100 Subject: [PATCH 4/6] reverting Helper.urls2links() method to old style, applied to element instead of string, allows inserting plain text as text node --- js/privatebin.js | 48 ++++++++++++++++------------------------------- js/test/Helper.js | 43 ++++++++++++++++++++++++++++++++---------- tpl/bootstrap.php | 2 +- tpl/page.php | 2 +- 4 files changed, 51 insertions(+), 44 deletions(-) diff --git a/js/privatebin.js b/js/privatebin.js index d2b85448..73bdd924 100644 --- a/js/privatebin.js +++ b/js/privatebin.js @@ -281,7 +281,7 @@ jQuery.PrivateBin = (function($, RawDeflate) { }; /** - * convert URLs to clickable links. + * convert URLs to clickable links in the provided element. * * URLs to handle: *
@@ -292,14 +292,15 @@ jQuery.PrivateBin = (function($, RawDeflate) {
          *
          * @name   Helper.urls2links
          * @function
-         * @param  {string} html
-         * @return {string}
+         * @param  {HTMLElement} element
          */
-        me.urls2links = function(html)
+        me.urls2links = function(element)
         {
-            return html.replace(
-                /(((https?|ftp):\/\/[\w?!=&.\/-;#@~%+*-]+(?![\w\s?!&.\/;#~%"=-]*>))|((magnet):[\w?=&.\/-;#@~%+*-]+))/ig,
-                '$1'
+            element.html(
+                element.html().replace(
+                    /(((https?|ftp):\/\/[\w?!=&.\/-;#@~%+*-]+(?![\w\s?!&.\/;#~%"=-]*>))|((magnet):[\w?=&.\/-;#@~%+*-]+))/ig,
+                    '$1'
+                )
             );
         };
 
@@ -2439,11 +2440,6 @@ jQuery.PrivateBin = (function($, RawDeflate) {
                 // add table classes from bootstrap css
                 $plainText.find('table').addClass('table-condensed table-bordered');
             } else {
-                // escape HTML entities, link URLs, sanitize
-                const escapedLinkedText = Helper.urls2links(text);
-                let sanitizeLinkedText = '',
-                    sanitizerConfiguration = {};
-
                 if (format === 'syntaxhighlighting') {
                     // yes, this is really needed to initialize the environment
                     if (typeof prettyPrint === 'function')
@@ -2451,22 +2447,16 @@ jQuery.PrivateBin = (function($, RawDeflate) {
                         prettyPrint();
                     }
 
-                    sanitizeLinkedText = prettyPrintOne(
-                        escapedLinkedText, null, true
+                    $prettyPrint.html(
+                        prettyPrintOne(
+                            Helper.htmlEntities(text), null, true
+                        )
                     );
                 } else {
                     // = 'plaintext'
-                    sanitizeLinkedText = escapedLinkedText;
-                    sanitizerConfiguration = {
-                        ALLOWED_TAGS: ['a'],
-                        ALLOWED_ATTR: ['href', 'rel']
-                    };
+                    $prettyPrint.text(text);
                 }
-                $prettyPrint.html(
-                    DOMPurify.sanitize(
-                        sanitizeLinkedText, sanitizerConfiguration
-                    )
-                );
+                Helper.urls2links($prettyPrint);
                 $prettyPrint.css('white-space', 'pre-wrap');
                 $prettyPrint.css('word-break', 'normal');
                 $prettyPrint.removeClass('prettyprint');
@@ -3243,14 +3233,8 @@ jQuery.PrivateBin = (function($, RawDeflate) {
             const $commentEntryData = $commentEntry.find('div.commentdata');
 
             // set & parse text
-            $commentEntryData.html(
-                DOMPurify.sanitize(
-                    Helper.urls2links(commentText), {
-                        ALLOWED_TAGS: ['a'],
-                        ALLOWED_ATTR: ['href', 'rel']
-                    }
-                )
-            );
+            $commentEntryData.text(commentText);
+            Helper.urls2links($commentEntryData);
 
             // set nickname
             if (nickname.length > 0) {
diff --git a/js/test/Helper.js b/js/test/Helper.js
index dd38e3c4..ab7638e9 100644
--- a/js/test/Helper.js
+++ b/js/test/Helper.js
@@ -81,7 +81,15 @@ describe('Helper', function () {
             'ignores non-URL content',
             'string',
             function (content) {
-                return content === $.PrivateBin.Helper.urls2links(content);
+                content = content.replace("\r", "\n").replace("\u0000", '');
+                let clean = jsdom();
+                $('body').html('
'); + let e = $('#foo'); + e.text(content); + $.PrivateBin.Helper.urls2links(e); + let result = e.text(); + clean(); + return content === result; } ); jsc.property( @@ -95,9 +103,12 @@ describe('Helper', function () { function (prefix, schema, address, query, fragment, postfix) { query = query.join(''); fragment = fragment.join(''); - prefix = $.PrivateBin.Helper.htmlEntities(prefix); - postfix = ' ' + $.PrivateBin.Helper.htmlEntities(postfix); - let url = schema + '://' + address.join('') + '/?' + query + '#' + fragment; + prefix = prefix.replace("\r", "\n").replace("\u0000", ''); + postfix = ' ' + postfix.replace("\r", "\n").replace("\u0000", ''); + let url = schema + '://' + address.join('') + '/?' + query + '#' + fragment, + clean = jsdom(); + $('body').html('
'); + let e = $('#foo'); // special cases: When the query string and fragment imply the beginning of an HTML entity, eg. � or &#x if ( @@ -108,8 +119,12 @@ describe('Helper', function () { url = schema + '://' + address.join('') + '/?' + query.substring(0, query.length - 1); postfix = ''; } - - return prefix + '' + url + '' + postfix === $.PrivateBin.Helper.urls2links(prefix + url + postfix); + e.text(prefix + url + postfix); + $.PrivateBin.Helper.urls2links(e); + let result = e.html(); + clean(); + url = $('
').text(url).html(); + return $('
').text(prefix).html() + '' + url + '' + $('
').text(postfix).html() === result; } ); jsc.property( @@ -118,10 +133,18 @@ describe('Helper', function () { jsc.array(common.jscQueryString()), 'string', function (prefix, query, postfix) { - prefix = $.PrivateBin.Helper.htmlEntities(prefix); - postfix = $.PrivateBin.Helper.htmlEntities(postfix); - let url = 'magnet:?' + query.join('').replace(/^&+|&+$/gm,''); - return prefix + '' + url + ' ' + postfix === $.PrivateBin.Helper.urls2links(prefix + url + ' ' + postfix); + prefix = prefix.replace("\r", "\n").replace("\u0000", ''); + postfix = ' ' + postfix.replace("\r", "\n").replace("\u0000", ''); + let url = 'magnet:?' + query.join('').replace(/^&+|&+$/gm,''), + clean = jsdom(); + $('body').html('
'); + let e = $('#foo'); + e.text(prefix + url + postfix); + $.PrivateBin.Helper.urls2links(e); + let result = e.html(); + clean(); + url = $('
').text(url).html(); + return $('
').text(prefix).html() + '' + url + '' + $('
').text(postfix).html() === result; } ); }); diff --git a/tpl/bootstrap.php b/tpl/bootstrap.php index 843dc6a7..b511a48e 100644 --- a/tpl/bootstrap.php +++ b/tpl/bootstrap.php @@ -72,7 +72,7 @@ endif; ?> - + diff --git a/tpl/page.php b/tpl/page.php index c976ce2f..f9b55840 100644 --- a/tpl/page.php +++ b/tpl/page.php @@ -50,7 +50,7 @@ endif; ?> - + From 7cb830e22fe8aae76498d116f20e68d0f711eeae Mon Sep 17 00:00:00 2001 From: rugk Date: Wed, 4 Mar 2020 11:45:56 +0100 Subject: [PATCH 5/6] It includes a change in the RegEx for URLs because that was broken when a & character later followed at any time after a link (even after a newline). (with a negative lookahead) Test with https://regex101.com/r/i7bZ73/1 Now the RegEx does not check for _all_ chars after a link, but just for the one following the link. (So the lookahead is not * anymore. I guess thsi behaviour was the expectation when it has been implemented.) --- js/privatebin.js | 2 +- tpl/bootstrap.php | 2 +- tpl/page.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/js/privatebin.js b/js/privatebin.js index 65b407e3..c6b98b9b 100644 --- a/js/privatebin.js +++ b/js/privatebin.js @@ -392,7 +392,7 @@ jQuery.PrivateBin = (function($, RawDeflate) { { element.html( element.html().replace( - /(((https?|ftp):\/\/[\w?!=&.\/-;#@~%+*-]+(?![\w\s?!&.\/;#~%"=-]*>))|((magnet):[\w?=&.\/-;#@~%+*-]+))/ig, + /(((https?|ftp):\/\/[\w?!=&.\/-;#@~%+*-]+(?![\w\s?!&.\/;#~%"=-]>))|((magnet):[\w?=&.\/-;#@~%+*-]+))/ig, '$1' ) ); diff --git a/tpl/bootstrap.php b/tpl/bootstrap.php index 1ea2686f..e9853f22 100644 --- a/tpl/bootstrap.php +++ b/tpl/bootstrap.php @@ -72,7 +72,7 @@ endif; ?> - + diff --git a/tpl/page.php b/tpl/page.php index 935a1721..f54170e1 100644 --- a/tpl/page.php +++ b/tpl/page.php @@ -50,7 +50,7 @@ endif; ?> - + From 71c76adac4abd198dfbc4221e61c8007fd37365f Mon Sep 17 00:00:00 2001 From: El RIDO Date: Fri, 6 Mar 2020 23:00:48 +0100 Subject: [PATCH 6/6] addressing false positive jsverify rngState 077c06da821594b3fe --- js/test/Helper.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/js/test/Helper.js b/js/test/Helper.js index ab7638e9..b8f6aa21 100644 --- a/js/test/Helper.js +++ b/js/test/Helper.js @@ -81,7 +81,7 @@ describe('Helper', function () { 'ignores non-URL content', 'string', function (content) { - content = content.replace("\r", "\n").replace("\u0000", ''); + content = content.replace(/\r/g, '\n').replace(/\u0000/g, ''); let clean = jsdom(); $('body').html('
'); let e = $('#foo'); @@ -103,8 +103,8 @@ describe('Helper', function () { function (prefix, schema, address, query, fragment, postfix) { query = query.join(''); fragment = fragment.join(''); - prefix = prefix.replace("\r", "\n").replace("\u0000", ''); - postfix = ' ' + postfix.replace("\r", "\n").replace("\u0000", ''); + prefix = prefix.replace(/\r/g, '\n').replace(/\u0000/g, ''); + postfix = ' ' + postfix.replace(/\r/g, '\n').replace(/\u0000/g, ''); let url = schema + '://' + address.join('') + '/?' + query + '#' + fragment, clean = jsdom(); $('body').html('
'); @@ -133,8 +133,8 @@ describe('Helper', function () { jsc.array(common.jscQueryString()), 'string', function (prefix, query, postfix) { - prefix = prefix.replace("\r", "\n").replace("\u0000", ''); - postfix = ' ' + postfix.replace("\r", "\n").replace("\u0000", ''); + prefix = prefix.replace(/\r/g, '\n').replace(/\u0000/g, ''); + postfix = ' ' + postfix.replace(/\r/g, '\n').replace(/\u0000/g, ''); let url = 'magnet:?' + query.join('').replace(/^&+|&+$/gm,''), clean = jsdom(); $('body').html('
');