Project

General

Profile

Bug(バグ) #2632

管理画面から画像をアップロードしたときに透過画像のときに透過されなくなる

Added by Yuya Watanabe over 7 years ago. Updated over 2 years ago.

Status:
Won't fix(対応せず)
Priority:
Normal(通常)
Assignee:
Target version:
Start date:
2011-11-29
Due date:
% Done:

0%

3.6 で発生するか:
Yes (はい)
3.8 で発生するか:
Unknown (未調査)

Description

概要

管理画面から画像をアップロードしたときに透過画像のときに透過されなくなる.

原因

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

修正方法

File クラスインスタンスを表示用のヘルパー関数に渡すようにしていき、インスタンスの保持する MIME Type 情報に基づく画像フォーマットを選択できるようにする.

diff --git a/apps/pc_backend/modules/monitoring/templates/_imageInfo.php b/apps/pc_backend/modules/monitoring/templates/_imageInfo.php
index 2e4af77..37d1913 100644
--- a/apps/pc_backend/modules/monitoring/templates/_imageInfo.php
+++ b/apps/pc_backend/modules/monitoring/templates/_imageInfo.php
@@ -2,7 +2,7 @@
 <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) ?>"><?php echo image_tag_sf_image($image, $options = array('size' => '120x120')) ?></a></dd>
 <dd class="fileName"><?php echo $image->getName() ?></dd>
 <?php if ($deleteBtn): ?>
 <dd class="delete"> 

361_backend.jpg View (8.42 KB) Yuma Sakata, 2011-12-15 11:12


Related issues

Related to OpenPNE 3 - Backport(バックポート) #2831: 管理画面から画像をアップロードしたときに透過画像のときに透過されなくなる Fixed(完了) 2011-11-29
Related to OpenPNE 3 - Backport(バックポート) #2839: 管理画面から画像をアップロードしたときに透過画像のときに透過されなくなる Fixed(完了) 2011-11-29
Related to OpenPNE 3 - Bug(バグ) #2864: OpenPNE 2 からコンバートされたか、管理画面からアップロードされた画像が常に JPEG として表示されてしまう箇所がある New(新規) 2012-03-08
Related to OpenPNE 3 - Backport(バックポート) #3110: 管理画面から画像をアップロードしたときに透過画像のときに透過されなくなる Fixed(完了) 2011-11-29

Associated revisions

Revision d29e27a1 (diff)
Added by Yuya Watanabe over 7 years ago

(fixes #2632) fixed not to display images using jpeg format in admin monitoring page

Revision 6ab38dc8 (diff)
Added by Yuya Watanabe over 7 years ago

Revert "(fixes #2632) fixed not to display images using jpeg format in admin monitoring page"

This reverts commit d29e27a1dc7aec3cab2724ff57e11eb16de7ad72.

Revision 965265e0 (diff)
Added by Yuya Watanabe over 7 years ago

(fixes #2632) fixed to use file object in sfImageHandlar's method argument for displaying correct format

History

#1 Updated by Yuya Watanabe over 7 years ago

  • 3.6 で発生するか set to Unknown (未調査)
  • 3.4 で発生するか set to Yes (はい)

#2 Updated by Yuma Sakata over 7 years ago

  • File 361_backend.jpg View added
  • 3.6 で発生するか changed from Unknown (未調査) to Yes (はい)

再現確認できました。

Environment (再現バージョン)

OpenPNE3.6.1

Way to repro (再現手順)

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

Way to fix (修正内容)

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

#3 Updated by Yuma Sakata over 7 years ago

管理画面から画像をアップできる箇所の調査しました。

  • 画像アップロードページ(/pc_backend.php/monitoring/editImage)から画像アップした場合、透過されない。
  • トップバナー(ログイン前)設定ページ(/pc_backend.php/design/banner/id/1)から画像アップした場合、透過される。
  • トップバナー(ログイン後)設定ページ(/pc_backend.php/design/banner/id/3)から画像アップした場合、透過される。
  • サイドバナー(ログイン前)設定ページ(/pc_backend.php/design/banner/id/2)から画像アップした場合、透過される。
  • サイドバナー(ログイン後)設定ページ(/pc_backend.php/design/banner/id/2)から画像アップした場合、透過される。

#4 Updated by Minoru Takai over 7 years ago

管理画面から画像をアップロードしたときに透過画像のときに透過されなくなる.
詳しい挙動を調べてないので問題内容のみ報告.

とチケットにありますが、補足しておきます。

note-2 の再現確認において「2. 透過されている png 画像をアップロードする」とあり、その添付画像が「361_backend.jpg」となっていることからも推察できるのですが、私が簡単に動作を確認したところ、管理画面の「画像アップロード」ページから画像をアップロードした場合、その画像の画像フォーマットに拘らず JPEG 形式で画像が生成されるようです。

「透過されない」というより「JPEGで生成されている」という問題と捉える方が的確かもしれません。透過画像に拘らずとも、例えば 256 色以内のグラデーションを表現した PNG 画像をアップロードした場合でも JPEG が生成されてしまうため、画質が劣化します(画像形式が適していないため)。

透過画像に限らず、様々な画像を用いた動作検証、および修正方針の検討をすべきです。

#5 Updated by Yuya Watanabe over 7 years ago

  • Status changed from New(新規) to Accepted(着手)
  • Assignee set to Yuya Watanabe
  • Target version set to OpenPNE 3.7.0

原因

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

#6 Updated by Yuya Watanabe over 7 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

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

#7 Updated by Yuya Watanabe over 7 years ago

  • Status changed from Pending Review(レビュー待ち) to Accepted(着手)
  • % Done changed from 50 to 0

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

#8 Updated by Kousuke Ebihara over 7 years ago

OpenPNE 2 からのコンバート時にインサートされた画像ファイルにおいても、これと同様の現象が発生します。

その際に急場でしのぐために適用したパッチを以下に添付します。
https://gist.github.com/1949225

#9 Updated by Kousuke Ebihara over 7 years ago

Yuya Watanabe は書きました:

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

海老原としてはアップロード時の問題ではなく、 http://redmine.openpne.jp/issues/2632#note-8 の修正のように、表示側で対処するべき問題であると考えています。

ファイル名しか得られないケースでは、このパッチのように場当たり的な対処にならざるを得ないと思うので、今回、もしくは今後は、できるだけ、ファイル名ではなく File クラスインスタンスを表示用のヘルパー関数に渡すようにしていき、インスタンスの保持する MIME Type 情報に基づく画像フォーマットを選択できるようにするべきです。

#10 Updated by Yuya Watanabe over 7 years ago

表示側で対処すべきという方法について把握しました.

note-5 での修正は取り消し,下記のような修正を行います.本チケットでは文字制限に関しては取り扱わないこととします.

diff --git a/apps/pc_backend/modules/monitoring/templates/_imageInfo.php b/apps/pc_backend/modules/monitoring/templates/_imageInfo.php
index 2e4af77..37d1913 100644
--- a/apps/pc_backend/modules/monitoring/templates/_imageInfo.php
+++ b/apps/pc_backend/modules/monitoring/templates/_imageInfo.php
@@ -2,7 +2,7 @@
 <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) ?>"><?php echo image_tag_sf_image($image, $options = array('size' => '120x120')) ?></a></dd>
 <dd class="fileName"><?php echo $image->getName() ?></dd>
 <?php if ($deleteBtn): ?>
 <dd class="delete"> 

#11 Updated by Yuya Watanabe over 7 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50

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

#12 Updated by Yuya Watanabe over 7 years ago

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

#13 Updated by Kousuke Ebihara over 7 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

問題なしとします。

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

#14 Updated by Shouta Kashiwagi over 7 years ago

  • Target version changed from OpenPNE 3.7.0 to 252

#15 Updated by Yuya Watanabe over 7 years ago

  • Target version changed from 252 to OpenPNE 3.8.x

#16 Updated by Yuma Sakata almost 7 years ago

  • Target version changed from OpenPNE 3.8.x to OpenPNE 3.8.2

#17 Updated by Yuma Sakata almost 7 years ago

  • Target version changed from OpenPNE 3.8.2 to OpenPNE 3.9.0-old
  • 3.8 で発生するか set to Unknown (未調査)

#18 Updated by Yuya Watanabe almost 7 years ago

  • Description updated (diff)

#20 Updated by isao sano over 2 years ago

  • Status changed from Pending Testing(テスト待ち) to Won't fix(対応せず)
  • % Done changed from 70 to 0

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

Also available in: Atom PDF