管理画面画像リスト(/pc_backend.php/monitoring/imageList)で画像を表示する際に保存されている画像のフォーマットが考慮されていない.
他の画像の場合にはファイル名のサフィックスとして拡張子を付与する(例: hoge_png)ことで sf_image_path() 使用時に自動的に保管するようになっているが, 管理画面の画像アップロードページからのアップロードではそのサフィックスが与えられていない.
apps/pc_backend/modules/monitoring/templates/_imageInfo.php
5 <dd class="upImage"><a href="<?php echo sf_image_path($image->getName()) ?>"><?php echo image_tag_sf_image($image->getName(), $options = array('size' => '120x120')) ?></a></dd>
lib/vendor/symfony/lib/plugins/sfImageHandlerPlugin/lib/helper/sfImageHelper.php
27 function image_tag_sf_image($filename, $options = array())
28 {
29 if (empty($options['alt']))
30 {
31 $options['alt'] = '';
32 }
33
34 if (!$filename)
35 {
36 if (isset($options['no_image']))
37 {
38 $filename = $options['no_image'];
39 unset($options['no_image']);
40 }
41 else
42 {
43 $filename = 'no_image.gif';
44 }
45 return image_tag($filename, $options);
46 }
47
48 $filepath = sf_image_path($filename, $options);
...
69 function sf_image_path($filename, $options = array(), $absolute = false)
70 {
71 if (isset($options['f']))
72 {
73 $f = $options['f'];
74 }
75 elseif (isset($options['format']))
76 {
77 $f = $options['format'];
78 }
79 elseif (is_callable(array($filename, 'getType')))
80 {
81 $f = str_replace('image/', '', $filename->getType());
82 }
83 else
84 {
85 $parts = explode('_', $filename);
86 $f = array_pop($parts);
87 }
88
89 if ($f !== 'jpg' && $f !== 'png' && $f !== 'gif')
90 {
91 $f = 'jpg';
92 }
修正案¶
少なくとも,保存されている画像自体は変更されているわけではないため,表示時に画像フォーマットを考慮することでも対処は可能である.単純に管理画面での画像表示について修正を行うならば以下のもので十分である.
diff --git a/apps/pc_backend/modules/monitoring/templates/_imageInfo.php b/apps/pc_backend/modules/monitoring/templates/_imageInfo.php
index 2e4af77..7df88b9 100644
--- a/apps/pc_backend/modules/monitoring/templates/_imageInfo.php
+++ b/apps/pc_backend/modules/monitoring/templates/_imageInfo.php
@@ -2,7 +2,11 @@
<div class="cell">
<dl>
<dt class="day"><?php echo $image->getCreatedAt() ?></dt>
-<dd class="upImage"><a href="<?php echo sf_image_path($image->getName()) ?>"><?php echo image_tag_sf_image($image->getName(), $options = array('size' => '120x120')) ?></a></dd>
+<dd class="upImage">
+<a href="<?php echo sf_image_path($image->getName(), array('format' => $image->getImageFormat())) ?>">
+<?php echo image_tag_sf_image($image->getName(), array('size' => '120x120', 'format' => $image->getImageFormat())) ?>
+</a>
+</dd>
<dd class="fileName"><?php echo $image->getName() ?></dd>
<?php if ($deleteBtn): ?>
<dd class="delete">
しかし,管理画面以外での利用を考慮すると,他の画像と同様にサフィックスに画像フォーマットを付与する方法を用いることが望ましいものと考えられる.ここで,管理画面からの画像アップロードの場合は管理者が自由に名前をつけることができ,これに文字制限は存在していないことを考慮する必要がある.実際に保存を行うスキームを確認すると string(64) となっている.
これは管理者がつける名前によって半角英数とハイフンを用いて少なくとも 64 文字以上を入力すると,入力した文字列全てが反映されない状態でファイル名が定まることになる.現状では "admin_" 6 文字と "_" 1 文字と UNIX 時間 10 文字(2012/02/27現在)の合計 17 文字を引いた 47 文字以上だとファイル名が入力したとおりにはならないと思われる.
この対策としては,入力された文字を指定した文字より長い部分を無視するようにするか,バリデーションで弾く方法が考えられる.
config/doctrine/schema.yml
60 File:
61 columns:
62 id: { type: integer(4), primary: true, autoincrement: true, comment: "Serial number" }
63 name: { type: string(64), default: "", notnull: true, comment: "File name" }
64 type: { type: string(64), default: "", notnull: true, comment: "Type of this file" }
65 filesize: { type: integer(4), default: 0, notnull: true, comment: "File size" }
66 original_filename: { type: string, comment: "Original filename" }
67 indexes:
68 name_UNIQUE:
69 fields: [name]
70 type: unique
71 options:
72 type: INNODB
73 collate: utf8_unicode_ci
74 charset: utf8
75 comment: "Saves informations of files uploaded"
ちなみに MemberImageForm では "m_" 2文字と メンバID の桁数分の文字と $file->setFromValidatedFile() によって 生成される文字列最大 49 文字(sha1() 関数の返り値 40 文字及び拡張子の文字列最大 9 文字)となっているため,メンバIDの桁数が 13 桁を超えない限りは問題ないはずである.
lib/form/doctrine/MemberImageForm.class.php
46 $file = new File();
47 $file->setFromValidatedFile($this->getValue('file'));
48 $file->setName('m_'.$this->member->getId().'_'.$file->getName());
修正内容案¶
バリデーションでファイル名の長さを決定してファイル名末尾に付与する拡張子部分が上書きされるのを防ぎ,既存のファイルももとのフォーマットで表示できるようにオプションを追加するように修正することを提案する.32 文字としたのはきりのいい数字であり,その数字自体に意味を持たないとすることを表すためである.
diff --git a/apps/pc_backend/modules/monitoring/templates/_imageInfo.php b/apps/pc_backend/modules/monitoring/templates/_imageInfo.php
index 2e4af77..7df88b9 100644
--- a/apps/pc_backend/modules/monitoring/templates/_imageInfo.php
+++ b/apps/pc_backend/modules/monitoring/templates/_imageInfo.php
@@ -2,7 +2,11 @@
<div class="cell">
<dl>
<dt class="day"><?php echo $image->getCreatedAt() ?></dt>
-<dd class="upImage"><a href="<?php echo sf_image_path($image->getName()) ?>"><?php echo image_tag_sf_image($image->getName(), $options = array('size' => '120x120')) ?></a></dd>
+<dd class="upImage">
+<a href="<?php echo sf_image_path($image->getName(), array('format' => $image->getImageFormat())) ?>">
+<?php echo image_tag_sf_image($image->getName(), array('size' => '120x120', 'format' => $image->getImageFormat())) ?>
+</a>
+</dd>
<dd class="fileName"><?php echo $image->getName() ?></dd>
<?php if ($deleteBtn): ?>
<dd class="delete">
diff --git a/lib/form/doctrine/ImageForm.php b/lib/form/doctrine/ImageForm.php
index 3f87b34..3d4416e 100644
--- a/lib/form/doctrine/ImageForm.php
+++ b/lib/form/doctrine/ImageForm.php
@@ -29,7 +29,7 @@ class ImageForm extends BaseFileForm
));
$this->setValidators(array(
'file' => new opValidatorImageFile(),
- 'imageName' => new sfValidatorRegex(array('pattern' => '/^[\w\-]+$/')),
+ 'imageName' => new sfValidatorRegex(array('pattern' => '/^[\w\-]+$/', 'max_length' => 32)),
));
$this->widgetSchema->setNameFormat('image[%s]');
@@ -39,7 +39,7 @@ class ImageForm extends BaseFileForm
{
$file = new File();
$file->setFromValidatedFile($this->getValue('file'));
- $file->setName(sprintf('admin_%s_%d', $this->getValue('imageName'), time()));
+ $file->setName(sprintf('admin_%s_%d_%s', $this->getValue('imageName'), time(), $file->getImageFormat()));
return $file->save();
}