loadServices: tighten up file match regex #16377
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before: Civi code living under a path containing the characters php (such as ~/src/php/civicrm) will crash trying to load services with a message like the following: ReflectionException: Class Civi\Api4\Service\Spec\Provider\src does not exist in ReflectionClass->__construct() After: Civi loads services normally no matter what the enclosing path
|
(Standard links)
|
totten
reviewed
Jan 26, 2020
| @@ -86,7 +86,7 @@ public static function loadServices($namespace, $tag, $container) { | |||
| $container->addResource($resource); | |||
| foreach (glob("$path*.php") as $file) { | |||
| $matches = []; | |||
| preg_match('/(\w*).php/', $file, $matches); | |||
| preg_match('/(\w*)\.php$/', $file, $matches); | |||
There was a problem hiding this comment.
I haven't tested this scenario, but this does look like a more correct implementation of "find things ending in *.php" 👍
There was a problem hiding this comment.
@totten i'm guessing also if there was a problem with this right then the api_v4_unit tests would fail right because this merges in the core resources + extension resources
|
Given Tim's supportive comment, i tested this out by applying the regex locally and installed the deduper and afform extensions and confirmed that this still worked as expected. |
ejegg
added a commit
to ejegg/civicrm-core
that referenced
this pull request
Feb 21, 2020
Before: Civi code living under a path containing the characters php
(such as ~/src/php/civicrm) will fail to load the API4 explorer with
a message like the following:
ReflectionException: "Class \Civi\Api4\src does not exist"
After: API4 explorer loads no matter what the enclosing path
Very similar to PR civicrm#16377
ejegg
added a commit
to ejegg/civicrm-core
that referenced
this pull request
Feb 21, 2020
Before: Civi code living under a path containing the characters php
(such as ~/src/php/civicrm) will fail to load the API4 explorer with
a message like the following:
ReflectionException: "Class \Civi\Api4\src does not exist"
After: API4 explorer loads no matter what the enclosing path
Very similar to PR civicrm#16377
magnolia61
pushed a commit
to magnolia61/civicrm-core
that referenced
this pull request
Mar 3, 2020
Before: Civi code living under a path containing the characters php
(such as ~/src/php/civicrm) will fail to load the API4 explorer with
a message like the following:
ReflectionException: "Class \Civi\Api4\src does not exist"
After: API4 explorer loads no matter what the enclosing path
Very similar to PR civicrm#16377
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Tighten up a regular expression used to find files so that it doesn't match a partial path.
Before
Civi code living under a path containing the characters 'php'
(such as ~/src/php/civicrm) will crash trying to load services with
a message like the following:
ReflectionException: Class Civi\Api4\Service\Spec\Provider\src
does not exist in ReflectionClass->__construct()
After
Civi loads services normally no matter what the enclosing path
Technical Details
The regex needs a $ anchor at the end because it's looking for files ending in php. It also needed the '.' escaped so it only matches a literal '.' and not any character.
Comments
There's another PR against master: #16376