Skip to content
Permalink
Browse files Browse the repository at this point in the history
XWIKI-19667: The move attachment form is missing escaping and some tr…
…anslations
  • Loading branch information
manuelleduc committed May 5, 2022
1 parent 55721e2 commit fbc4bfb
Show file tree
Hide file tree
Showing 9 changed files with 400 additions and 36 deletions.
Expand Up @@ -37,6 +37,9 @@ attachment.move.submit=Submit
attachment.move.status.notFound=The requested attachment move status could not be found.
attachment.move.status.label=Attachment Move Status
attachment.move.status.hint=The following attachment move operation has been started by {0} on {1}.
attachment.move.targetLocation.modal.title=Select New Attachment Location
attachment.move.targetLocation.modal.selectButton=Select
attachment.move.targetLocation.modal.closeButton=Close
attachment.job.saveDocument.inPlace=Attachment {0} moved to {1}.
attachment.job.saveDocument.source=Attachment moved to {0}.
attachment.job.saveDocument.target=Attachment moved from {0}.
Expand Down
Expand Up @@ -63,7 +63,7 @@
## Do the move. It's the form in attachment/moveStep1.vm page that calls this page with step=2.
#if ("$!targetAttachmentName.trim()" == '' || "$!targetLocation.trim()" == '')
$response.setStatus(400)
#error($services.localization.render('attachment.move.emptyName'))
#error($escapetool.xml($services.localization.render('attachment.move.emptyName')))
#template("attachment/moveStep1.vm")
#else
#template('attachment/refactoring_macros.vm')
Expand All @@ -75,8 +75,7 @@
#template("attachment/moveStep1.vm")
#elseif (!$services.security.authorization.hasAccess('edit', $targetLocation))
$response.setStatus(403)
#error($services.localization.render('attachment.move.targetNotWritable',
[${escapetool.xml($targetLocation)}]))
#error($escapetool.xml($services.localization.render('attachment.move.targetNotWritable')))
#template('attachment/moveStep1.vm')
#else
#try("moveJobException")
Expand All @@ -85,7 +84,7 @@
#set ($moveJob = $services.attachment.createMoveJob($moveRequest))
#end
#if ("$!moveJobException" != '')
#displayException($services.localization.render('attachment.job.creation.error'), $moveJobException)
#displayException($escapetool.xml($services.localization.render('attachment.job.creation.error')), $moveJobException)
#else
$response.sendRedirect($doc.getURL($xcontext.action, $escapetool.url({
'xpage': 'attachment/move',
Expand Down
Expand Up @@ -40,9 +40,11 @@
#macro (displayMoveJobStatus $moveJobStatus)
#set ($moveJobRequest = $moveJobStatus.request)
<div class="xcontent">
<h2>$services.localization.render('attachment.move.status.label')</h2>
<p class="text-muted small">$services.localization.render('attachment.move.status.hint',
[$xwiki.getUserName($moveJobRequest.userReference), $xwiki.formatDate($moveJobStatus.startDate)])</p>
<h2>$escapetool.xml($services.localization.render('attachment.move.status.label'))</h2>
<p class="text-muted small">
$escapetool.xml($services.localization.render('attachment.move.status.hint',
[$xwiki.getUserName($moveJobRequest.userReference), $xwiki.formatDate($moveJobStatus.startDate)]))
</p>
#displayMoveJobRequest($moveJobRequest)
#template('job_macros.vm')
#displayJobStatus($moveJobStatus)
Expand All @@ -58,13 +60,13 @@
#set ($moveJobStatus = $services.job.getJobStatus(['refactoring', 'moveAttachment', $request.moveId]))
#if ($moveJobStatus)
#if ($xcontext.action == 'get')
#outputMoveJobStatusJSON($moveJobStatus)
#outputmoveJobStatusJSON($moveJobStatus)
#else
#displayMoveJobStatus($moveJobStatus)
#end
#else
$response.setStatus(404)
<div class="box errormessage">
$services.localization.render('attachment.move.status.notFound')
$escapetool.xml($services.localization.render('attachment.move.status.notFound'))
</div>
#end
Expand Up @@ -23,8 +23,11 @@

#set ($discard = $xwiki.ssrx.use('css/attachment/move.css'))
#set ($discard = $xwiki.jsrx.use('js/attachment/move.js'))
#set($titleToDisplay = $services.localization.render('attachment.move.title',
[$attachment.name, $escapetool.xml($doc.plainTitle), $doc.getURL()]))
#set($titleToDisplay = $services.localization.render('attachment.move.title', [
$escapetool.xml($attachment.name),
$escapetool.xml($doc.plainTitle),
$escapetool.xml($doc.getURL())
]))
<div class="xcontent">
#template('contentheader.vm')
#template('attachment/refactoring_macros.vm')
Expand Down Expand Up @@ -63,16 +66,18 @@
## In the future we'll offer a config option to define the default behavior, see
## http://jira.xwiki.org/browse/XWIKI-13384
#set ($checked = $request.autoRedirect == 'true')
<dt>
<dt class="autoRedirect">
<label>
<input type="checkbox" name="autoRedirect" value="true" #if ($checked)checked="checked"#end />
$services.localization.render('attachment.move.autoRedirect.label')
$escapetool.xml($services.localization.render('attachment.move.autoRedirect.label'))
</label>
## The value submitted when the checkbox is not checked, used to preserve the form state.
<input type="hidden" name="autoRedirect" value="false" />
</dt>
<dd>
<span class="xHint">$services.localization.render('attachment.move.autoRedirect.hint')</span>
<dd class="autoRedirect">
<span class="xHint">
$escapetool.xml($services.localization.render('attachment.move.autoRedirect.hint'))
</span>
</dd>
</dl>
</div>
Expand All @@ -81,21 +86,32 @@
##------------
## Target attachment
##------------
<dt>
<label for="targetAttachmentName">$services.localization.render('attachment.move.newName.label')</label>
<span class="xHint">$!services.localization.render('attachment.move.newName.hint')</span>
<dt class="targetAttachmentName">
<label for="targetAttachmentNameTitle">
$escapetool.xml($services.localization.render('attachment.move.newName.label'))
</label>
<span class="xHint">
$escapetool.xml($services.localization.render('attachment.move.newName.hint'))
</span>
</dt>
<dd>
<input type="text" id="targetAttachmentNameTitle" name="targetAttachmentName" value="$escapetool.xml($attachment.name)"
<input type="text"
id="targetAttachmentNameTitle"
name="targetAttachmentName"
value="$escapetool.xml($attachment.name)"
class="location-title-field"
placeholder="$!services.localization.render('attachment.move.newName.placeholder')" />
placeholder="$escapetool.xml($services.localization.render('attachment.move.newName.placeholder'))" />
</dd>
##------------
## Target Page
##------------
<dt>
<label for="$!{options.id}Title">$services.localization.render('attachment.move.newLocation.label')</label>
<span class="xHint">$!services.localization.render('attachment.move.newLocation.hint')</span>
<label for="targetLocation">
$escapetool.xml($services.localization.render('attachment.move.newLocation.label'))
</label>
<span class="xHint">
$escapetool.xml($services.localization.render('attachment.move.newLocation.hint'))
</span>
</dt>
<dd>
#set ($targetRef = $doc.documentReference)
Expand All @@ -110,7 +126,9 @@
<button type="button" class="close" data-dismiss="modal" aria-label="Close">
<span aria-hidden="true">&times;</span>
</button>
<h5 class="modal-title">Modal title</h5>
<h5 class="modal-title">
$escapetool.xml($services.localization.render('attachment.move.targetLocation.modal.title'))
</h5>
</div>
<div class="modal-body">
#template("documentTree_macros.vm")
Expand All @@ -124,8 +142,12 @@
})
</div>
<div class="modal-footer">
<button type="button" class="btn btn-default" data-dismiss="modal">Close</button>
<button type="button" class="btn btn-primary">Select</button>
<button type="button" class="btn btn-default" data-dismiss="modal">
$escapetool.xml($services.localization.render('attachment.move.targetLocation.modal.closeButton'))
</button>
<button type="button" class="btn btn-primary">
$escapetool.xml($services.localization.render('attachment.move.targetLocation.modal.selectButton'))
</button>
</div>
</div>
</div>
Expand All @@ -150,10 +172,14 @@
</div>
<div class="buttons">
<span class="buttonwrapper">
<input type="submit" value="$services.localization.render('attachment.move.submit')" class="button" />
<input type="submit"
value="$escapetool.xml($services.localization.render('attachment.move.submit'))"
class="button" />
</span>
<span class="buttonwrapper">
<a class="secondary button" href="$doc.getURL()">$services.localization.render('cancel')</a>
<a class="secondary button" href="$doc.getURL()">
$escapetool.xml($services.localization.render('cancel'))
</a>
</span>
</div>
</form>
Expand Down
Expand Up @@ -22,8 +22,12 @@
###
#macro (displaySourceAttachment)
<dt>
<label>$services.localization.render('attachment.move.source.label')</label>
<span class="xHint">$services.localization.render('attachment.move.source.hint')</span>
<label>
$escapetool.xml($services.localization.render('attachment.move.source.label'))
</label>
<span class="xHint">
$escapetool.xml($services.localization.render('attachment.move.source.hint'))
</span>
</dt>
<dd>
#hierarchy($attachment)
Expand All @@ -41,7 +45,7 @@
<label>
#set ($checked = !$request.updateReferences || $request.updateReferences == 'true')
<input type="checkbox" name="updateReferences" value="true" #if ($checked)checked="checked"#end />
$services.localization.render('attachment.move.links.label')
$escapetool.xml($services.localization.render('attachment.move.links.label'))
</label>
## The value submitted when the checkbox is not checked, used to preserve the form state.
<input type="hidden" name="updateReferences" value="false"/>
Expand All @@ -50,7 +54,7 @@
#set ($backLinksCount = $services.attachment.backlinksCount($attachment))
#set ($backLinksURL = '')
<span class="xHint">
$services.localization.render('attachment.move.links.hint', [$backLinksCount])
$escapetool.xml($services.localization.render('attachment.move.links.hint', [$backLinksCount]))
</span>
</dd>
#end
Expand Up @@ -151,15 +151,25 @@ void submitMoveTargetEditNotAllowed() throws Exception
this.request.put("step", "2");

Document render = Jsoup.parse(this.templateManager.render(MOVE_TEMPLATE));
assertEquals("error: attachment.move.targetNotWritable [Space.Target]",
assertEquals("error: attachment.move.targetNotWritable",
render.getElementsByClass("errormessage").get(0).text());
}

@Test
void submitTargetAttachmentNameEmpty() throws Exception
{
this.context.setDoc(this.xwiki.getDocument(new DocumentReference("xwiki", "Space", "Source"), this.context));
this.request.put("step", "2");
this.request.put("form_token", "a6DSv7pKWcPargoTvyx2Ww");
Document render = Jsoup.parse(this.templateManager.render(MOVE_TEMPLATE));
assertEquals("error: attachment.move.emptyName", render.select(".errormessage").text());
}

@Test
void submitMoveTargetAlreadyExists() throws Exception
{
DocumentReference sourceDocumentReference = new DocumentReference("xwiki", "Space", "Source");
DocumentReference targetDocumentReference = new DocumentReference("xwiki", "Space", "Target");
DocumentReference targetDocumentReference = new DocumentReference("xwiki", "Space", "Target\"'");
this.context.setDoc(this.xwiki.getDocument(sourceDocumentReference, this.context));

// Allow the user to edit the source and target documents.
Expand All @@ -178,12 +188,12 @@ void submitMoveTargetAlreadyExists() throws Exception
this.request.put("sourceAttachmentName", "oldname.txt");
this.request.put("autoRedirect", "true");
this.request.put("targetAttachmentName", ATTACHMENT_NAME);
this.request.put("targetLocation", "Space.Target");
this.request.put("targetLocation", "Space.Target\"'");
this.request.put("step", "2");

Document render = Jsoup.parse(this.templateManager.render(MOVE_TEMPLATE));
assertEquals(
"error: attachment.move.alreadyExists [attachment.txt, Space.Target, /xwiki/bin/view/Space/Target]",
assertEquals("error: attachment.move.alreadyExists "
+ "[attachment.txt, Space.Target\"', /xwiki/bin/view/Space/Target%22%27]",
render.getElementsByClass("errormessage").get(0).text());
}
}
@@ -0,0 +1,105 @@
/*
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
*
* This is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as
* published by the Free Software Foundation; either version 2.1 of
* the License, or (at your option) any later version.
*
* This software 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
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this software; if not, write to the Free
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/
package org.xwiki.attachment;

import java.util.List;

import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.xwiki.job.Request;
import org.xwiki.job.event.status.JobProgress;
import org.xwiki.job.event.status.JobStatus;
import org.xwiki.job.script.JobScriptService;
import org.xwiki.script.service.ScriptService;
import org.xwiki.template.TemplateManager;
import org.xwiki.template.script.TemplateScriptService;
import org.xwiki.test.annotation.ComponentList;
import org.xwiki.test.page.HTML50ComponentList;
import org.xwiki.test.page.PageTest;
import org.xwiki.test.page.XWikiSyntax21ComponentList;
import org.xwiki.xml.internal.html.filter.ControlCharactersFilter;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

/**
* Test of the macros provided by {@code moveStatus.vm}.
*
* @version $Id$
* @since 14.4RC1
*/
@HTML50ComponentList
@XWikiSyntax21ComponentList
@ComponentList({
ControlCharactersFilter.class,
TemplateScriptService.class
})
class MoveStatusPagesTest extends PageTest
{
private static final String MOVE_STATUS_TEMPLATE = "attachment/moveStatus.vm";

private TemplateManager templateManager;

@Mock
private JobScriptService jobScriptService;

@BeforeEach
void setUp() throws Exception
{
this.templateManager = this.oldcore.getMocker().getInstance(TemplateManager.class);
this.componentManager.registerComponent(ScriptService.class, "job", this.jobScriptService);
}

@Test
void renderScriptStatusUnknown() throws Exception
{
this.request.put("moveId", "42");
Document render = Jsoup.parse(this.templateManager.render(MOVE_STATUS_TEMPLATE));
assertEquals("attachment.move.status.notFound", render.select(".errormessage").text());
verify(this.jobScriptService).getJobStatus(List.of("refactoring", "moveAttachment", "42"));
}

@Test
void renderScriptActionGet() throws Exception
{
JobStatus jobStatus = mock(JobStatus.class);
Request request = mock(Request.class);
JobProgress jobProgress = mock(JobProgress.class);

when(request.getId()).thenReturn(List.of("rq", "id"));
when(jobProgress.getOffset()).thenReturn(10d);
when(jobStatus.getRequest()).thenReturn(request);
when(jobStatus.getState()).thenReturn(JobStatus.State.RUNNING);
when(jobStatus.getProgress()).thenReturn(jobProgress);
when(this.jobScriptService.getJobStatus(List.of("refactoring", "moveAttachment", "42")))
.thenReturn(jobStatus);
this.request.put("moveId", "42");
this.context.setAction("get");
assertEquals(
"{\"id\":[\"rq\",\"id\"],\"state\":\"RUNNING\",\"progress\":{\"offset\":10.0},"
+ "\"log\":{\"offset\":0,\"items\":[]},\"message\":\" \",\"questionTimeLeft\":0}",
this.templateManager.render(MOVE_STATUS_TEMPLATE).trim());
}
}

0 comments on commit fbc4bfb

Please sign in to comment.