8000 [Yaml] Added support for parsing PHP constants by HeahDude · Pull Request #18626 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Yaml] Added support for parsing PHP constants #18626

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 23, 2016
Merged

[Yaml] Added support for parsing PHP constants #18626

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
02d1dea
[Yaml] added support for parsing PHP constants
HeahDude Apr 23, 2016
17ec26e
[DI] added support for PHP constants in yaml configuration files
HeahDude Apr 25, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[Yaml] added support for parsing PHP constants
  • Loading branch information
@HeahDude
HeahDude committed Jun 23, 2016
commit 02d1dea33b50310a18f1dacd3361e76b9ac38aa2
9 changes: 9 additions & 0 deletions src/Symfony/Component/Yaml/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
CHANGELOG
=========

3.2.0
-----

* Added support for parsing PHP constants:

```php
Yaml::parse('!php/const:PHP_INT_MAX', Yaml::PARSE_CONSTANT);
```

3.1.0
-----

Expand Down
15 changes: 15 additions & 0 deletions src/Symfony/Component/Yaml/Inline.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class Inline
private static $exceptionOnInvalidType = false;
private static $objectSupport = false;
private static $objectForMap = false;
private static $constantSupport = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure we need a flag here? Couldn't we unconditionally enable this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xabbuh what do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depends on whether we want to throw exceptions when a constant is not defined. Imo the flag makes sense if we then throw an exception when a constant does not exist (which would make debugging a lot easier than when we just returned null). Though if we enabled this feature unconditionally, we would not be able to throw exceptions when the constant was not defined.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throwing the exception already depends on the exceptionOnInvalidType flag currently

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok then, they are many options.

  1. Parse constants by defaults:

    case 0 === strpos($scalar, '!php/const:'):
if (defined($const = substr($scalar, 11))) {
    return constant($const);
}
if (self::$exceptionOnInvalidType) {
    throw new ParseException(sprintf('The PHP constant "%s" is not defined.', $const));
}

return;

  2. Or using a flag as is:

    // use Symfony\Component\Yaml\Exception\RuntimeException;

case 0 === strpos($scalar, '!php/const:'):
if (self::$constantSupport) {
    $const = substr($scalar, 11);
    if (defined($const)) {
        return constant($const);
    }

    throw new RuntimeException(sprintf('The constant "%s" is not defined.', $const));
}
if (self::$exceptionOnInvalidType) {
    throw new ParseException(sprintf('The string "%s" could not be parsed as constant. Have you forgotten to use "Yaml::PARSE_CONSTANT" flag?', $scalar));
}

return;

  3. Suggestions?

For now I vote for option 2, since php objects are not parsed by default.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My vote is for a flag too because this component should be as close to the Yaml spec as possible. !php/const: is a Symfony thing, not a Yaml thing.

Copy link
Member
@xabbuh xabbuh Jun 15, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with that. It will consistent with how we already treat PHP object support.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me, then my comment above applies.


/**
* Converts a YAML string to a PHP array.
Expand Down Expand Up @@ -77,6 +78,7 @@ public static function parse($value, $flags = 0, $references = array())
self::$exceptionOnInvalidType = (bool) (Yaml::PARSE_EXCEPTION_ON_INVALID_TYPE & $flags);
self::$objectSupport = (bool) (Yaml::PARSE_OBJECT & $flags);
self::$objectForMap = (bool) (Yaml::PARSE_OBJECT_FOR_MAP & $flags);
self::$constantSupport = (bool) (Yaml::PARSE_CONSTANT & $flags);

$value = trim($value);

Expand Down Expand Up @@ -580,6 +582,19 @@ private static function evaluateScalar($scalar, $flags, $references = array())
throw new ParseException('Object support when parsing a YAML file has been disabled.');
}

return;
case 0 === strpos($scalar, '!php/const:'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about:

                    case 0 === strpos($scalar, '!php/const:'):
                        if (self::$constantSupport && defined($const = substr($scalar, 11))) {
                            return constant($const);
                        }
                        if (self::$exceptionOnInvalidType) {
                            throw new ParseException(sprintf('The "%s" YAML string could not be parsed because the %s.', $scalar, self::$constantSupport ? ' "Yaml::PARSE_CONSTANT" flag was not set' : 'corresponding PHP constant was not defined'));
                        }

                        return;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolas-grekas 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolas-grekas Working on it I think we should throw an exception when the constant is not defined even if self::$exceptionOnInvalidType is false, what do you think @symfony/deciders?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case maybe I should only change the YAML RuntimeException to a ParseException so it's caught in the DI loader.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think throwing an exception when a constant is not defined is a good idea.

if (self::$constantSupport) {
if (defined($const = substr($scalar, 11))) {
return constant($const);
}

throw new ParseException(sprintf('The constant "%s" is not defined.', $const));
}
if (self::$exceptionOnInvalidType) {
throw new ParseException(sprintf('The string "%s" could not be parsed as a constant. Have you forgotten to pass the "Yaml::PARSE_CONSTANT" flag to the parser?', $scalar));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must throw an exception when constant support is disabled and $exceptionOnInvalidType is set (like we do for object support).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright!


return;
case 0 === strpos($scalar, '!!float '):
return (float) substr($scalar, 8);
Expand Down
38 changes: 38 additions & 0 deletions src/Symfony/Component/Yaml/Tests/InlineTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,44 @@ public function testParseWithMapObjects($yaml, $value)
$this->assertSame(serialize($value), serialize($actual));
}

/**
* @dataProvider getTestsForParsePhpConstants
*/
public function testParsePhpConstants($yaml, $value)
{
$actual = Inline::parse($yaml, Yaml::PARSE_CONSTANT);

$this->assertSame($value, $actual);
}

public function getTestsForParsePhpConstants()
{
return array(
array('!php/const:Symfony\Component\Yaml\Yaml::PARSE_CONSTANT', Yaml::PARSE_CONSTANT),
array('!php/const:PHP_INT_MAX', PHP_INT_MAX),
array('[!php/const:PHP_INT_MAX]', array(PHP_INT_MAX)),
array('{ foo: !php/const:PHP_INT_MAX }', array('foo' => PHP_INT_MAX)),
);
}

/**
* @expectedException \Symfony\Component\Yaml\Exception\ParseException
* @expectedExceptionMessage The constant "WRONG_CONSTANT" is not defined
*/
public function testParsePhpConstantThrowsExceptionWhenUndefined()
{
Inline::parse('!php/const:WRONG_CONSTANT', Yaml::PARSE_CONSTANT);
}

/**
* @expectedException \Symfony\Component\Yaml\Exception\ParseException
* @expectedExceptionMessageRegExp #The string "!php/const:PHP_INT_MAX" could not be parsed as a constant.*#
*/
public function testParsePhpConstantThrowsExceptionOnInvalidType()
{
Inline::parse('!php/const:PHP_INT_MAX', Yaml::PARSE_EXCEPTION_ON_INVALID_TYPE);
}

/**
* @group legacy
* @dataProvider getTestsForParseWithMapObjects
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/Yaml/Yaml.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class Yaml
const PARSE_DATETIME = 32;
const DUMP_OBJECT_AS_MAP = 64;
const DUMP_MULTI_LINE_LITERAL_BLOCK = 128;
const PARSE_CONSTANT = 256;

/**
* Parses YAML into a PHP value.
Expand Down
0