Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[HttpKernel] trim the leading backslash in the controller init#32541

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@Simperfit
Copy link
Contributor

QA
Branch?4.4
Bug fix?no
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#29945
LicenseMIT
Doc PRnone

This fixes an error where the classes exists when using a leading backslash in the controller, it's not invalid to do so.

see#30045 (comment)

@SimperfitSimperfitforce-pushed thefeature/trim-backslash-a-the-start-of-a-controller branch fromdce0fc6 toa8ec428CompareJuly 15, 2019 06:43
@Simperfit
Copy link
ContributorAuthor

updated

Status: Needs Review

@fabpot
Copy link
Member

I would trim only when really needed. What makes trimming neeeded in the first place?

@Simperfit
Copy link
ContributorAuthor

@fabpot Using a leading backslash will made the `instiateController method fail because it can find the class, according to#30045 (comment), I've update this method to trim when an leading blackslash is passed and to fix this behaviour.

@fabpot
Copy link
Member

You've changed a method that deals with errors, so this looks suspicious to me.

@SimperfitSimperfitforce-pushed thefeature/trim-backslash-a-the-start-of-a-controller branch froma8ec428 to029f653CompareJuly 15, 2019 07:23
@Simperfit
Copy link
ContributorAuthor

Okay I've misread the whole think, was only thinking about the comment, but hey, this is the right fix and I understand why is was deprecated. So maybe the even better fix is to deprecate using backslash like in the first PR that has been closed ;).

WDYT@fabpot ?

@nicolas-grekasnicolas-grekas added this to thenext milestoneJul 15, 2019
@fabpot
Copy link
Member

I think I still don't get the problem. Can you explain it to me again? Perhaps with an example?

@Pierstoval
Copy link
Contributor

IIUC, this PR fixes issues when defining routes like this:

my_route:path:/whatevercontroller:\App\Controller\DefaultController

Not sure it's really common, but IMO this should be supported anyway

@fabpotfabpotforce-pushed thefeature/trim-backslash-a-the-start-of-a-controller branch from029f653 to8be6297CompareAugust 9, 2019 12:13
@fabpotfabpot changed the base branch from4.4 to4.3August 9, 2019 12:15
@fabpotfabpotforce-pushed thefeature/trim-backslash-a-the-start-of-a-controller branch from8be6297 to3c8d395CompareAugust 9, 2019 12:15
@fabpot
Copy link
Member

Thank you@Simperfit.

Simperfit and lucchese-pd reacted with heart emoji

@fabpotfabpot merged commit3c8d395 intosymfony:4.3Aug 9, 2019
fabpot added a commit that referenced this pull requestAug 9, 2019
…init (Simperfit, fabpot)This PR was submitted for the 4.4 branch but it was merged into the 4.3 branch instead (closes#32541).Discussion----------[HttpKernel] trim the leading backslash in the controller init| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->| BC breaks?    | no     <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->| Fixed tickets |#29945| License       | MIT| Doc PR        | none <!-- required for new features --><!--Replace this notice by a short README for your feature/bugfix. This will help peopleunderstand your PR and can be used as a start for the documentation.Additionally (seehttps://symfony.com/roadmap): - Bug fixes must be submitted against the lowest maintained branch where they apply   (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against branch 4.4. - Legacy code removals go to the master branch.-->This fixes an error where the classes exists when using a leading backslash in the controller, it's not invalid to do so.see#30045 (comment)Commits-------3c8d395 [HttpKernel] fixed class having a leading \ in a route controller6fdf252 [HttpKernel] trim the leading backslash in the controller init
@SimperfitSimperfit deleted the feature/trim-backslash-a-the-start-of-a-controller branchAugust 9, 2019 12:16
@fabpotfabpot mentioned this pull requestAug 26, 2019
@nicolas-grekasnicolas-grekas modified the milestones:next,4.4Oct 27, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

+1 more reviewer

@PierstovalPierstovalPierstoval left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

5 participants

@Simperfit@fabpot@Pierstoval@nicolas-grekas@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp