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

Using a PHP class constant for a key in a YAML hash as a method argument produces a ParseException #36624

Closed
rhift opened this issue Apr 29, 2020 · 3 comments

Comments

@rhift
Copy link

rhift commented Apr 29, 2020

Symfony version(s) affected: 4.4.7

Description
Virtually the same as #35179 but with different exceptions.

When trying to use a PHP class constant as a YAML key for a hash (as a method argument), I receive one of the following.

  • a \Symfony\Component\Yaml\Exception\ParseException with the message The constant "\VENDOR\PRODUCT\Example" is not defined at line 7 (near "!php/const \VENDOR\PRODUCT\Example"). if I use YAML Variant A below.
  • a \Symfony\Component\Yaml\Exception\ParseException with the message Malformed inline YAML string: ""\VENDOR\PRODUCT\Example" at line 7 (near ""\VENDOR\PRODUCT\Example"). if I use YAML Variant B below. I observe the same behavior when using single quotes.

How to reproduce
Using the following class and Symfony YAML DI files produces the above.

<?php
declare(strict_types=1);

namespace VENDOR\PRODUCT;

class Example
{
    public const CONSTANT_1 = 'constant_1';
    public const CONSTANT_2 = 'constant_2';

    private $Options;

    public function addOptions(array $options): Example
    {
        $this->Options[] = $options;

        return $this;
    }
}
# YAML Variant A
services:
  VENDOR\PRODUCT\Example:
    class: VENDOR\PRODUCT\Example
    public: false # probably unrelated
    shared: true # probably unrelated
    calls:
      - [addOptions, [{
          !php/const \VENDOR\PRODUCT\Example::CONSTANT_1: 'value_0',
          key_1: "value_1",
          key_2: "value_2"
        }]]
# YAML Variant B
services:
  VENDOR\PRODUCT\Example:
    class: VENDOR\PRODUCT\Example
    public: false # probably unrelated
    shared: true # probably unrelated
    calls:
      - [addOptions, [{
          !php/const "\VENDOR\PRODUCT\Example::CONSTANT_1": 'value_0',
          key_1: "value_1",
          key_2: "value_2"
        }]]

Also, the control case and global PHP constants (in both variants) work as expected, i.e:

# YAML Variant A
services:
  VENDOR\PRODUCT\Example:
    class: VENDOR\PRODUCT\Example
    public: false # probably unrelated
    shared: true # probably unrelated
    calls:
      - [addOptions, [{
          !php/const PHP_INT_MAX: "value_0",
          key_1: "value_1",
          key_2: "value_2"
        }]]
# YAML Variant B
services:
  VENDOR\PRODUCT\Example:
    class: VENDOR\PRODUCT\Example
    public: false # probably unrelated
    shared: true # probably unrelated
    calls:
      - [addOptions, [{
          !php/const "PHP_INT_MAX": "value_0",
          key_1: "value_1",
          key_2: "value_2"
        }]]
@rhift rhift added the Bug label Apr 29, 2020
@rhift rhift changed the title Using a PHP class constant for a key in a YAML hash as a method argument produces an ParseException Using a PHP class constant for a key in a YAML hash as a method argument produces a ParseException Apr 29, 2020
@xabbuh
Copy link
Member

xabbuh commented Apr 30, 2020

Thank you for the example. This helped to reproduce it. It looks like the parser still doesn't handle constants well in all places.

For example, this doesn't work:

App\Example:
    calls:
      - method: addOptions
        arguments:
          - !php/const App\Example::CONSTANT_1: 'value_0'
            key_1: "value_1"
            key_2: "value_2"

while this parses correctly:

App\Example:
    calls:
      - method: addOptions
        arguments:
          - key_1: "value_1"
            !php/const App\Example::CONSTANT_1: 'value_0'
            key_2: "value_2"

@rhift
Copy link
Author

rhift commented Apr 30, 2020

Absolutely, I wasn't really sure what was wrong so I just tried to give the detail that I had at least.

Oh neat!! I didn't realize that that syntax existed for invoking methods, that's so much better! And thanks for the explanation, I reproduced it and have a much clearer understanding of what is wrong now.

Thanks a ton for tracking this down @xabbuh, really appreciated.

@xabbuh
Copy link
Member

xabbuh commented Apr 30, 2020

Thank you for providing this very good example that allows to reproduce it. I don't know how hard it will be to fully fix this, but now I at least have some test input for the parser. 😃

@fabpot fabpot closed this as completed Aug 11, 2020
fabpot added a commit that referenced this issue Aug 11, 2020
…block (jnye)

This PR was squashed before being merged into the 3.4 branch.

Discussion
----------

[Yaml] Fix for #36624; Allow PHP constant as first key in block

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

When using a PHP constant in the first key of a mapping the parser would throw an exception. However if you used a PHP constant in any other key but the first it was allowed. This fix allows PHP constant as the first key.

I've included a test for this parser error along with the fix.

Commits
-------

46f635c [Yaml] Fix for #36624; Allow PHP constant as first key in block
fabpot added a commit that referenced this issue Aug 13, 2020
* 3.4:
  [Yaml] Fix for #36624; Allow PHP constant as first key in block
  Use PHPUnit 9.3 on php 8.
  fix mapping errors from unmapped forms
  [Validator] Add target guards for Composite nested constraints
fabpot added a commit that referenced this issue Aug 13, 2020
* 4.4:
  [Validator] RangeTest: fix expected deprecation
  [Yaml] Fix for #36624; Allow PHP constant as first key in block
  Use PHPUnit 9.3 on php 8.
  fix mapping errors from unmapped forms
  [Validator] Add target guards for Composite nested constraints
fabpot added a commit that referenced this issue Aug 13, 2020
* 5.1:
  [Validator] RangeTest: fix expected deprecation
  Fix bad merge
  [Yaml] Fix for #36624; Allow PHP constant as first key in block
  Use PHPUnit 9.3 on php 8.
  fix mapping errors from unmapped forms
  [Validator] Add target guards for Composite nested constraints
This was referenced Aug 31, 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