- "NewsletterTablePager.php" line 96 - Run htmlspecialchars after you truncate, not before. In this particular case, the worst that could happen would be for an entity reference to get cut off in the middle, but nonetheless escaping should always be the last thing you do.
- NewsletterDiffEngine - The messages for the h4 headers are not escaped.
- "SpecialNewsletter.php" line 142 - newsletter-subtitlelinks-foo message is not escaped when it is the active link.
- "SpecialNewsletter.php" line 207 - Double escaping newsletter-do-unsubscribe.
- "SpecialNewsletter.php" line 220 - Double escaping newsletter-do-subscribe.
- "NewsletterContent.php" line 253 - Double escaping in the OOUI button labels (5 instances)
- "NewsletterTablePager.php" line 33 - getFieldNames() has double escaping.
Description
Description
Details
Details
Customize query in gerrit
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Duplicate | Qgil | T125545 Phabricator Q&A session for Community Liaisons | |||
| Resolved | Qgil | T116025 Goal: Align Community Liaison and Developer Relations project management practices | |||
| Resolved | Qgil | T119387 Community Liaison and Developer Relation quarterly goals for January - March 2016 | |||
| Declined | None | T104131 Exporting existing newsletter to the Newsletter extension | |||
| Resolved | Addshore | T110170 Goal: Deploy Newsletter extension in Wikimedia | |||
| Duplicate | None | T115098 Deploy Newsletter extension in beta cluster | |||
| Resolved | ori | T127297 Add the Newsletter extension to the Beta Cluster | |||
| Resolved | Bawolff | T115095 Security review of Newsletter extension | |||
| Resolved | xSavitar | T159075 Fix character escaping throughout the extension files |
Event Timeline
Comment Actions
i want to try to fix them as its my first time contribution how to get source code for this help me getting it here tony
Comment Actions
Welcome @mvmohitverma54! Please see https://www.mediawiki.org/wiki/How_to_become_a_MediaWiki_hacker for general questions (like how to get the code) which are not specifically related to the topic of this task. (And using punctuation is also welcome - makes things easier to read! :) ) Thanks a lot! Looking forward to your patch!
Comment Actions
Change 339963 had a related patch set uploaded (by D3r1ck01):
Converting $values to HTML entity after truncating
Comment Actions
Change 339963 merged by jenkins-bot:
Converting $values to HTML entity after truncating
Comment Actions
Change 339967 had a related patch set uploaded (by D3r1ck01):
Escaping newsletter-subtitlelinks-foo message
Comment Actions
Change 339967 abandoned by D3r1ck01:
Escaping newsletter-subtitlelinks-foo message
Reason:
Will submit a different PS instead.
Comment Actions
Change 340054 had a related patch set uploaded (by D3r1ck01):
h4 messages not escaped in NewsletterDiffEngine
Comment Actions
Taking this one but anyone is free to still do any task in the list if its undone since its a many-in-one task.
Comment Actions
@Bawolff : I was just wondering why we would need ->escape() here
$output .= Html::element( 'h4', [], $this->msg( 'newsletter-diff-descheader' )->text() );
as its just an extension message - without any extra params. Probably I am missing something ?
Comment Actions
Change 340314 had a related patch set uploaded (by D3r1ck01; owner: Alangi Derick):
Escaping newsletter-subtitlelinks-foo message
Comment Actions
@Bawolff, I don't think 'default' => $this->msg( 'newsletter-do-unsubscribe' )->escaped(), is double escaped and I have tested and I am sure. As per this task: "SpecialNewsletter.php" line 207 - Double escaping newsletter-do-unsubscribe. Have a second look and let me know :)
Comment Actions
For this case, my assumption is that HTMLForm::factory() might be doing the magic of escaping.
Comment Actions
@01tonythomas, when I remove escaped() and replace with text(), its no longer escaped but when escaped() is allowed, the string is actually escaped. So what magic does HTMLForm::factory() do that doesn't affect text() or are my missing something?
Comment Actions
Sorry folks I think I made a mistake here. The h4 does indeed appear to already be properly escaped (since Html::element escapes things).
Comment Actions
Change 340054 abandoned by D3r1ck01:
h4 messages not escaped in NewsletterDiffEngine
Reason:
Abandoning PS since its no longer needed.
Comment Actions
Change 340566 had a related patch set uploaded (by D3r1ck01; owner: Alangi Derick):
[mediawiki/extensions/Newsletter] Removing double escaping of (un)subscribe message
Comment Actions
Change 340566 merged by D3r1ck01:
[mediawiki/extensions/Newsletter] Removing double escaping of (un)subscribe message
Comment Actions
Change 340604 had a related patch set uploaded (by D3r1ck01; owner: Alangi Derick):
[mediawiki/extensions/Newsletter] Double escaping of OOUI labels
Comment Actions
Change 340604 merged by D3r1ck01:
[mediawiki/extensions/Newsletter] Removing double escaping of OOUI labels
Comment Actions
Change 340650 had a related patch set uploaded (by D3r1ck01; owner: Alangi Derick):
[mediawiki/extensions/Newsletter] Removing double escaping from getFieldNames()
Comment Actions
Change 340650 merged by D3r1ck01:
[mediawiki/extensions/Newsletter] Removing double escaping from getFieldNames()
Comment Actions
Change 340801 had a related patch set uploaded (by Paladox):
[operations/puppet] Gerrit: Fix bot so that it checks against *-name and *-username