Commit 55294ce6 authored by Kushal Pandya's avatar Kushal Pandya Committed by Douwe Maan

Improve slash command stripping, escape temporary note contents

parent 61ececb5
...@@ -24,7 +24,7 @@ const normalizeNewlines = function(str) { ...@@ -24,7 +24,7 @@ const normalizeNewlines = function(str) {
(function() { (function() {
this.Notes = (function() { this.Notes = (function() {
const MAX_VISIBLE_COMMIT_LIST_COUNT = 3; const MAX_VISIBLE_COMMIT_LIST_COUNT = 3;
const REGEX_SLASH_COMMANDS = /^\/\w+/gm; const REGEX_SLASH_COMMANDS = /^\/\w+.*$/gm;
Notes.interval = null; Notes.interval = null;
...@@ -1170,6 +1170,7 @@ const normalizeNewlines = function(str) { ...@@ -1170,6 +1170,7 @@ const normalizeNewlines = function(str) {
*/ */
Notes.prototype.createPlaceholderNote = function({ formContent, uniqueId, isDiscussionNote, currentUsername, currentUserFullname }) { Notes.prototype.createPlaceholderNote = function({ formContent, uniqueId, isDiscussionNote, currentUsername, currentUserFullname }) {
const discussionClass = isDiscussionNote ? 'discussion' : ''; const discussionClass = isDiscussionNote ? 'discussion' : '';
const escapedFormContent = _.escape(formContent);
const $tempNote = $( const $tempNote = $(
`<li id="${uniqueId}" class="note being-posted fade-in-half timeline-entry"> `<li id="${uniqueId}" class="note being-posted fade-in-half timeline-entry">
<div class="timeline-entry-inner"> <div class="timeline-entry-inner">
...@@ -1187,7 +1188,7 @@ const normalizeNewlines = function(str) { ...@@ -1187,7 +1188,7 @@ const normalizeNewlines = function(str) {
</div> </div>
<div class="note-body"> <div class="note-body">
<div class="note-text"> <div class="note-text">
<p>${formContent}</p> <p>${escapedFormContent}</p>
</div> </div>
</div> </div>
</div> </div>
......
...@@ -377,7 +377,7 @@ import '~/notes'; ...@@ -377,7 +377,7 @@ import '~/notes';
}); });
it('should return true when comment begins with a slash command', () => { it('should return true when comment begins with a slash command', () => {
const sampleComment = '/wip \n/milestone %1.0 \n/merge \n/unassign Merging this'; const sampleComment = '/wip\n/milestone %1.0\n/merge\n/unassign Merging this';
const hasSlashCommands = this.notes.hasSlashCommands(sampleComment); const hasSlashCommands = this.notes.hasSlashCommands(sampleComment);
expect(hasSlashCommands).toBeTruthy(); expect(hasSlashCommands).toBeTruthy();
...@@ -401,10 +401,18 @@ import '~/notes'; ...@@ -401,10 +401,18 @@ import '~/notes';
describe('stripSlashCommands', () => { describe('stripSlashCommands', () => {
it('should strip slash commands from the comment which begins with a slash command', () => { it('should strip slash commands from the comment which begins with a slash command', () => {
this.notes = new Notes(); this.notes = new Notes();
const sampleComment = '/wip \n/milestone %1.0 \n/merge \n/unassign Merging this'; const sampleComment = '/wip\n/milestone %1.0\n/merge\n/unassign Merging this';
const stripedComment = this.notes.stripSlashCommands(sampleComment); const stripedComment = this.notes.stripSlashCommands(sampleComment);
expect(stripedComment).not.toBe(sampleComment); expect(stripedComment).toBe('');
});
it('should strip slash commands from the comment but leaves plain comment if it is present', () => {
this.notes = new Notes();
const sampleComment = '/wip\n/milestone %1.0\n/merge\n/unassign\nMerging this';
const stripedComment = this.notes.stripSlashCommands(sampleComment);
expect(stripedComment).toBe('Merging this');
}); });
it('should NOT strip string that has slashes within', () => { it('should NOT strip string that has slashes within', () => {
...@@ -424,6 +432,22 @@ import '~/notes'; ...@@ -424,6 +432,22 @@ import '~/notes';
beforeEach(() => { beforeEach(() => {
this.notes = new Notes('', []); this.notes = new Notes('', []);
spyOn(_, 'escape').and.callFake((comment) => {
const escapedString = comment.replace(/["&'<>]/g, (a) => {
const escapedToken = {
'&': '&amp;',
'<': '&lt;',
'>': '&gt;',
'"': '&quot;',
"'": '&#x27;',
'`': '&#x60;'
}[a];
return escapedToken;
});
return escapedString;
});
}); });
it('should return constructed placeholder element for regular note based on form contents', () => { it('should return constructed placeholder element for regular note based on form contents', () => {
...@@ -444,7 +468,21 @@ import '~/notes'; ...@@ -444,7 +468,21 @@ import '~/notes';
expect($tempNote.find('.timeline-content').hasClass('discussion')).toBeFalsy(); expect($tempNote.find('.timeline-content').hasClass('discussion')).toBeFalsy();
expect($tempNoteHeader.find('.hidden-xs').text().trim()).toEqual(currentUserFullname); expect($tempNoteHeader.find('.hidden-xs').text().trim()).toEqual(currentUserFullname);
expect($tempNoteHeader.find('.note-headline-light').text().trim()).toEqual(`@${currentUsername}`); expect($tempNoteHeader.find('.note-headline-light').text().trim()).toEqual(`@${currentUsername}`);
expect($tempNote.find('.note-body .note-text').text().trim()).toEqual(sampleComment); expect($tempNote.find('.note-body .note-text p').text().trim()).toEqual(sampleComment);
});
it('should escape HTML characters from note based on form contents', () => {
const commentWithHtml = '<script>alert("Boom!");</script>';
const $tempNote = this.notes.createPlaceholderNote({
formContent: commentWithHtml,
uniqueId,
isDiscussionNote: false,
currentUsername,
currentUserFullname
});
expect(_.escape).toHaveBeenCalledWith(commentWithHtml);
expect($tempNote.find('.note-body .note-text p').html()).toEqual('&lt;script&gt;alert("Boom!");&lt;/script&gt;');
}); });
it('should return constructed placeholder element for discussion note based on form contents', () => { it('should return constructed placeholder element for discussion note based on form contents', () => {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment