From 15ce7b56ccfe16641a349c82886b5949f17dec88 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Tue, 17 Nov 2009 08:53:56 -0800 Subject: [PATCH] Quote the id generated by the Image asset. Also, do not overwrite user uploaded parameters. Fixes bug #11246 --- docs/changelog/7.x.x.txt | 1 + lib/WebGUI/Asset/File/Image.pm | 28 +++++++++++------------ t/Asset/File/Image.t | 42 +++++++++++++++++++++++++--------- 3 files changed, 46 insertions(+), 25 deletions(-) diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 3fdba456a..28137d40e 100644 --- a/docs/changelog/7.x.x.txt +++ b/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. diff --git a/lib/WebGUI/Asset/File/Image.pm b/lib/WebGUI/Asset/File/Image.pm index f0f62444c..7b2b9f257 100644 --- a/lib/WebGUI/Asset/File/Image.pm +++ b/lib/WebGUI/Asset/File/Image.pm @@ -221,8 +221,9 @@ Renders this asset. =cut sub view { - my $self = shift; - if (!$self->session->var->isAdminOn && $self->get("cacheTimeout") > 10) { + my $self = shift; + 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{fileUrl} = $self->getFileUrl; - $var{fileIcon} = $self->getFileIconUrl; - $var{thumbnail} = $self->getThumbnailUrl; - $var{annotateJs} = "$crop_js$domMe"; - $var{parameters} = sprintf("id=%s", $self->getId()); - my $form = $self->session->form; + $var{controls} = $self->getToolbar; + $var{fileUrl} = $self->getFileUrl; + $var{fileIcon} = $self->getFileIconUrl; + $var{thumbnail} = $self->getThumbnailUrl; + $var{annotateJs} = "$crop_js$domMe"; + $var{parameters} .= sprintf(q{ id="%s"}, $self->getId()); my $out = $self->processTemplate(\%var,undef,$self->{_viewTemplate}); - if (!$self->session->var->isAdminOn && $self->get("cacheTimeout") > 10) { - WebGUI::Cache->new($self->session,"view_".$self->getId)->set($out,$self->get("cacheTimeout")); - } - return $out; + if (!$session->var->isAdminOn && $self->get("cacheTimeout") > 10) { + WebGUI::Cache->new($self->session,"view_".$self->getId)->set($out,$self->get("cacheTimeout")); + } + return $out; } diff --git a/t/Asset/File/Image.t b/t/Asset/File/Image.t index 9b2a8bc97..a5a572f91 100644 --- a/t/Asset/File/Image.t +++ b/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; - -END { - if (defined $versionTag and ref $versionTag eq 'WebGUI::VersionTag') { - $versionTag->rollback; - } -} +addToCleanup($versionTag); sub isnt_array { my ($a, $b) = @_;