search

HollerLondon / sfImagePoolPlugin

Provides a means to manage a pool of images and associate them with other Doctrine_Record objects. Also handles dynamic cropping/sizing with caching on the filesystem, a reverse proxy or on The Edge
MIT License
5 stars 2 forks source link

sfImagePoolable::setImages does not check the type of the $image object #9

Closed nixilla closed 12 years ago

nixilla commented 12 years ago

Looks like in some cases the sfImagePoolable::setImages receives an array of images which are not instances of sfImagePoolImage.

This results passing this instances to Doctrine_Collection::compareRecords which expects Doctrine_Record and call getOid on them.

This ends up with Fatal error "calling getOid on non-object".

This patch fixes this problem and shouldn't break anything, as all it does is ignoring non-sfImagePoolImage types.

Please merge.

Regards Janusz

P.S. The whitespace changes were made by vim - not me :-)

angelsk commented 12 years ago

Out of curiosity what is passing none images to this method? Thinking in terms of unit tests to try and prevent these things happening.

nixilla commented 12 years ago 
  sfImagePoolable->setImages
  call_user_func_array
  Doctrine_Record->__call
  sfDoctrineRecord->__call
  CaseStudy->setImages
  Doctrine_Record->fromArray
  sfFormDoctrine->doUpdateObject
  sfFormObject->updateObject
  BaseCaseStudyForm->doSave
  CaseStudyForm->doSave
  sfFormObject->save

It looks like standard symfony stuff. The only difference is that on the form there are embed 4 sfImagePoolImageForms.

angelsk commented 12 years ago

To be fair I think it's best if this throws an Exception if an sfImagePool object array is not passed as it helps with debugging.

As discussed the embedded forms with the new sfImagePool objects should be saved first, added to the Doctrine_Collection, unset those values in the $values array, and let the form do the saving. That way setImages never gets the array that it can't handle.

angelsk commented 12 years ago

That embedded form could really do with more documentation :)