Skip to content

Commit e9746f4

Browse files
avsejavsej
committed
PCBC-751: verify subdoc Replace and Remove with empty path
Change-Id: I05533dc971b0bb3779bfe2bf8c184e67f1e9b341 Reviewed-on: http://review.couchbase.org/c/php-couchbase/+/163463 Tested-by: Build Bot <[email protected]> Reviewed-by: Sergey Avseyev <[email protected]>
1 parent 8f04469 commit e9746f4

File tree

2 files changed

+60
-25
lines changed

2 files changed

+60
-25
lines changed

‎src/couchbase/bucket/subdoc.c

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -265,29 +265,33 @@ PHP_METHOD(Collection, lookupIn)
265265
}
266266
ZEND_HASH_FOREACH_VAL(spec, val)
267267
{
268+
lcb_STATUS rc = LCB_SUCCESS;
268269
flags = 0;
269270
if (Z_OBJCE_P(val) == pcbc_lookup_get_spec_ce) {
270271
if (Z_TYPE_P(pcbc_read_property(pcbc_lookup_get_spec_ce, val, ("is_xattr"), 0, &tmp)) == IS_TRUE) {
271272
flags |= LCB_SUBDOCSPECS_F_XATTRPATH;
272273
}
273274
prop = pcbc_read_property(pcbc_lookup_get_spec_ce, val, ("path"), 0, &tmp);
274-
lcb_subdocspecs_get(operations, idx, flags, Z_STRVAL_P(prop), Z_STRLEN_P(prop));
275+
rc = lcb_subdocspecs_get(operations, idx, flags, Z_STRVAL_P(prop), Z_STRLEN_P(prop));
275276
} else if (Z_OBJCE_P(val) == pcbc_lookup_count_spec_ce) {
276277
if (Z_TYPE_P(pcbc_read_property(pcbc_lookup_count_spec_ce, val, ("is_xattr"), 0, &tmp)) == IS_TRUE) {
277278
flags |= LCB_SUBDOCSPECS_F_XATTRPATH;
278279
}
279280
prop = pcbc_read_property(pcbc_lookup_count_spec_ce, val, ("path"), 0, &tmp);
280-
lcb_subdocspecs_get_count(operations, idx, flags, Z_STRVAL_P(prop), Z_STRLEN_P(prop));
281+
rc = lcb_subdocspecs_get_count(operations, idx, flags, Z_STRVAL_P(prop), Z_STRLEN_P(prop));
281282
} else if (Z_OBJCE_P(val) == pcbc_lookup_exists_spec_ce) {
282283
if (Z_TYPE_P(pcbc_read_property(pcbc_lookup_exists_spec_ce, val, ("is_xattr"), 0, &tmp)) == IS_TRUE) {
283284
flags |= LCB_SUBDOCSPECS_F_XATTRPATH;
284285
}
285286
prop = pcbc_read_property(pcbc_lookup_exists_spec_ce, val, ("path"), 0, &tmp);
286-
lcb_subdocspecs_exists(operations, idx, flags, Z_STRVAL_P(prop), Z_STRLEN_P(prop));
287+
rc = lcb_subdocspecs_exists(operations, idx, flags, Z_STRVAL_P(prop), Z_STRLEN_P(prop));
287288
} else {
288-
/* TODO: raise argument exception */
289+
rc = LCB_ERR_INVALID_ARGUMENT;
290+
}
291+
if (rc != LCB_SUCCESS) {
289292
lcb_subdocspecs_destroy(operations);
290-
return;
293+
throw_lcb_exception_ex(rc, PCBC_OPCODE_UNSPEC, NULL);
294+
RETURN_NULL();
291295
}
292296
idx++;
293297
}
@@ -461,6 +465,7 @@ PHP_METHOD(Collection, mutateIn)
461465
uint32_t flags;
462466
ZEND_HASH_FOREACH_VAL(spec, entry)
463467
{
468+
lcb_STATUS rc = LCB_SUCCESS;
464469
flags = 0;
465470
if (Z_OBJCE_P(entry) == pcbc_mutate_insert_spec_ce) {
466471
if (Z_TYPE_P(pcbc_read_property(Z_OBJCE_P(entry), entry, ("is_xattr"), 0, &rv1)) == IS_TRUE) {
@@ -474,8 +479,8 @@ PHP_METHOD(Collection, mutateIn)
474479
}
475480
path = pcbc_read_property(Z_OBJCE_P(entry), entry, ("path"), 0, &rv1);
476481
value = pcbc_read_property(Z_OBJCE_P(entry), entry, ("value"), 0, &rv2);
477-
lcb_subdocspecs_dict_add(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path), Z_STRVAL_P(value),
478-
Z_STRLEN_P(value));
482+
rc = lcb_subdocspecs_dict_add(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path), Z_STRVAL_P(value),
483+
Z_STRLEN_P(value));
479484
} else if (Z_OBJCE_P(entry) == pcbc_mutate_upsert_spec_ce) {
480485
if (Z_TYPE_P(pcbc_read_property(Z_OBJCE_P(entry), entry, ("is_xattr"), 0, &rv1)) == IS_TRUE) {
481486
flags |= LCB_SUBDOCSPECS_F_XATTRPATH;
@@ -488,8 +493,8 @@ PHP_METHOD(Collection, mutateIn)
488493
}
489494
path = pcbc_read_property(Z_OBJCE_P(entry), entry, ("path"), 0, &rv1);
490495
value = pcbc_read_property(Z_OBJCE_P(entry), entry, ("value"), 0, &rv2);
491-
lcb_subdocspecs_dict_upsert(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path), Z_STRVAL_P(value),
492-
Z_STRLEN_P(value));
496+
rc = lcb_subdocspecs_dict_upsert(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path),
497+
Z_STRVAL_P(value), Z_STRLEN_P(value));
493498
} else if (Z_OBJCE_P(entry) == pcbc_mutate_replace_spec_ce) {
494499
if (Z_TYPE_P(pcbc_read_property(Z_OBJCE_P(entry), entry, ("is_xattr"), 0, &rv1)) == IS_TRUE) {
495500
flags |= LCB_SUBDOCSPECS_F_XATTRPATH;
@@ -499,14 +504,14 @@ PHP_METHOD(Collection, mutateIn)
499504
}
500505
path = pcbc_read_property(Z_OBJCE_P(entry), entry, ("path"), 0, &rv1);
501506
value = pcbc_read_property(Z_OBJCE_P(entry), entry, ("value"), 0, &rv2);
502-
lcb_subdocspecs_replace(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path), Z_STRVAL_P(value),
503-
Z_STRLEN_P(value));
507+
rc = lcb_subdocspecs_replace(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path), Z_STRVAL_P(value),
508+
Z_STRLEN_P(value));
504509
} else if (Z_OBJCE_P(entry) == pcbc_mutate_remove_spec_ce) {
505510
if (Z_TYPE_P(pcbc_read_property(Z_OBJCE_P(entry), entry, ("is_xattr"), 0, &rv1)) == IS_TRUE) {
506511
flags |= LCB_SUBDOCSPECS_F_XATTRPATH;
507512
}
508513
path = pcbc_read_property(Z_OBJCE_P(entry), entry, ("path"), 0, &rv1);
509-
lcb_subdocspecs_remove(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path));
514+
rc = lcb_subdocspecs_remove(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path));
510515
} else if (Z_OBJCE_P(entry) == pcbc_mutate_array_append_spec_ce) {
511516
if (Z_TYPE_P(pcbc_read_property(Z_OBJCE_P(entry), entry, ("is_xattr"), 0, &rv1)) == IS_TRUE) {
512517
flags |= LCB_SUBDOCSPECS_F_XATTRPATH;
@@ -519,8 +524,8 @@ PHP_METHOD(Collection, mutateIn)
519524
}
520525
path = pcbc_read_property(Z_OBJCE_P(entry), entry, ("path"), 0, &rv1);
521526
value = pcbc_read_property(Z_OBJCE_P(entry), entry, ("value"), 0, &rv2);
522-
lcb_subdocspecs_array_add_last(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path),
523-
Z_STRVAL_P(value), Z_STRLEN_P(value));
527+
rc = lcb_subdocspecs_array_add_last(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path),
528+
Z_STRVAL_P(value), Z_STRLEN_P(value));
524529
} else if (Z_OBJCE_P(entry) == pcbc_mutate_array_prepend_spec_ce) {
525530
if (Z_TYPE_P(pcbc_read_property(Z_OBJCE_P(entry), entry, ("is_xattr"), 0, &rv1)) == IS_TRUE) {
526531
flags |= LCB_SUBDOCSPECS_F_XATTRPATH;
@@ -533,8 +538,8 @@ PHP_METHOD(Collection, mutateIn)
533538
}
534539
path = pcbc_read_property(Z_OBJCE_P(entry), entry, ("path"), 0, &rv1);
535540
value = pcbc_read_property(Z_OBJCE_P(entry), entry, ("value"), 0, &rv2);
536-
lcb_subdocspecs_array_add_first(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path),
537-
Z_STRVAL_P(value), Z_STRLEN_P(value));
541+
rc = lcb_subdocspecs_array_add_first(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path),
542+
Z_STRVAL_P(value), Z_STRLEN_P(value));
538543
} else if (Z_OBJCE_P(entry) == pcbc_mutate_array_insert_spec_ce) {
539544
if (Z_TYPE_P(pcbc_read_property(Z_OBJCE_P(entry), entry, ("is_xattr"), 0, &rv1)) == IS_TRUE) {
540545
flags |= LCB_SUBDOCSPECS_F_XATTRPATH;
@@ -547,8 +552,8 @@ PHP_METHOD(Collection, mutateIn)
547552
}
548553
path = pcbc_read_property(Z_OBJCE_P(entry), entry, ("path"), 0, &rv1);
549554
value = pcbc_read_property(Z_OBJCE_P(entry), entry, ("value"), 0, &rv2);
550-
lcb_subdocspecs_array_insert(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path), Z_STRVAL_P(value),
551-
Z_STRLEN_P(value));
555+
rc = lcb_subdocspecs_array_insert(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path),
556+
Z_STRVAL_P(value), Z_STRLEN_P(value));
552557
} else if (Z_OBJCE_P(entry) == pcbc_mutate_array_add_unique_spec_ce) {
553558
if (Z_TYPE_P(pcbc_read_property(Z_OBJCE_P(entry), entry, ("is_xattr"), 0, &rv1)) == IS_TRUE) {
554559
flags |= LCB_SUBDOCSPECS_F_XATTRPATH;
@@ -561,8 +566,8 @@ PHP_METHOD(Collection, mutateIn)
561566
}
562567
path = pcbc_read_property(Z_OBJCE_P(entry), entry, ("path"), 0, &rv1);
563568
value = pcbc_read_property(Z_OBJCE_P(entry), entry, ("value"), 0, &rv2);
564-
lcb_subdocspecs_array_add_unique(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path),
565-
Z_STRVAL_P(value), Z_STRLEN_P(value));
569+
rc = lcb_subdocspecs_array_add_unique(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path),
570+
Z_STRVAL_P(value), Z_STRLEN_P(value));
566571
} else if (Z_OBJCE_P(entry) == pcbc_mutate_counter_spec_ce) {
567572
if (Z_TYPE_P(pcbc_read_property(Z_OBJCE_P(entry), entry, ("is_xattr"), 0, &rv1)) == IS_TRUE) {
568573
flags |= LCB_SUBDOCSPECS_F_XATTRPATH;
@@ -572,11 +577,14 @@ PHP_METHOD(Collection, mutateIn)
572577
}
573578
path = pcbc_read_property(Z_OBJCE_P(entry), entry, ("path"), 0, &rv1);
574579
value = pcbc_read_property(Z_OBJCE_P(entry), entry, ("delta"), 0, &rv2);
575-
lcb_subdocspecs_counter(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path), Z_LVAL_P(value));
580+
rc = lcb_subdocspecs_counter(operations, idx, flags, Z_STRVAL_P(path), Z_STRLEN_P(path), Z_LVAL_P(value));
576581
} else {
577-
/* TODO: raise argument exception */
582+
rc = LCB_ERR_INVALID_ARGUMENT;
583+
}
584+
if (rc != LCB_SUCCESS) {
578585
lcb_subdocspecs_destroy(operations);
579-
return;
586+
throw_lcb_exception_ex(rc, PCBC_OPCODE_UNSPEC, NULL);
587+
RETURN_NULL();
580588
}
581589
idx++;
582590
}

‎tests/BucketTest.php

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -667,14 +667,41 @@ function testMutateIn($c) {
667667
/**
668668
* @depends testConnect
669669
*/
670-
function testMutationInWithEmptyPath($c) {
671-
$key = $this->makeKey('lookup_in_with_empty_path');
670+
function testSubdocUpsertWithEmptyPath($c) {
671+
$key = $this->makeKey('upsert_in_with_empty_path');
672672
$c->upsert($key, ['path1' => 'value1']);
673673

674674
$this->expectException(\Couchbase\BadInputException::class);
675675
$result = $c->mutateIn($key, [new \Couchbase\MutateUpsertSpec('', 'value')]);
676676
}
677677

678+
/**
679+
* @depends testConnect
680+
*/
681+
function testSubdocReplaceWithEmptyPath($c) {
682+
$key = $this->makeKey('replace_in_with_empty_path');
683+
$c->upsert($key, ['path1' => 'value1']);
684+
685+
$c->mutateIn($key, [new \Couchbase\MutateReplaceSpec('', ['path2' => 'value2'])]);
686+
$result = $c->get($key);
687+
$this->assertEquals($result->content(), ['path2' => 'value2']);
688+
}
689+
690+
/**
691+
* @depends testConnect
692+
*/
693+
function testSubdocRemoveWithEmptyPath($c) {
694+
if ($this->usingMock()) {
695+
$this->markTestSkipped('CouchbaseMock does not support remove with empty path');
696+
}
697+
$key = $this->makeKey('remove_in_with_empty_path');
698+
$c->upsert($key, ['path1' => 'value1']);
699+
700+
$c->mutateIn($key, [new \Couchbase\MutateRemoveSpec('')]);
701+
$this->expectException(\Couchbase\DocumentNotFoundException::class);
702+
$c->get($key);
703+
}
704+
678705
/**
679706
* @depends testConnect
680707
*/

0 commit comments

Comments
 (0)