Phuedx has uploaded a new change for review.
https://gerrit.wikimedia.org/r/288199
Change subject: Add client-side validation of PopupsSurveyLink
......................................................................
Add client-side validation of PopupsSurveyLink
Changes:
* Extract survey link element creation into
mw.popups.render.renderers.article#createSurveyLink
* Make #createSurveyLink throw an error if the URL doesn't start with
https or http
* Add unit tests that cover the new behaviour of #createSurveyLink
Bug: T129177
Change-Id: I8b61cb0e94ab4e30bc894c279bb05918ecc7719e
---
M extension.json
M resources/ext.popups.renderer.article.js
M tests/qunit/ext.popups.renderer.article.test.js
3 files changed, 47 insertions(+), 15 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Popups
refs/changes/99/288199/1
diff --git a/extension.json b/extension.json
index 8706e7c..980a25e 100644
--- a/extension.json
+++ b/extension.json
@@ -45,7 +45,7 @@
"config": {
"@PopupsBetaFeature": "@var bool: Whether the extension should
be enabled as an opt-in beta feature. If true, the BetaFeatures extension must
be installed. False by default.",
"PopupsBetaFeature": false,
- "@PopupsSurveyLink": "@var bool|string: When defined a link
will be rendered at the bottom of the popup for the user to provide feedback.
Be warned there is no validation of this string so care must be taken to
protect against tab-napping and urls prefixed with `javascript:`",
+ "@PopupsSurveyLink": "@var bool|string: When defined a link
will be rendered at the bottom of the popup for the user to provide feedback.
The URL must start with https or http. If not, then an error is thrown
client-side. The link is annotated with `rel=\"noreferrer\"` so no referrer
information or `window.opener` is leaked to the survey hosting site (see
https://html.spec.whatwg.org/multipage/semantics.html#link-type-noreferrer for
more information).",
"PopupsSurveyLink": false,
"EnablePopupsMobile": false
},
diff --git a/resources/ext.popups.renderer.article.js
b/resources/ext.popups.renderer.article.js
index b8440df..d0aa1c4 100644
--- a/resources/ext.popups.renderer.article.js
+++ b/resources/ext.popups.renderer.article.js
@@ -136,7 +136,6 @@
'mwe-popups-timestamp-recent' :
'mwe-popups-timestamp-older',
$settingsImage = $( '<a>' ).addClass( 'mwe-popups-icon
mwe-popups-settings-icon' ),
- $surveyImage,
$footer = $( '<footer>' )
.append(
$( '<span>' )
@@ -147,19 +146,7 @@
);
if ( article.surveyLink ) {
- $surveyImage = $( '<a>' )
- .attr( 'href', article.surveyLink )
- .attr( 'target', '_blank' )
- .attr( 'title', mw.message(
'popups-send-feedback' ) )
-
- // Don't leak referrer information to the site
hosting the survey. N.B. that
- // `rel=noreferrer` implies `rel=noopener`. See
- //
https://html.spec.whatwg.org/multipage/semantics.html#link-type-noreferrer for
more
- // information.
- .attr( 'rel', 'noreferrer' )
-
- .addClass( 'mwe-popups-icon
mwe-popups-survey-icon' );
- $footer.append( $surveyImage );
+ $footer.append( article.createSurveyLink(
article.surveyLink ) );
}
// createThumbnail returns an empty <span> if there is no
thumbnail
@@ -184,6 +171,32 @@
};
/**
+ * Creates a link to a survey, possibly hosted on an external site.
+ *
+ * @param {string} url
+ * @return {jQuery}
+ */
+ article.createSurveyLink = function ( url ) {
+ if ( !/https?:\/\//.test( url ) ) {
+ throw new Error(
+ 'The survey link URL, i.e. PopupsSurveyLink,
must start with https or http.'
+ );
+ }
+
+ return $( '<a>' )
+ .attr( 'href', url )
+ .attr( 'target', '_blank' )
+ .attr( 'title', mw.message( 'popups-send-feedback' ) )
+
+ // Don't leak referrer information or `window.opener`
to the survey hosting site. See
+ //
https://html.spec.whatwg.org/multipage/semantics.html#link-type-noreferrer for
more
+ // information.
+ .attr( 'rel', 'noreferrer' )
+
+ .addClass( 'mwe-popups-icon mwe-popups-survey-icon' );
+ };
+
+ /**
* Returns an array of elements to be appended after removing
parentheses
* and making the title in the extract bold.
*
diff --git a/tests/qunit/ext.popups.renderer.article.test.js
b/tests/qunit/ext.popups.renderer.article.test.js
index 127c747..9f331a8 100644
--- a/tests/qunit/ext.popups.renderer.article.test.js
+++ b/tests/qunit/ext.popups.renderer.article.test.js
@@ -144,4 +144,23 @@
], true ), 120, 'Correct upper Y 2.' );
} );
+ QUnit.test( 'render.article.createSurveyLink', function ( assert ) {
+ var $surveyLink;
+
+ QUnit.expect( 2 );
+
+ $surveyLink =
mw.popups.render.renderers.article.createSurveyLink( 'http://path/to/resource'
);
+
+ assert.ok( /noreferrer/.test( $surveyLink.attr( 'rel' ) ),
'Survey link doesn\'t leak referrer information or `window.opener`' );
+
+ // ---
+
+ assert.throws(
+ function () {
+
mw.popups.render.renderers.article.createSurveyLink( 'htt://path/to/resource' );
+ },
+ new Error( 'The survey link URL, i.e. PopupsSurveyLink,
must start with https or http.' )
+ );
+ } );
+
} )( jQuery, mediaWiki );
--
To view, visit https://gerrit.wikimedia.org/r/288199
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8b61cb0e94ab4e30bc894c279bb05918ecc7719e
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: master
Gerrit-Owner: Phuedx <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
