OpenPNE 3

再現確認できました。

Environment (再現バージョン)¶

OpenPNE3.6.1

Way to repro (再現手順)¶

1. 管理画面画像アップロードページ(/pc_backend.php/monitoring/editImage)にアクセスする
2. 透過されている png 画像をアップロードする
3. 手順2 でアップロードした画像が透過されていない

Way to fix (修正内容)¶

管理画面からアップロードした透過画像が透過されるように修正お願いします。

原因¶

管理画面画像リスト（/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();
   }

更新履歴 d29e27a1dc7aec3cab2724ff57e11eb16de7ad72 で適用されました。

他の手段でアップロードされたものと同様の命名規則を用いる方法を検討するため，一旦ステータスを Accepted に戻します．

更新履歴 6ab38dc8e29ad0a60fc6ec8403399d5df3c98a0b で適用されました。

問題なしとします。

ただし、管理画面でアップロードされたり、 OpenPNE 2 からのコンバート後の画像のような形式のファイル名を持つ画像は、今回対応がおこなわれた画面以外でも意図せず JPEG フォーマットとしてみなされてしまうため、それらすべての箇所においても対応が必要です。この問題に対応するためのチケットを #2864 で作成しました。

OpenPNE 3.8.2 にて対応済みであったため、対応せずとします。