Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Failing injector test for components registered with escaped slashes #22

Merged
merged 1 commit into from
Oct 17, 2016
Merged

Conversation

eimanavicius
Copy link
Contributor

related to Config injector do not support escaped slashes in modules names #21

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling d97d2c2 on eimanavicius:master into * on zendframework:master*.

@weierophinney
Copy link
Member

The installer will inject the name verbatim as it appears in the extra.zf composer configuration, which means that a subsequent install attempt will match it.

With your test case, you're essentially proposing a new scenario: an edited module list that uses different escaping from what is in the composer.json. I would expect that to fail, to be honest; you shouldn't be manipulating the escaping in your configuration files, and, if you do, there will likely be issues.

@eimanavicius
Copy link
Contributor Author

I found this issue while working with ZF Apigility.

When you checkout the Apigility Skeleton you get modules.config.php were modules names are not escaped with backslashes.
Then you go to admin and add a new api (or do any other action that updates modules.config.php).
Your modules.config.php gets updated with all backslashes escaped.

I think that this is not only one scenarios that can introduce escaped slashes in module names.

Also when project supports short arrays and long arrays syntax, why it should not support very legal slash escaping in strings.

@weierophinney weierophinney merged commit d97d2c2 into zendframework:master Oct 17, 2016
weierophinney added a commit that referenced this pull request Oct 17, 2016
weierophinney added a commit that referenced this pull request Oct 17, 2016
weierophinney added a commit that referenced this pull request Oct 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants