Skip to content

[DependencyInjection] Problem with unescaping class arguments #4665

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

Closed
wants to merge 1 commit into from
Closed

[DependencyInjection] Problem with unescaping class arguments #4665

wants to merge 1 commit into from

Conversation

avorobiev
Copy link

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT

Hello!
I've got an error when I tried to use DI to initiate Monolog logger with next settings:

services:
  logger_formatter:
    class:        \Monolog\Formatter\LineFormatter
    arguments:    [ "[%%datetime%%] %%channel%%.%%level_name%%: %%message%% %%context%%\n" ]  
  logger_handler:
    class:        \Monolog\Handler\StreamHandler
    arguments:
      - app.log
      - <?php echo \Monolog\Logger::DEBUG, PHP_EOL; ?>
    calls:
      - [ setFormatter, [ @logger_formatter ]]
  logger:
    class:        \Monolog\Logger
    arguments:    [ 'logger_name' ]
    calls:
      - [ pushHandler, [ @logger_handler ]]

If I set argument for service logger_formatter with single % (like "[%datetime%]…") then DI threw exception that it could not resolve reference because there was non-existent parameter "datetime".
If I set argument with double % (like "[%%datetime%%]…") then DI transferred parameter to the LineFormatter object without unescaping, and LineFormatter did not work properly.
The problem was in ContainerBuilder, because there was no unescaping class parameters.

I’ve fixed problem and added tests to cover such case.

@travisbot
Copy link

This pull request passes (merged 08fe216 into d0e1547).

@vicb
Copy link
Contributor

vicb commented Jul 2, 2012

@avorobiev Could you just make unescapeValue public and call it from the Container Builder ? (ie without adding a new method).

Please also add a note in the component changelog. Thanks.

@avorobiev
Copy link
Author

@vicb I've done #4707!

@avorobiev avorobiev closed this Jul 2, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants