Quote the id generated by the Image asset. Also, do not overwrite user uploaded parameters. Fixes bug #11246

docs/changelog/7.x.x.txt

@@ -23,6 +23,7 @@
  - added TimeZone form controls accepts spaces or underscores in zone names.
  - fixed #11245: send stats
  - fixed #11247: Survey edit screen broken
+ - fixed #11246: Image.pm - Validation error
 
 7.8.4
  - Fixed a compatibility problem between WRE and new Spectre code.

lib/WebGUI/Asset/File/Image.pm

@@ -221,8 +221,9 @@ Renders this asset.
 =cut
 
 sub view {
-	my $self = shift;
+	my $self    = shift;
-	if (!$self->session->var->isAdminOn && $self->get("cacheTimeout") > 10) {
+    my $session = $self->session;
+	if (!$session->var->isAdminOn && $self->get("cacheTimeout") > 10) {
         my $cache = $self->getCache;
         my $out   = $cache->get if defined $cache;
 		return $out if $out;
@@ -231,7 +232,7 @@ sub view {
     my ($crop_js, $domMe) = $self->annotate_js({ just_image => 1 });
 
     if ($crop_js) {
-        my ($style, $url) = $self->session->quick(qw(style url));
+        my ($style, $url) = $session->quick(qw(style url));
 
         $style->setLink($url->extras('yui/build/fonts/fonts-min.css'), {rel=>'stylesheet', type=>'text/css'});
         $style->setLink($url->extras('yui/container/assets/container.css'), {rel=>'stylesheet', type=>'text/css'});
@@ -239,18 +240,17 @@ sub view {
         $style->setScript($url->extras('yui/build/container/container-min.js'), {type=>'text/javascript'});
     }
 
-	$var{controls} = $self->getToolbar;
+    $var{controls}    = $self->getToolbar;
-	$var{fileUrl} = $self->getFileUrl;
+    $var{fileUrl}     = $self->getFileUrl;
-	$var{fileIcon} = $self->getFileIconUrl;
+    $var{fileIcon}    = $self->getFileIconUrl;
-	$var{thumbnail} = $self->getThumbnailUrl;
+    $var{thumbnail}   = $self->getThumbnailUrl;
-	$var{annotateJs} = "$crop_js$domMe";
+    $var{annotateJs}  = "$crop_js$domMe";
-    $var{parameters} = sprintf("id=%s", $self->getId());
+    $var{parameters} .= sprintf(q{ id="%s"}, $self->getId());
-	my $form = $self->session->form;
     my $out = $self->processTemplate(\%var,undef,$self->{_viewTemplate});
-	if (!$self->session->var->isAdminOn && $self->get("cacheTimeout") > 10) {
+    if (!$session->var->isAdminOn && $self->get("cacheTimeout") > 10) {
-		WebGUI::Cache->new($self->session,"view_".$self->getId)->set($out,$self->get("cacheTimeout"));
+        WebGUI::Cache->new($self->session,"view_".$self->getId)->set($out,$self->get("cacheTimeout"));
-	}
+    }
-       	return $out;
+    return $out;
 }
 
 

t/Asset/File/Image.t

@@ -13,6 +13,7 @@ use strict;
 use lib "$FindBin::Bin/../../lib";
 
 use Test::MockObject;
+use Test::MockObject::Extends;
 my $mocker;
 BEGIN {
     $mocker = Test::MockObject->new();
@@ -20,7 +21,6 @@ BEGIN {
     $mocker->fake_new('WebGUI::Form::Image');
 }
 
-use File::Copy;
 use WebGUI::Test;
 use WebGUI::Session;
 use WebGUI::Image;
@@ -30,7 +30,8 @@ use WebGUI::Form::File;
 
 use Test::More; # increment this value for each test you create
 use Test::Deep;
-plan tests => 11;
+use Data::Dumper;
+plan tests => 13;
 
 my $session = WebGUI::Test->session;
 
@@ -39,7 +40,7 @@ $rectangle->setBackgroundColor('#0000FF');
 
 ##Create a storage location
 my $storage = WebGUI::Storage->create($session);
-WebGUI::Test->storagesToDelete($storage);
+addToCleanup($storage);
 
 ##Save the image to the location
 $rectangle->saveToStorageLocation($storage, 'blue.png');
@@ -73,6 +74,9 @@ $asset->update({
 	filename => 'blue.png',
 });
 
+is($storage->getId, $asset->get('storageId'), 'Asset updated with correct new storageId');
+is($storage->getId, $asset->getStorageLocation->getId, 'Cached Asset storage location updated with correct new storageId');
+
 my $filename = $asset->getStorageLocation->getPath . "/" . $asset->get("filename");
 
 my @stat_before = stat($filename);
@@ -93,16 +97,32 @@ is(isnt_array(\@stat_before, \@stat_after), 1, 'Image is different after crop');
 my $sth = $session->db->read('describe ImageAsset annotations');
 isnt($sth->hashRef, undef, 'Annotations column is defined');
 
-is($storage->getId, $asset->get('storageId'), 'Asset updated with correct new storageId');
+#------------------------------------------------------------------------------
-is($storage->getId, $asset->getStorageLocation->getId, 'Cached Asset storage location updated with correct new storageId');
+# Template variables
+my $templateId = 'FILE_IMAGE_TEMPLATE___';
+
+my $templateMock = Test::MockObject->new({});
+$templateMock->set_isa('WebGUI::Asset::Template');
+$templateMock->set_always('getId', $templateId);
+my $templateVars;
+$templateMock->mock('process', sub { $templateVars = $_[1]; } );
+
+$asset->update({
+    parameters => 'alt="alternate"',
+    templateId => $templateId,
+});
+
+{
+    WebGUI::Test->mockAssetId($templateId, $templateMock);
+    $asset->prepareView();
+    $asset->view();
+    like($templateVars->{parameters}, qr{ id="[^"]{22}"}, 'id in parameters is quoted');
+    like($templateVars->{parameters}, qr{alt="alternate"}, 'original parameters included');
+    WebGUI::Test->unmockAssetId($templateId);
+}
 
 $versionTag->commit;
-
+addToCleanup($versionTag);
-END {
-	if (defined $versionTag and ref $versionTag eq 'WebGUI::VersionTag') {
-		$versionTag->rollback;
-	}
-}
 
 sub isnt_array {
     my ($a, $b) = @_;