There is an infinite recursion bug with revision 1.209.2.44, DRUPAL-5 (branch: 1.209.2). The $node->images[preview] comes empty in certain cases to the image_display() function, seemingly because the image is the same size as the original. The image_display() function sends Drupal into infinite recursion because the conditionals see an empty $node->images[$label] and attempt to generate derivs. Proper fix would be to make sure that $node->images['preview'] is never empty. Temporary patch is provided, by changing image_display to check for an empty $node->image['preview'] and make it equal to $node->image['_original'] if it is empty.

adding:

$node->images['preview'] = empty($node->images['preview']) ? $node->images['_original'] : $node->images['preview'];

Simple fix patch is attached, but I would fix it by making sure that images['preview'] is not empty easier on in the process.

CommentFileSizeAuthor
#2 image_171080.patch591 bytesdrewish
image-d5-recursion.patch769 bytessami_k

Comments

sami_k’s picture

sami_k commented

Might want to avoid all infinite recursion in this function by using:

$node->images[$label] = empty($node->images[$label]) ? $node->images['_original'] : $node->images[$label];

That way if any fields come empty to this function it won't go into infinite recursion. Behavior was observed at time of creation of a node only.

drewish’s picture

drewish commented
Status: Active » Needs review
StatusFileSize
new image_171080.patch591 bytes

i think this would actually be a better way of doing it. can you test it out for me?

drewish’s picture

drewish commented
Status: Needs review » Fixed

It fixes the previews when the original image is smaller than the preview size so I'm committing it to HEAD and DRUPAL-5.

sami_k’s picture

sami_k commented
Status: Fixed » Active

Your patch causes a problem which has the module trying to copy nothing to files/{filename}.preview.png:

[file] => public_html/sites/all/modules/image/image.module
[line] => 949
[function] => file_move
[args] => Array
(
[0] =>
[1] => {filename}.preview.png
)

sami_k’s picture

sami_k commented

Related with: http://drupal.org/node/160671

sami_k’s picture

sami_k commented

In addition the module says that it regenerated derivs which it does not do without the latest patch. The functionality seems to be fine, but the logic of the module gets screwed up along the way throwing out unnecessary messages.

drewish’s picture

drewish commented

i think you're right but the issue you point out is addressed over on: http://drupal.org/node/160671

drewish’s picture

drewish commented

in #6 i'm not sure if you're referring to the patch on comment #2 or #160671. if you're referring to #160671 please follow up over there.

sami_k’s picture

sami_k commented

#6 I am talking about 5.x head. The patch you posted above causes that problem.

drewish’s picture

drewish commented
Status: Active » Fixed

#160671 was committed to HEAD and should have cleared this up.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)