8000 [Bugfix] Reject resource objects sent in relationships · GIANTCRAB/laravel-json-api@f9e00f1 · GitHub
[go: up one dir, main page]
Skip to content

Commit f9e00f1

Browse files
lindyhopchrislindyhopchris
committed
[Bugfix] Reject resource objects sent in relationships
The spec defines that only resource identifiers are expected, so it is invalid to send a resource object. This is detected based on the presence of either an attributes or a relationships member in the object.
1 parent 41b12af commit f9e00f1

File tree

6 files changed

+110
-10
lines changed

6 files changed

+110
-10
lines changed

‎CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ All notable changes to this project will be documented in this file. This projec
55
## Unreleased
66

77
### Fixed
8+
- [#302](https://github.com/cloudcreativity/laravel-json-api/issues/302)
9+
Reject resource objects sent in relationships, as the spec defines that only resource identifiers
10+
are expected. This is based on the object having either an `attributes` or a `relationships` member.
811
- [#295](https://github.com/cloudcreativity/laravel-json-api/issues/295)
912
Use `null` for empty `from`/`to` fields in cursor page meta.
1013

‎resources/lang/en/errors.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@
4646
'code' => '',
4747
],
4848

49+
'member_identifier_expected' => [
50+
'title' => 'Non-Compliant JSON API Document',
51+
'detail' => 'The member :member must be a resource identifier.',
52+
'code' => '',
53+
],
54+
4955
'member_string_expected' => [
5056
'title' => 'Non-Compliant JSON API Document',
5157
'detail' => "The member :member must be a string.",

‎src/Validation/ErrorTranslator.php

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,26 @@ public function memberNotObject(string $path, string $member): ErrorInterface
100100
);
101101
}
102102

103+
/**
104+
* Create an error for a member that must be a resource identifier.
105+
*
106+
* @param string $path
107+
* @param string $member
108+
* @return ErrorInterface
109+
*/
110+
public function memberNotIdentifier(string $path, string $member): ErrorInterface
111+
{
112+
return new Error(
113+
null,
114+
null,
115+
Response::HTTP_BAD_REQUEST,
116+
$this->trans('member_identifier_expected', 'code'),
117+
$this->trans('member_identifier_expected', 'title'),
118+
$this->trans('member_identifier_expected', 'detail', compact('member')),
119+
$this->pointer($path, $member)
120+
);
121+
}
122+
103123
/**
104124
* Create an error for when a member has a field that is not allowed.
105125
*
@@ -475,11 +495,9 @@ protected function trans(string $key, string $member, array $replace = [], ?stri
475495
*/
476496
protected function pointer(string $path, ?string $member = null): array
477497
{
478-
if (!$member) {
479-
$pointer = $path;
480-
} else {
481-
$pointer = $member ? sprintf('%s/%s', rtrim($path, '/'), $member) : $path;
482-
}
498+
/** Member can be '0' which is an empty string. */
499+
$withoutMember = is_null($member) || '' === $member;
500+
$pointer = !$withoutMember ? sprintf('%s/%s', rtrim($path, '/'), $member) : $path;
483501

484502
return [Error::SOURCE_POINTER => $pointer];
485503
}

‎src/Validation/Spec/AbstractValidator.php

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -170,22 +170,22 @@ protected function validateIdMember($value, string $path): bool
170170
* Validate an identifier object.
171171
*
172172
* @param mixed $value
173-
* @param string $dataPath
173+
* @param string $path
174174
* the path to the data member in which the identifier is contained.
175175
* @param int|null $index
176176
* the index for the identifier, if in a collection.
177177
* @return bool
178178
*/
179-
protected function validateIdentifier($value, string $dataPath, ?int $index = null): bool
179+
protected function validateIdentifier($value, string $path, ?int $index = null): bool
180180
{
181181
$member = is_int($index) ? (string) $index : 'data';
182182

183183
if (!is_object($value)) {
184-
$this->memberNotObject($dataPath, $member);
184+
$this->memberNotObject($path, $member);
185185
return false;
186186
}
187187

188-
$dataPath = sprintf('%s/%s', rtrim($dataPath, '/'), $member);
188+
$dataPath = sprintf('%s/%s', rtrim($path, '/'), $member);
189189
$valid = true;
190190

191191
if (!property_exists($value, 'type')) {
@@ -202,6 +202,12 @@ protected function validateIdentifier($value, string $dataPath, ?int $index = nu
202202
$valid = false;
203203
}
204204

205+
/** If it has attributes or relationships, it is a resource object not a resource identifier */
206+
if (property_exists($value, 'attributes') || property_exists($value, 'relationships')) {
207+
$this->memberNotIdentifier($path, $member);
208+
$valid = false;
209+
}
210+
205211
return $valid;
206212
}
207213

@@ -363,6 +369,18 @@ protected function memberNotObject(string $path, string $member): void
363369
$this->errors->add($this->translator->memberNotObject($path, $member));
364370
}
365371

372+
/**
373+
* Add an error for a member that must be an object.
374+
*
375+
* @param string $path
376+
* @param string|null $member
377+
* @return void
378+
*/
379+
protected function memberNotIdentifier(string $path, string $member): void
380+
{
381+
$this->errors->add($this->translator->memberNotIdentifier($path, $member));
382+
}
383+
366384
/**
367385
* Add errors for when a member has a field that is not allowed.
368386
*

‎tests/lib/Integration/Validation/Spec/RelationshipValidationTest.php

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,42 @@ public function toOneProvider()
175175
'source' => ['pointer' => '/data'],
176176
],
177177
],
178+
'data:resource object with attributes' => [
179+
[
180+
'data' => [
181+
'type' => 'users',
182+
'id' => '1',
183+
'attributes' => [
184+
'name' => 'John Doe',
185+
],
186+
],
187+
],
188+
[
189+
'title' => 'Non-Compliant JSON API Document',
190+
'detail' => 'The member data must be a resource identifier.',
191+
'status' => '400',
192+
'source' => ['pointer' => '/data'],
193+
],
194+
],
195+
'data:resource object with relationships' => [
196+
[
197+
'data' => [
198+
'type' => ' 10000 users',
199+
'id' => '1',
200+
'relationships' => [
201+
'sites' => [
202+
'data' => [],
203+
],
204+
],
205+
],
206+
],
207+
[
208+
'title' => 'Non-Compliant JSON API Document',
209+
'detail' => 'The member data must be a resource identifier.',
210+
'status' => '400',
211+
'source' => ['pointer' => '/data'],
212+
],
213+
],
178214
];
179215
}
180216

@@ -310,6 +346,25 @@ public function toManyProvider()
310346
'source' => ['pointer' => '/data/0'],
311347
],
312348
],
349+
'data:resource object with attributes' => [
350+
[
351+
'data' => [
352+
[
353+
'type' => 'tags',
354+
'id' => '100',
355+
'attributes' => [
356+
'name' => 'News',
357+
],
358+
],
359+
],
360+
],
361+
[
362+
'title' => 'Non-Compliant JSON API Document',
363+
'detail' => 'The member 0 must be a resource identifier.',
364+
'status' => '400',
365+
'source' => ['pointer' => '/data/0'],
366+
],
367+
],
313368
];
314369
}
315370

‎tests/lib/Integration/Validation/Spec/ResourceValidationTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ public function postProvider()
338338
'title' => 'Non-Compliant JSON API Document',
339339
'detail' => "The member 0 must be an object.",
340340
'status' => '400',
341-
'source' => ['pointer' => '/data/relationships/tags/data'],
341+
'source' => ['pointer' => '/data/relationships/tags/data/0'],
342342
],
343343
],
344344
'data.relationships.*.data.*.type:required' => [

0 commit comments

Comments
 (0)
0