Skip to content

Commit e49a9a5

Browse files
c15k0c15k0
committed
Refactor core helpers and traits to reduce complexity and harden flows
1 parent 94945a2 commit e49a9a5

6 files changed

Lines changed: 262 additions & 60 deletions

File tree

‎.github/workflows/snyk-security.yml‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,26 @@
1-
name: Snyk Scan
1+
name: Snyk Security Scan
22

3-
on: [push, pull_request]
3+
on:
4+
push:
5+
pull_request:
46

57
jobs:
68
snyk:
79
runs-on: ubuntu-latest
810

911
steps:
10-
- uses: actions/checkout@v4
12+
- name: Checkout
13+
uses: actions/checkout@v4
1114

12-
- name: Install Snyk
15+
- name: Install Snyk CLI
1316
run: npm install -g snyk
1417

15-
- name: Authenticate Snyk
16-
run: snyk auth ${{ secrets.SNYK_TOKEN }}
18+
- name: Run Snyk Code (no fail)
19+
env:
20+
SNYK_TOKEN: ${{ secrets.SNYK_TOKEN }}
21+
run: snyk code test --sarif > snyk-code.sarif || true
1722

18-
- name: Run Snyk Code
19-
run: snyk code test --sarif > snyk-code.sarif
23+
- name: Upload SARIF to GitHub Security
24+
uses: github/codeql-action/upload-sarif@v3
25+
with:
26+
sarif_file: snyk-code.sarif

‎src/base/types/helpers/InjectorHelper.php‎

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,14 @@ public static function extractProperties(
6969
return self::extractInjectableProperties($reflector, $type);
7070
}
7171

72+
return self::extractPatternMatchedProperties($reflector, (int)$type, (string)$pattern);
73+
}
74+
75+
private static function extractPatternMatchedProperties(ReflectionClass $reflector, int $type, string $pattern): array
76+
{
7277
$properties = [];
7378
foreach ($reflector->getProperties($type) as $property) {
74-
$doc = $property->getDocComment() ?: '';
79+
$doc = self::propertyDoc($property);
7580
if (preg_match($pattern, $doc) === 1) {
7681
$instanceType = self::extractVarType($doc, $property);
7782
if (null !== $instanceType) {
@@ -135,7 +140,7 @@ private static function matchesPropertyVisibility(ReflectionProperty $property,
135140
*/
136141
public static function extractVarType($doc, ReflectionProperty $property = null)
137142
{
138-
return MetadataReader::extractVarType($property, $doc ?: '');
143+
return MetadataReader::extractVarType($property, self::docValue($doc));
139144
}
140145

141146
/**
@@ -144,13 +149,14 @@ public static function extractVarType($doc, ReflectionProperty $property = null)
144149
*/
145150
public static function checkIsRequired($doc, ReflectionProperty $property = null)
146151
{
152+
$doc = self::docValue($doc);
147153
if (null !== $property) {
148-
$required = MetadataReader::getTagValue('required', $doc ?: '', null, $property);
154+
$required = MetadataReader::getTagValue('required', $doc, null, $property);
149155
if (null !== $required) {
150156
return (bool)$required;
151157
}
152158
}
153-
return preg_match('/@required/', $doc ?: '', $matches) === 1 && (bool)count($matches);
159+
return preg_match('/@required/', $doc, $matches) === 1 && (bool)count($matches);
154160
}
155161

156162
/**
@@ -168,7 +174,7 @@ public static function checkIsVisible($doc)
168174
*/
169175
public static function getLabel($doc, ReflectionProperty $property = null)
170176
{
171-
return t(AnnotationHelper::extractReflectionLabel($doc ?: '', $property));
177+
return t(AnnotationHelper::extractReflectionLabel(self::docValue($doc), $property));
172178
}
173179

174180
/**
@@ -177,12 +183,12 @@ public static function getLabel($doc, ReflectionProperty $property = null)
177183
*/
178184
public static function getValues($doc, ReflectionProperty $property = null)
179185
{
180-
$values = AnnotationHelper::extractFromDoc('values', $doc ?: '', '', $property);
186+
$values = AnnotationHelper::extractFromDoc('values', self::docValue($doc), '', $property);
181187
if (is_array($values)) {
182188
return $values;
183189
}
184-
if (is_string($values) && false !== strpos($values, '|')) {
185-
return explode('|', $values);
190+
if (is_string($values)) {
191+
return self::splitDelimitedValues($values);
186192
}
187193
return $values;
188194
}
@@ -193,7 +199,7 @@ public static function getValues($doc, ReflectionProperty $property = null)
193199
*/
194200
public static function getDefaultValue($doc, ReflectionProperty $property = null)
195201
{
196-
return AnnotationHelper::extractFromDoc('default', $doc ?: '', null, $property);
202+
return AnnotationHelper::extractFromDoc('default', self::docValue($doc), null, $property);
197203
}
198204

199205
/**
@@ -270,4 +276,25 @@ public static function getClassProperties($class)
270276
return $properties;
271277
}
272278

279+
private static function propertyDoc(ReflectionProperty $property): string
280+
{
281+
return (string)($property->getDocComment() ?: '');
282+
}
283+
284+
private static function docValue(mixed $doc): string
285+
{
286+
return is_string($doc) ? $doc : '';
287+
}
288+
289+
/**
290+
* @return string|array<int, string>
291+
*/
292+
private static function splitDelimitedValues(string $values): string|array
293+
{
294+
if (str_contains($values, '|')) {
295+
return explode('|', $values);
296+
}
297+
return $values;
298+
}
299+
273300
}

‎src/base/types/helpers/ResponseCookieHelper.php‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,12 @@ class ResponseCookieHelper
66
{
77
public static function buildCookiePayload(array $cookie, bool $isSecureRequest, ?string $defaultDomain): ?array
88
{
9-
if (!array_key_exists('name', $cookie) || !array_key_exists('value', $cookie)) {
9+
if (!self::hasRequiredCookieKeys($cookie)) {
1010
return null;
1111
}
1212

13-
$httpOnly = array_key_exists('httpOnly', $cookie)
14-
? (bool)$cookie['httpOnly']
15-
: ((array_key_exists('http', $cookie)) ? (bool)$cookie['http'] : true);
16-
$secure = array_key_exists('secure', $cookie)
17-
? (bool)$cookie['secure']
18-
: $isSecureRequest;
13+
$httpOnly = self::resolveHttpOnlyFlag($cookie);
14+
$secure = self::resolveSecureFlag($cookie, $isSecureRequest);
1915
$sameSite = self::normalizeSameSite((string)($cookie['sameSite'] ?? $cookie['samesite'] ?? 'Lax'));
2016
$cookieDomain = self::normalizeCookieDomain((string)($cookie['domain'] ?? $defaultDomain));
2117

@@ -41,6 +37,30 @@ public static function buildCookiePayload(array $cookie, bool $isSecureRequest,
4137
];
4238
}
4339

40+
private static function hasRequiredCookieKeys(array $cookie): bool
41+
{
42+
return array_key_exists('name', $cookie) && array_key_exists('value', $cookie);
43+
}
44+
45+
private static function resolveHttpOnlyFlag(array $cookie): bool
46+
{
47+
if (array_key_exists('httpOnly', $cookie)) {
48+
return (bool)$cookie['httpOnly'];
49+
}
50+
if (array_key_exists('http', $cookie)) {
51+
return (bool)$cookie['http'];
52+
}
53+
return true;
54+
}
55+
56+
private static function resolveSecureFlag(array $cookie, bool $isSecureRequest): bool
57+
{
58+
if (array_key_exists('secure', $cookie)) {
59+
return (bool)$cookie['secure'];
60+
}
61+
return $isSecureRequest;
62+
}
63+
4464
public static function normalizeCookieDomain(?string $domain): ?string
4565
{
4666
if (empty($domain)) {

‎src/base/types/traits/Api/MutationTrait.php‎

Lines changed: 104 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -143,21 +143,31 @@ private function getTableMap()
143143
*/
144144
protected function getPkDbName()
145145
{
146-
$tableMap = $this->getTableMap();
146+
$tableMap = $this->requireTableMapForPrimaryKeys();
147147
$tableName = $this->resolveTableName($tableMap);
148148
$pks = $tableMap->getPrimaryKeys();
149-
if (count($pks) === 1) {
150-
$pks = array_keys($pks);
151-
return [
152-
$tableName . '.' . $pks[0] => Api::API_MODEL_KEY_FIELD
153-
];
149+
$pkCount = count($pks);
150+
if ($pkCount === 1) {
151+
return $this->buildSinglePkMap($tableName, $pks);
154152
}
155-
if (count($pks) > 1) {
153+
if ($pkCount > 1) {
156154
return $this->buildCompositePkMap($tableName, $pks);
157155
}
158156
throw new ApiException(t('The API model is not properly mapped, there is no Primary Key or it is composite'));
159157
}
160158

159+
/**
160+
* @throws ApiException
161+
*/
162+
private function requireTableMapForPrimaryKeys(): TableMap
163+
{
164+
$tableMap = $this->getTableMap();
165+
if (!$tableMap instanceof TableMap) {
166+
throw new ApiException(t('The API model is not properly mapped, there is no Primary Key or it is composite'));
167+
}
168+
return $tableMap;
169+
}
170+
161171
private function resolveTableName(TableMap $tableMap): string
162172
{
163173
$tableMapClass = get_class($tableMap);
@@ -173,25 +183,50 @@ private function resolveTableName(TableMap $tableMap): string
173183
return $tableName;
174184
}
175185

186+
/**
187+
* @param array<int, ColumnMap> $primaryKeys
188+
* @return array<string, string>
189+
*/
190+
private function buildSinglePkMap(string $tableName, array $primaryKeys): array
191+
{
192+
$pkKeys = array_keys($primaryKeys);
193+
$pkName = (string)($pkKeys[0] ?? '');
194+
195+
return [
196+
$tableName . '.' . $pkName => Api::API_MODEL_KEY_FIELD,
197+
];
198+
}
199+
176200
/**
177201
* @param array<int, ColumnMap> $primaryKeys
178202
* @return array<string, string>
179203
*/
180204
private function buildCompositePkMap(string $tableName, array $primaryKeys): array
181205
{
182206
$apiPks = [];
183-
$principal = '';
184-
$sep = 'CONCAT(';
207+
$segments = [];
185208
foreach ($primaryKeys as $pk) {
186209
$apiPks[$tableName . '.' . $pk->getName()] = $pk->getPhpName();
187-
$principal .= $sep . $tableName . '.' . $pk->getName();
188-
$sep = ', "' . Api::API_PK_SEPARATOR . '", ';
210+
$segments[] = $tableName . '.' . $pk->getName();
189211
}
190-
$principal .= ')';
212+
$principal = $this->buildCompositePkExpression($segments);
191213
$apiPks[$principal] = Api::API_MODEL_KEY_FIELD;
192214
return $apiPks;
193215
}
194216

217+
/**
218+
* @param array<int, string> $segments
219+
*/
220+
private function buildCompositePkExpression(array $segments): string
221+
{
222+
if (count($segments) === 0) {
223+
return 'CONCAT()';
224+
}
225+
226+
$glue = ', "' . Api::API_PK_SEPARATOR . '", ';
227+
return 'CONCAT(' . implode($glue, $segments) . ')';
228+
}
229+
195230
/**
196231
* @throws ApiException
197232
*/
@@ -207,13 +242,24 @@ protected function addPkToList()
207242
*/
208243
private function addClassListName(TableMap $tableMap)
209244
{
210-
$pks = '';
211-
$sep = '';
245+
$segments = [];
212246
foreach ($tableMap->getPrimaryKeys() as $pk) {
213-
$pks .= $sep . $pk->getFullyQualifiedName();
214-
$sep = ', "|", ';
247+
$segments[] = $pk->getFullyQualifiedName();
215248
}
216-
$this->extraColumns['CONCAT("' . $tableMap->getPhpName() . ' #", ' . $pks . ')'] = Api::API_LIST_NAME_FIELD;
249+
250+
$this->extraColumns[$this->buildClassListNameExpression((string)$tableMap->getPhpName(), $segments)] = Api::API_LIST_NAME_FIELD;
251+
}
252+
253+
/**
254+
* @param array<int, string> $segments
255+
*/
256+
private function buildClassListNameExpression(string $phpName, array $segments): string
257+
{
258+
if (count($segments) === 0) {
259+
return 'CONCAT("' . $phpName . ' #")';
260+
}
261+
262+
return 'CONCAT("' . $phpName . ' #", ' . implode(', "|", ', $segments) . ')';
217263
}
218264

219265

@@ -237,22 +283,38 @@ protected function addDefaultListField()
237283
*/
238284
private function addExtraColumns(ModelCriteria &$query, $action)
239285
{
240-
if (Api::API_ACTION_LIST === $action) {
241-
// Legacy tokens kept for compatibility (`__name__`, `__pk`).
242-
// Planned future cleanup can remove them behind a versioned contract switch.
243-
$this->addDefaultListField();
244-
$this->addPkToList();
286+
$this->prepareLegacyListExtraColumns((string)$action);
287+
if (empty($this->extraColumns)) {
288+
return;
245289
}
246-
if (!empty($this->extraColumns)) {
247-
$fields = $this->resolveRequestedExtraFields();
248-
foreach ($this->extraColumns as $expression => $columnName) {
249-
if (empty($fields) || in_array($columnName, $fields, true)) {
250-
$query->withColumn($expression, $columnName);
251-
}
290+
291+
$fields = $this->resolveRequestedExtraFields();
292+
foreach ($this->extraColumns as $expression => $columnName) {
293+
if ($this->shouldIncludeExtraColumn($columnName, $fields)) {
294+
$query->withColumn($expression, $columnName);
252295
}
253296
}
254297
}
255298

299+
private function prepareLegacyListExtraColumns(string $action): void
300+
{
301+
if (Api::API_ACTION_LIST !== $action) {
302+
return;
303+
}
304+
// Legacy tokens kept for compatibility (`__name__`, `__pk`).
305+
// Planned future cleanup can remove them behind a versioned contract switch.
306+
$this->addDefaultListField();
307+
$this->addPkToList();
308+
}
309+
310+
/**
311+
* @param array<int, string> $fields
312+
*/
313+
private function shouldIncludeExtraColumn(string $columnName, array $fields): bool
314+
{
315+
return empty($fields) || in_array($columnName, $fields, true);
316+
}
317+
256318
/**
257319
* @return array
258320
*/
@@ -338,10 +400,21 @@ protected function resolveRequestedExtraFields(): array
338400
if (Config::getParam('api.extrafields.compat', true)) {
339401
return array_values($this->extraColumns);
340402
}
341-
$returnFields = Request::getInstance()->getQuery(Api::API_FIELDS_RESULT_FIELD);
342-
$fields = explode(',', $returnFields ?: '');
403+
return $this->normalizeRequestedExtraFields(
404+
(string)(Request::getInstance()->getQuery(Api::API_FIELDS_RESULT_FIELD) ?? '')
405+
);
406+
}
407+
408+
/**
409+
* @return array<int, string>
410+
*/
411+
private function normalizeRequestedExtraFields(string $returnFields): array
412+
{
413+
$fields = explode(',', $returnFields);
343414
$fields[] = self::API_MODEL_KEY_FIELD;
344-
return $fields;
415+
$fields = array_values(array_filter(array_map('trim', $fields), static fn(string $field): bool => $field !== ''));
416+
417+
return array_values(array_unique($fields));
345418
}
346419

347420
protected function sanitizeString(string $value): string

0 commit comments

Comments
 (0)