8000 [HttpKernel] Fix invalid Container class name by yceruto · Pull Request #20786 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[HttpKernel] Fix invalid Container class name #20786

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

Conversation

yceruto
Copy link
Member
@yceruto yceruto commented Dec 6, 2016
Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Steps to reproduce this bug (based on Building your own Framework with the MicroKernelTrait):

  1. $ git clone https://github.com/yceruto/micro-kernel 123foobar
  2. $ cd 123foobar
  3. $ composer update
  4. $ php index.php

Result:

Parse error: syntax error, unexpected '123' (T_LNUMBER), expecting identifier (T_STRING) in /var/www/vhost/123foobar/cache/dev/123foobarDevDebugProjectContainer.php on line 16

The solution was taken from http://php.net/manual/en/language.oop5.basic.php:

A valid class name starts with a letter or underscore, followed by any number of letters, numbers, or underscores. As a regular expression, it would be expressed thus: ^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$.

@stof
Copy link
Member
stof commented Dec 6, 2016

Please add a test covering this

@stof stof added this to the 2.7 milestone Dec 6, 2016
@yceruto yceruto force-pushed the fix_container_class_name branch from 27bf28a to 39aa638 Compare December 6, 2016 15:25
@fabpot
Copy link
Member
fabpot commented Dec 6, 2016

duplicate #20750

@yceruto
Copy link
Member Author
yceruto commented Dec 6, 2016

Opps! do not check this, sorry

@yceruto yceruto closed this Dec 6, 2016
@yceruto yceruto deleted the fix_container_class_name branch December 6, 2016 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0