Skip to content

Commit 0183a9d

Browse files
blizzzblizzz
authored
Merge pull request #2505 from nextcloud/fix/noid/reorderingValidation
Reordering validation
2 parents bc8d415 + f036c0d commit 0183a9d

16 files changed

Lines changed: 515 additions & 102 deletions

‎lib/Controller/ApiTablesController.php‎

Lines changed: 54 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
use OCA\Tables\Errors\NotFoundError;
1515
use OCA\Tables\Errors\PermissionError;
1616
use OCA\Tables\Middleware\Attribute\RequirePermission;
17+
use OCA\Tables\Model\ColumnSettings;
18+
use OCA\Tables\Model\SortRuleSet;
1719
use OCA\Tables\Model\ViewUpdateInput;
1820
use OCA\Tables\ResponseDefinitions;
1921
use OCA\Tables\Service\ColumnService;
@@ -132,12 +134,19 @@ public function showScheme(int $id): DataResponse {
132134
* @param list<TablesView> $views views
133135
* @param list<array{columnId: int, order: int, readonly: bool}> $columnOrder Default column order settings
134136
* @param list<array{columnId: int, mode: 'ASC'|'DESC'}> $sort Default sort rules
135-
* @return DataResponse<Http::STATUS_OK, TablesTable, array{}>|DataResponse<Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
137+
* @return DataResponse<Http::STATUS_OK, TablesTable, array{}>|DataResponse<Http::STATUS_BAD_REQUEST|Http::STATUS_INTERNAL_SERVER_ERROR, array{message: string}, array{}>
136138
*
137139
* 200: Tables returned
140+
* 400: Invalid request data
138141
*/
139142
#[NoAdminRequired]
140143
public function createFromScheme(string $title, string $emoji, string $description, array $columns, array $views, array $columnOrder = [], array $sort = []): DataResponse {
144+
try {
145+
ColumnSettings::createFromInputArray($columnOrder);
146+
SortRuleSet::createFromInputArray($sort);
147+
} catch (\InvalidArgumentException $e) {
148+
return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
149+
}
141150
try {
142151
$this->db->beginTransaction();
143152
$table = $this->service->create($title, 'custom', $emoji, $description);
@@ -178,18 +187,24 @@ public function createFromScheme(string $title, string $emoji, string $descripti
178187
$colMap[$column['id']] = $col->getId();
179188
}
180189
if (!empty($columnOrder) || !empty($sort)) {
181-
$remappedColumnOrder = !empty($columnOrder) ? array_map(static function (array $entry) use ($colMap): array {
182-
if (isset($entry['columnId']) && $entry['columnId'] > 0) {
183-
$entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId'];
184-
}
185-
return $entry;
186-
}, $columnOrder) : null;
187-
$remappedSort = !empty($sort) ? array_map(static function (array $entry) use ($colMap): array {
188-
if (isset($entry['columnId']) && $entry['columnId'] > 0) {
189-
$entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId'];
190-
}
191-
return $entry;
192-
}, $sort) : null;
190+
$remappedColumnOrder = null;
191+
if (!empty($columnOrder)) {
192+
$remappedColumnOrder = ColumnSettings::createFromInputArray(array_map(static function (array $entry) use ($colMap): array {
193+
if ($entry['columnId'] > 0) {
194+
$entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId'];
195+
}
196+
return $entry;
197+
}, $columnOrder));
198+
}
199+
$remappedSort = null;
200+
if (!empty($sort)) {
201+
$remappedSort = SortRuleSet::createFromInputArray(array_map(static function (array $entry) use ($colMap): array {
202+
if ($entry['columnId'] > 0) {
203+
$entry['columnId'] = $colMap[$entry['columnId']] ?? $entry['columnId'];
204+
}
205+
return $entry;
206+
}, $sort));
207+
}
193208
$table = $this->service->update($table->getId(), null, null, null, null, $this->userId, $remappedColumnOrder, $remappedSort);
194209
}
195210
foreach ($views as $view) {
@@ -240,6 +255,14 @@ public function createFromScheme(string $title, string $emoji, string $descripti
240255
}
241256
$this->db->commit();
242257
return new DataResponse($table->jsonSerialize());
258+
} catch (\InvalidArgumentException $e) {
259+
try {
260+
$this->db->rollBack();
261+
} catch (\OCP\DB\Exception $re) {
262+
return $this->handleError($re);
263+
}
264+
$this->logger->warning('An invalid request occurred: ' . $e->getMessage(), ['exception' => $e]);
265+
return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
243266
} catch (InternalError|Exception $e) {
244267
try {
245268
$this->db->rollBack();
@@ -281,9 +304,10 @@ public function create(string $title, ?string $emoji, ?string $description, stri
281304
* @param string $description the tables description
282305
* @param list<array{columnId: int, order: int, readonly: bool}>|string|null $columnSettings Default column order settings (array or JSON string)
283306
* @param list<array{columnId: int, mode: 'ASC'|'DESC'}>|string|null $sort Default sort rules (array or JSON string)
284-
* @return DataResponse<Http::STATUS_OK, TablesTable, array{}>|DataResponse<Http::STATUS_FORBIDDEN|Http::STATUS_INTERNAL_SERVER_ERROR|Http::STATUS_NOT_FOUND, array{message: string}, array{}>
307+
* @return DataResponse<Http::STATUS_OK, TablesTable, array{}>|DataResponse<Http::STATUS_BAD_REQUEST|Http::STATUS_FORBIDDEN|Http::STATUS_INTERNAL_SERVER_ERROR|Http::STATUS_NOT_FOUND, array{message: string}, array{}>
285308
*
286309
* 200: Tables returned
310+
* 400: Invalid request data
287311
* 403: No permissions
288312
* 404: Not found
289313
*/
@@ -297,9 +321,24 @@ public function update(int $id, ?string $title = null, ?string $emoji = null, ?s
297321
$sort = json_decode($sort, true) ?? null;
298322
}
299323
try {
300-
return new DataResponse($this->service->update($id, $title, $emoji, $description, $archived, $this->userId, $columnSettings, $sort)->jsonSerialize());
324+
if ($columnSettings !== null && !is_array($columnSettings)) {
325+
throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array');
326+
}
327+
if ($sort !== null && !is_array($sort)) {
328+
throw new \InvalidArgumentException('Invalid sort: must be a JSON array');
329+
}
330+
$columnSettingsObj = $columnSettings !== null ? ColumnSettings::createFromInputArray($columnSettings) : null;
331+
$sortObj = $sort !== null ? SortRuleSet::createFromInputArray($sort) : null;
332+
} catch (\InvalidArgumentException $e) {
333+
return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
334+
}
335+
try {
336+
return new DataResponse($this->service->update($id, $title, $emoji, $description, $archived, $this->userId, $columnSettingsObj, $sortObj)->jsonSerialize());
301337
} catch (PermissionError $e) {
302338
return $this->handlePermissionError($e);
339+
} catch (\InvalidArgumentException $e) {
340+
$this->logger->warning('An invalid request occurred: ' . $e->getMessage(), ['exception' => $e]);
341+
return new DataResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
303342
} catch (InternalError $e) {
304343
return $this->handleError($e);
305344
} catch (NotFoundError $e) {

‎lib/Controller/TableController.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
use OCA\Tables\AppInfo\Application;
1111
use OCA\Tables\Middleware\Attribute\RequirePermission;
12+
use OCA\Tables\Model\ColumnSettings;
13+
use OCA\Tables\Model\SortRuleSet;
1214
use OCA\Tables\Service\TableService;
1315
use OCP\AppFramework\Controller;
1416
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
@@ -78,7 +80,17 @@ public function update(int $id, ?string $title = null, ?string $emoji = null, ?b
7880
$sort = json_decode($sort, true) ?? null;
7981
}
8082
return $this->handleError(function () use ($id, $title, $emoji, $archived, $columnSettings, $sort) {
81-
return $this->service->update($id, $title, $emoji, null, $archived, $this->userId, $columnSettings, $sort);
83+
if ($columnSettings !== null && !is_array($columnSettings)) {
84+
throw new \InvalidArgumentException('Invalid columnSettings: must be a JSON array');
85+
}
86+
if ($sort !== null && !is_array($sort)) {
87+
throw new \InvalidArgumentException('Invalid sort: must be a JSON array');
88+
}
89+
return $this->service->update(
90+
$id, $title, $emoji, null, $archived, $this->userId,
91+
$columnSettings !== null ? ColumnSettings::createFromInputArray($columnSettings) : null,
92+
$sort !== null ? SortRuleSet::createFromInputArray($sort) : null,
93+
);
8294
});
8395
}
8496
}

‎lib/Db/Table.php‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
use OCA\Tables\Model\Permissions;
1212
use OCA\Tables\Model\SortRuleSet;
1313
use OCA\Tables\ResponseDefinitions;
14-
use OCA\Tables\Service\ValueObject\ViewColumnInformation;
14+
use OCA\Tables\Service\ValueObject\ColumnOrderInformation;
1515

1616
/**
1717
* @psalm-suppress PropertyNotSetInConstructor
@@ -44,7 +44,7 @@
4444
* @method setRowsCount(int $rowsCount)
4545
* @method getColumnsCount(): int
4646
* @method setColumnsCount(int $columnsCount)
47-
* @method getViews(): ?array
47+
* @method getViews(): ?array<TablesView>
4848
* @method setViews(array $views)
4949
* @method getColumns(): array
5050
* @method setColumns(array $columns)
@@ -109,14 +109,17 @@ public function jsonSerialize(): array {
109109
'lastEditAt' => $this->lastEditAt ?: '',
110110
'archived' => $this->archived,
111111
'isShared' => (bool)$this->isShared,
112-
'favorite' => $this->favorite,
112+
'favorite' => (bool)$this->favorite,
113113
'onSharePermissions' => $this->getSharePermissions()?->jsonSerialize(),
114114
'hasShares' => (bool)$this->hasShares,
115115
'rowsCount' => $this->rowsCount ?: 0,
116116
'columnsCount' => $this->columnsCount ?: 0,
117-
'views' => $this->getViewsArray(),
118-
'description' => $this->description ?:'',
119-
'columnOrder' => $this->getColumnOrderSettingsArray(),
117+
'description' => $this->description ?: '',
118+
'views' => array_values($this->getViewsArray()),
119+
'columnOrder' => array_map(
120+
static fn (ColumnOrderInformation $c) => ['columnId' => $c->getId(), 'order' => $c->getOrder()],
121+
$this->getColumnOrderSettingsArray()
122+
),
120123
'sort' => $this->getSortArray(),
121124
];
122125
}
@@ -126,47 +129,45 @@ private function getSharePermissions(): ?Permissions {
126129
}
127130

128131
/**
129-
* @psalm-suppress MismatchingDocblockReturnType
130132
* @return TablesView[]
131133
*/
132134
private function getViewsArray(): array {
133135
return $this->getViews() ?: [];
134136
}
135137

136138
/**
137-
* @psalm-suppress MismatchingDocblockReturnType
138139
* @return int[]
139140
*/
140141
public function getColumnOrderArray(): array {
141142
$columnSettings = $this->getColumnOrderSettingsArray();
142-
usort($columnSettings, static function (ViewColumnInformation $a, ViewColumnInformation $b) {
143+
usort($columnSettings, static function (ColumnOrderInformation $a, ColumnOrderInformation $b) {
143144
return $a->getOrder() - $b->getOrder();
144145
});
145-
return array_map(static fn (ViewColumnInformation $vci): int => $vci->getId(), $columnSettings);
146+
/** @var list<ColumnOrderInformation> $columnSettings */
147+
return array_map(static fn (ColumnOrderInformation $vci): int => $vci->getId(), $columnSettings);
146148
}
147149

148150
/**
149-
* @return array<ViewColumnInformation>
151+
* @return list<ColumnOrderInformation>
150152
*/
151153
public function getColumnOrderSettingsArray(): array {
152154
$columns = $this->getArray($this->getColumnOrder());
153155
if (empty($columns)) {
154156
return [];
155157
}
156158

157-
if (is_array(reset($columns))) {
158-
return array_values(array_map(static fn (array $a): ViewColumnInformation => ViewColumnInformation::fromArray($a), $columns));
159+
if (is_array($columns[array_key_first($columns)] ?? null)) {
160+
return array_values(array_map(static fn (array $a): ColumnOrderInformation => ColumnOrderInformation::fromArray($a), $columns));
159161
}
160162

161163
$result = [];
162164
foreach ($columns as $index => $columnId) {
163-
$result[] = new ViewColumnInformation($columnId, order: (int)$index + 1);
165+
$result[] = new ColumnOrderInformation((int)$columnId, order: (int)$index + 1);
164166
}
165167
return $result;
166168
}
167169

168170
/**
169-
* @psalm-suppress MismatchingDocblockReturnType
170171
* @return list<array{columnId: int, mode: 'ASC'|'DESC'}>
171172
*/
172173
public function getSortArray(): array {
@@ -176,9 +177,8 @@ public function getSortArray(): array {
176177

177178
private function getArray(?string $json): array {
178179
if ($json !== '' && $json !== null && $json !== 'null') {
179-
return \json_decode($json, true);
180-
} else {
181-
return [];
180+
return \json_decode($json, true) ?? [];
182181
}
182+
return [];
183183
}
184184
}

‎lib/Model/ColumnSettings.php‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,19 @@
1111

1212
use Generator;
1313
use JsonSerializable;
14+
use OCA\Tables\Service\ValueObject\ColumnOrderInformation;
1415
use OCA\Tables\Service\ValueObject\ViewColumnInformation;
1516

16-
class ColumnSettings implements JSONSerializable {
17+
class ColumnSettings implements JsonSerializable {
1718
/**
18-
* @param ViewColumnInformation[] $columnSettings
19+
* @param ColumnOrderInformation[] $columnSettings
1920
*/
2021
public function __construct(
2122
protected array $columnSettings,
2223
) {
2324
foreach ($this->columnSettings as $columnSetting) {
24-
if (!$columnSetting instanceof ViewColumnInformation) {
25-
throw new \InvalidArgumentException('Provided column settings must be of type ViewColumnInformation');
25+
if (!$columnSetting instanceof ColumnOrderInformation) {
26+
throw new \InvalidArgumentException('Provided column settings must be of type ColumnOrderInformation');
2627
}
2728
}
2829
}
@@ -33,15 +34,35 @@ public function columnInformation(): Generator {
3334
}
3435
}
3536

37+
/**
38+
* Creates column settings with only columnId and order (for table use).
39+
*/
3640
public static function createFromInputArray(array $inputColumnSettings): self {
3741
$columnSettings = [];
3842
foreach ($inputColumnSettings as $inputColumnSetting) {
43+
if (!is_array($inputColumnSetting)) {
44+
throw new \InvalidArgumentException('Each column settings entry must be an array');
45+
}
46+
$columnSettings[] = ColumnOrderInformation::fromArray($inputColumnSetting);
47+
}
48+
return new self($columnSettings);
49+
}
50+
51+
/**
52+
* Creates column settings with view-specific fields (columnId, order, readonly, mandatory).
53+
*/
54+
public static function createViewSettingsFromInputArray(array $inputColumnSettings): self {
55+
$columnSettings = [];
56+
foreach ($inputColumnSettings as $inputColumnSetting) {
57+
if (!is_array($inputColumnSetting)) {
58+
throw new \InvalidArgumentException('Each column settings entry must be an array');
59+
}
3960
$columnSettings[] = ViewColumnInformation::fromArray($inputColumnSetting);
4061
}
4162
return new self($columnSettings);
4263
}
4364

44-
public function jsonSerialize(): mixed {
45-
return $this->columnSettings;
65+
public function jsonSerialize(): array {
66+
return array_map(static fn (ColumnOrderInformation $vci) => $vci->jsonSerialize(), $this->columnSettings);
4667
}
4768
}

‎lib/Model/SortRuleSet.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ public function __construct(
3535
public static function createFromInputArray(array $data): self {
3636
$sortRules = [];
3737
foreach ($data as $inputSortRule) {
38+
if (!is_array($inputSortRule)) {
39+
throw new InvalidArgumentException('Each sort rule entry must be an array');
40+
}
3841
if (!isset($inputSortRule['columnId'], $inputSortRule['mode'])) {
3942
throw new InvalidArgumentException('Required sort parameters are missing');
4043
}
@@ -47,6 +50,9 @@ public static function createFromInputArray(array $data): self {
4750
return new self($sortRules);
4851
}
4952

53+
/**
54+
* @return list<array{columnId: int, mode: 'ASC'|'DESC'}>
55+
*/
5056
public function jsonSerialize(): array {
5157
return array_map(static fn (SortRule $s) => $s->jsonSerialize(), $this->sortRules);
5258
}

‎lib/Model/ViewUpdateInput.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public static function fromInputArray(array $data): self {
8484
title: ($data['title'] ?? null) ? new Title($data['title']) : null,
8585
description: $data['description'] ?? null,
8686
emoji: ($data['emoji'] ?? null) ? new Emoji($data['emoji']) : null,
87-
columnSettings: ($data['columnSettings'] ?? null) ? ColumnSettings::createFromInputArray($data['columnSettings']) : null,
87+
columnSettings: ($data['columnSettings'] ?? null) ? ColumnSettings::createViewSettingsFromInputArray($data['columnSettings']) : null,
8888
filterSet: ($data['filter'] ?? null) ? FilterSet::createFromInputArray($data['filter']) : null,
8989
sortRuleSet: ($data['sort'] ?? null) ? SortRuleSet::createFromInputArray($data['sort']) : null,
9090
);

‎lib/ResponseDefinitions.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,10 @@
6464
* },
6565
* hasShares: bool,
6666
* rowsCount: int,
67+
* description: string,
6768
* views: list<TablesView>,
6869
* columnsCount: int,
69-
* columnOrder: list<array{columnId: int, order: int, readonly: bool}>,
70+
* columnOrder: list<array{columnId: int, order: int}>,
7071
* sort: list<array{columnId: int, mode: 'ASC'|'DESC'}>,
7172
* }
7273
*

‎lib/Service/TableService.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ public function delete(int $id, ?string $userId = null): Table {
476476
* @throws NotFoundError
477477
* @throws PermissionError
478478
*/
479-
public function update(int $id, ?string $title, ?string $emoji, ?string $description, ?bool $archived = null, ?string $userId = null, ?array $columnSettings = null, ?array $sort = null): Table {
479+
public function update(int $id, ?string $title, ?string $emoji, ?string $description, ?bool $archived = null, ?string $userId = null, ?ColumnSettings $columnSettings = null, ?SortRuleSet $sort = null): Table {
480480
$userId = $this->permissionsService->preCheckUserId($userId);
481481

482482
try {
@@ -509,10 +509,10 @@ public function update(int $id, ?string $title, ?string $emoji, ?string $descrip
509509
$table->setDescription($description);
510510
}
511511
if ($columnSettings !== null) {
512-
$table->setColumnOrder(\json_encode(ColumnSettings::createFromInputArray($columnSettings)->jsonSerialize()));
512+
$table->setColumnOrder(\json_encode($columnSettings->jsonSerialize()));
513513
}
514514
if ($sort !== null) {
515-
$table->setSort(\json_encode(SortRuleSet::createFromInputArray($sort)->jsonSerialize()));
515+
$table->setSort(\json_encode($sort->jsonSerialize()));
516516
}
517517
$table->setLastEditBy($userId);
518518
$table->setLastEditAt($time->format('Y-m-d H:i:s'));

0 commit comments

Comments
 (0)