Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DomCrawler] Fails in PHP7.2 for same name fields, different types #28179

Closed
michsk opened this issue Aug 9, 2018 · 3 comments
Closed

[DomCrawler] Fails in PHP7.2 for same name fields, different types #28179

michsk opened this issue Aug 9, 2018 · 3 comments

Comments

@michsk
Copy link

michsk commented Aug 9, 2018

Symfony version(s) affected: x.y.z
"name": "symfony/dom-crawler",
"version": "v2.7.49",

Description
Codeception/Codeception#5119
#11689
#11692

<form id="form" action="/index.php" method="post">
<input type="text" id="originator" class="alpha form-control" name="form[origin]" value="thisshouldbeinpost" placeholder="Afzender" maxLength="18">
<select id="originator" name="form[origin]" disabled>
<option value="one">one</option>
<option value="two">two</option>
<option value="tree">tree</option>
</select>
<input type="submit">
</form>

So it seems that the fix mentioned above does not work for PHP7.2. In the above example. The field of the disabled select is chosen over the input field. This is not what PHP would do. Because the field is disabled, the POST does not contain the field.

Possible Solution
Investigate on how to allow different types to be used as well in \Symfony\Component\DomCrawler\Form::addField

@michsk michsk changed the title [DomCrawler] Fails in PHP7.2 for same name fields [DomCrawler] Fails in PHP7.2 for same name fields, different types Aug 9, 2018
@michsk
Copy link
Author

michsk commented Aug 9, 2018

Could it be the order of nodes is changed? Because im amazed that this did work in PHP7.0. I looked at the changelog for php but couldnt find anything. What i could imagine is that the order changed, and that before we had the select first, and than the input. Which would make the input override the select.

@michsk
Copy link
Author

michsk commented Aug 9, 2018

I think we should just exclude disabled form elements when searching for form elements

@michsk
Copy link
Author

michsk commented Aug 9, 2018

So changing
vendor/symfony/dom-crawler/Form.php:456

to ignore the disabled fields. Just like PHP would do, and just like we do when requesting the values in getValues. Fixes this issue. Since the correct input value is not overwritten with the disabled field.

        if (!$node->hasAttribute('name') || !$node->getAttribute('name') || $node->hasAttribute('disabled')) {
            return;
        }

Guess this would also mean we could dump the $field->isDisabled() in public function getValues(), since we don't use the field anymore.

@fabpot fabpot closed this as completed Feb 3, 2020
fabpot added a commit that referenced this issue Feb 3, 2020
This PR was merged into the 3.4 branch.

Discussion
----------

[DomCrawler] Skip disabled fields processing in Form

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #28179
| License       | MIT

Commits
-------

c73b042 bug #28179 [DomCrawler] Skip disabled fields processing in Form
nicolas-grekas added a commit that referenced this issue Feb 3, 2020
* 3.4:
  [Phpunit] Fix running skipped tests expecting only deprecations
  [DependencyInjection] #35505 Fix typo in test name
  [Yaml][Inline] Fail properly on empty object tag and empty const tag
  Check non-null type for numeric type
  Check value isset to avoid PHP notice
  bug #28179 [DomCrawler] Skip disabled fields processing in Form
chalasr added a commit that referenced this issue Feb 3, 2020
* 4.4:
  [Phpunit] Fix running skipped tests expecting only deprecations
  Fix merge
  [Config] dont catch instances of Error
  [HttpClient] fix HttpClientDataCollector when handling canceled responses
  [DependencyInjection] #35505 Fix typo in test name
  [Yaml][Inline] Fail properly on empty object tag and empty const tag
  Check non-null type for numeric type
  Check value isset to avoid PHP notice
  bug #28179 [DomCrawler] Skip disabled fields processing in Form
chalasr added a commit that referenced this issue Feb 3, 2020
* 5.0:
  [Phpunit] Fix running skipped tests expecting only deprecations
  Fix merge
  [Config] dont catch instances of Error
  [HttpClient] fix HttpClientDataCollector when handling canceled responses
  [FrameworkBundle] remove mention of the old Controller class
  [DependencyInjection] #35505 Fix typo in test name
  [Yaml][Inline] Fail properly on empty object tag and empty const tag
  Check non-null type for numeric type
  Check value isset to avoid PHP notice
  bug #28179 [DomCrawler] Skip disabled fields processing in Form
dmaicher added a commit to dmaicher/symfony that referenced this issue Mar 3, 2020
fabpot added a commit that referenced this issue Mar 3, 2020
…ssing in Form" (dmaicher)

This PR was merged into the 3.4 branch.

Discussion
----------

Revert "bug #28179 [DomCrawler] Skip disabled fields processing in Form"

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | #35923
| License       | MIT
| Doc PR        | -

Reverts #34059

See #35923

Commits
-------

af17f5a Revert "bug #28179 [DomCrawler] Skip disabled fields processing in Form"
nicolas-grekas added a commit that referenced this issue Mar 11, 2020
* 3.4:
  [Yaml] fix dumping strings containing CRs
  [DI] Fix XmlFileLoader bad error message
  Tweak message
  improve PlaintextPasswordEncoder docBlock summary
  [Validator] Add two missing translations for the Arabic (ar) locale
  Use some PHP 5.4 constants unconditionally
  Revert "bug #28179 [DomCrawler] Skip disabled fields processing in Form"
  Add Spanish translation
  Fix typo
  [Validator] add Japanese translation
  Fix typo
  Add Polish translation
  [SecurityBundle] Minor fixes in configuration tree builder
  bumped Symfony version to 3.4.39
  updated VERSION for 3.4.38
  update CONTRIBUTORS for 3.4.38
  updated CHANGELOG for 3.4.38
This was referenced Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants