Fix supports formtype parser #81

Merged
merged 5 commits into from Oct 11, 2012

Conversation

Projects
None yet
4 participants
Contributor

docteurklein commented Oct 3, 2012

If you give class names which require constructor arguments, the FormTypeParser fails with fatal error when calling supports.

Owner

Seldaek commented Oct 3, 2012

Congrats on the insane hack ;) Looks alright to me but I'll let @willdurand check it out and merge because I'm not very familiar with that part of the code.

Contributor

stof commented Oct 3, 2012

If your class as some required constructor arguments, you should be using the type name instead.
your hack will indeed allow to build the object, but will still break stuff as the type will not have its dependencies and so will break when using it.

Parser/FormTypeParser.php
@@ -65,7 +65,7 @@ public function supports($item)
public function parse($type)
{
if (is_string($type) && class_exists($type)) {
- $type = new $type();
+ $item = unserialize(sprintf('O:%d:"%s":0:{}', strlen($item), $item));
@stof

stof Oct 3, 2012

Contributor

$item ? There is no such variable

@docteurklein

docteurklein Oct 3, 2012

Contributor

oops. fixed.

Contributor

docteurklein commented Oct 3, 2012

@stof I agree about the fact that when parsing the form, we should have a valid object instance.

Maybe should we just keep the hack for the supports method ?
This way, the fatal error would be triggered only when it's actually a real form type.

Contributor

stof commented Oct 3, 2012

@docteurklein supports should check if the class is implementing FormTypeInterface in case it is a class name or an object instead (and call the form factory only when passing a string considered as type name).

And to have a real check, it could even check if the constructor can be called in case a class name is passed (i.e. does not have mandatory arguments).

Contributor

docteurklein commented Oct 4, 2012

@stof there is another problem when creating the form: required options.

Parser/FormTypeParser.php
- $form = $this->formFactory->create($item);
- } catch (FormException $e) {
+ }
+ catch (FormException $e) {
@stof

stof Oct 4, 2012

Contributor

Wrong CS here

Parser/FormTypeParser.php
+use Symfony\Component\OptionsResolver\Exception\MissingOptionsException;
+
+use Symfony\Component\Form\FormRegistry;
+
@stof

stof Oct 4, 2012

Contributor

please remove the empty lines here

Contributor

docteurklein commented Oct 11, 2012

any news ?

willdurand added a commit that referenced this pull request Oct 11, 2012

@willdurand willdurand merged commit bd625ae into nelmio:master Oct 11, 2012

Collaborator

willdurand commented Oct 11, 2012

Thank you, and sorry for the delay.

Contributor

docteurklein commented Oct 11, 2012

no problem! thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment