Skip to content

Commit 69f267e

Browse files
dktappsdktapps
committed
Throw consistent exceptions in PalettedBlockArray::fromData()
closes #27
1 parent 5614724 commit 69f267e

File tree

8 files changed

+55
-18
lines changed

8 files changed

+55
-18
lines changed

‎src/PhpPalettedBlockArray.cpp‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#include "PhpPalettedBlockArrayObj.h"
88
#include "stubs/pocketmine/world/format/PalettedBlockArray_arginfo.h"
9+
#include "stubs/pocketmine/world/format/PalettedBlockArrayLoadException_arginfo.h"
910

1011
extern "C" {
1112
#include "php.h"
@@ -16,21 +17,23 @@ extern "C" {
1617
}
1718

1819
zend_class_entry *paletted_block_array_entry;
20+
zend_class_entry* paletted_block_array_load_exception_entry;
21+
1922
static zend_object_handlers paletted_block_array_handlers;
2023

2124
/* internal object methods */
2225

23-
static inline bool checkPaletteEntrySize(zend_long v) {
26+
static inline bool checkPaletteEntrySize(zend_long v, zend_class_entry* exception_ce) {
2427
Block casted = (Block)v;
2528
zend_long castedBack = (zend_long)casted;
2629
if (castedBack != v) {
27-
zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0, "value %zd is too large to be used as a palette entry", v);
30+
zend_throw_exception_ex(exception_ce, 0, "value %zd is too large to be used as a palette entry", v);
2831
return false;
2932
}
3033
return true;
3134
}
3235

33-
static bool palette_data_from_array(HashTable* paletteHt, std::vector<Block>& palette) {
36+
static bool palette_data_from_array(HashTable* paletteHt, std::vector<Block>& palette, zend_class_entry* exception_ce) {
3437
HashPosition pos;
3538
zval *current;
3639
Block b;
@@ -42,7 +45,7 @@ static bool palette_data_from_array(HashTable* paletteHt, std::vector<Block>& pa
4245
zend_hash_move_forward_ex(paletteHt, &pos)
4346
) {
4447

45-
if (!checkPaletteEntrySize(Z_LVAL_P(current))) {
48+
if (!checkPaletteEntrySize(Z_LVAL_P(current), exception_ce)) {
4649
return false;
4750
}
4851
b = (Block)Z_LVAL_P(current);
@@ -52,9 +55,9 @@ static bool palette_data_from_array(HashTable* paletteHt, std::vector<Block>& pa
5255
return true;
5356
}
5457

55-
static bool palette_data_from_string(zend_string* paletteZstr, std::vector<Block>& palette) {
58+
static bool palette_data_from_string(zend_string* paletteZstr, std::vector<Block>& palette, zend_class_entry* exception_ce) {
5659
if ((ZSTR_LEN(paletteZstr) % sizeof(Block)) != 0) {
57-
zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0, "palette length in bytes must be a multiple of %zu, but have %zu bytes", sizeof(Block), ZSTR_LEN(paletteZstr));
60+
zend_throw_exception_ex(exception_ce, 0, "palette length in bytes must be a multiple of %zu, but have %zu bytes", sizeof(Block), ZSTR_LEN(paletteZstr));
5861
return false;
5962
}
6063

@@ -74,9 +77,9 @@ static bool paletted_block_array_from_data(zval *return_value, zend_long bitsPer
7477

7578
assert(paletteHt != nullptr || paletteZstr != nullptr);
7679

77-
if (paletteHt != nullptr && !palette_data_from_array(paletteHt, palette)) {
80+
if (paletteHt != nullptr && !palette_data_from_array(paletteHt, palette, paletted_block_array_load_exception_entry)) {
7881
return false;
79-
} else if (paletteZstr != nullptr && !palette_data_from_string(paletteZstr, palette)) {
82+
} else if (paletteZstr != nullptr && !palette_data_from_string(paletteZstr, palette, paletted_block_array_load_exception_entry)) {
8083
return false;
8184
}
8285

@@ -85,7 +88,7 @@ static bool paletted_block_array_from_data(zval *return_value, zend_long bitsPer
8588
return true;
8689
}
8790
catch (std::exception& e) {
88-
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "%s", e.what());
91+
zend_throw_exception_ex(paletted_block_array_load_exception_entry, 0, "%s", e.what());
8992
return false;
9093
}
9194
}
@@ -123,11 +126,12 @@ static void paletted_block_array_get_palette_bytes(zval* object, zval* return_va
123126
ZVAL_STRINGL(return_value, reinterpret_cast<const char*>(paletteValues), palette.size_bytes());
124127
}
125128

129+
126130
static bool paletted_block_array_set_palette(zval* object, HashTable* paletteHt) {
127131
paletted_block_array_obj* intern = fetch_from_zend_object<paletted_block_array_obj>(Z_OBJ_P(object));
128132

129133
std::vector<Block> palette;
130-
if (!palette_data_from_array(paletteHt, palette)) {
134+
if (!palette_data_from_array(paletteHt, palette, spl_ce_InvalidArgumentException)) {
131135
return false;
132136
}
133137

@@ -262,7 +266,7 @@ PALETTED_BLOCK_ARRAY_METHOD(__construct) {
262266
Z_PARAM_LONG(fillEntry)
263267
ZEND_PARSE_PARAMETERS_END();
264268

265-
if (!checkPaletteEntrySize(fillEntry)) {
269+
if (!checkPaletteEntrySize(fillEntry, spl_ce_InvalidArgumentException)) {
266270
return;
267271
}
268272

@@ -271,7 +275,7 @@ PALETTED_BLOCK_ARRAY_METHOD(__construct) {
271275
new(&intern->container) NormalBlockArrayContainer((Block)fillEntry, 0);
272276
}
273277
catch (std::exception& e) {
274-
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "%s", e.what());
278+
zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0, "%s", e.what());
275279
}
276280
}
277281

@@ -389,6 +393,11 @@ PALETTED_BLOCK_ARRAY_METHOD(getExpectedWordArraySize) {
389393
Z_PARAM_LONG(bitsPerBlock)
390394
ZEND_PARSE_PARAMETERS_END();
391395

396+
//TODO: this probably shouldn't be throwing InvalidArgumentException
397+
//the parameters given to this function will typically be coming from serialized
398+
//data, so it doesn't really make sense to declare them logically invalid
399+
//however using a different exception type will break BC
400+
392401
uint8_t casted = (uint8_t)bitsPerBlock;
393402
zend_long castedBack = (zend_long)casted;
394403
if (bitsPerBlock != castedBack) {
@@ -405,6 +414,8 @@ PALETTED_BLOCK_ARRAY_METHOD(getExpectedWordArraySize) {
405414
}
406415

407416
void register_paletted_block_array_class() {
417+
paletted_block_array_load_exception_entry = register_class_pocketmine_world_format_PalettedBlockArrayLoadException(spl_ce_RuntimeException);
418+
408419
memcpy(&paletted_block_array_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
409420
paletted_block_array_handlers.offset = XtOffsetOf(paletted_block_array_obj, std);
410421
paletted_block_array_handlers.free_obj = paletted_block_array_free;

‎stubs/pocketmine/world/format/PalettedBlockArray.stub.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ public function __construct(int $fillEntry){}
1212

1313
/**
1414
* @param int[] $palette
15+
* @throws PalettedBlockArrayLoadException if the provided data is invalid in any way
1516
*/
1617
public static function fromData(int $bitsPerBlock, string $wordArray, array|string $palette) : \pocketmine\world\format\PalettedBlockArray{}
1718

‎stubs/pocketmine/world/format/PalettedBlockArrayLoadException.stub.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?php
2+
3+
/**
4+
* @generate-class-entries
5+
*/
6+
7+
namespace pocketmine\world\format;
8+
9+
class PalettedBlockArrayLoadException extends \RuntimeException{}

‎stubs/pocketmine/world/format/PalettedBlockArrayLoadException_arginfo.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/* This is a generated file, edit the .stub.php file instead.
2+
* Stub hash: 99204f020e50c95977c6991a1fd9e5e72348ec77 */
3+
4+
static zend_class_entry *register_class_pocketmine_world_format_PalettedBlockArrayLoadException(zend_class_entry *class_entry_RuntimeException)
5+
{
6+
zend_class_entry ce, *class_entry;
7+
8+
INIT_NS_CLASS_ENTRY(ce, "pocketmine\\world\\format", "PalettedBlockArrayLoadException", NULL);
9+
class_entry = zend_register_internal_class_with_flags(&ce, class_entry_RuntimeException, 0);
10+
11+
return class_entry;
12+
}

‎stubs/pocketmine/world/format/PalettedBlockArray_arginfo.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: 6cf9cfe99edfe236b1fd600d5684b3b14d7f465e */
2+
* Stub hash: 474344accc11ae782ff1b7d3ba695bce5bfa3fc1 */
33

44
ZEND_BEGIN_ARG_INFO_EX(arginfo_class_pocketmine_world_format_PalettedBlockArray___construct, 0, 0, 1)
55
ZEND_ARG_TYPE_INFO(0, fillEntry, IS_LONG, 0)

‎tests/PalettedBlockArray/palette-value-overflow.phpt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ if(PHP_INT_SIZE !== 8) die("skip only for 64-bit");
88
--FILE--
99
<?php
1010

11+
use pocketmine\world\format\PalettedBlockArrayLoadException;
12+
1113
$values = [PHP_INT_MIN, -1, PHP_INT_MAX, (1 << 32)];
1214

1315
foreach($values as $v){
@@ -21,7 +23,7 @@ foreach($values as $v){
2123
try{
2224
$palette = \pocketmine\world\format\PalettedBlockArray::fromData(1, str_repeat("\x00", 512), [$v]);
2325
var_dump("bad");
24-
}catch(\InvalidArgumentException $e){
26+
}catch(PalettedBlockArrayLoadException $e){
2527
echo $e->getMessage() . "\n";
2628
}
2729
}

‎tests/PalettedBlockArray/paletted-from-data-valid.phpt‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ Test PalettedBlockArray::fromData() correctly checks payload sizes
55
--FILE--
66
<?php
77

8+
use pocketmine\world\format\PalettedBlockArrayLoadException;
9+
810
foreach([1,2,3,4,5,6,8,16] as $bitsPerBlock){
911
$blocksPerWord = intdiv(32, $bitsPerBlock);
1012
$payloadSize = (intdiv(4096, $blocksPerWord) + ((4096 % $blocksPerWord) !== 0 ? 1 : 0)) * 4;
@@ -15,21 +17,21 @@ foreach([1,2,3,4,5,6,8,16] as $bitsPerBlock){
1517
try{
1618
$palette = \pocketmine\world\format\PalettedBlockArray::fromData($bitsPerBlock, str_repeat("\x00", $payloadSize - 1), [$bitsPerBlock]);
1719
var_dump($palette->get(0, 0, 0));
18-
}catch(\RuntimeException $e){
20+
}catch(PalettedBlockArrayLoadException $e){
1921
echo "size - 1 is not allowed\n";
2022
}
2123

2224
try{
2325
$palette = \pocketmine\world\format\PalettedBlockArray::fromData($bitsPerBlock, str_repeat("\x00", $payloadSize + 1), [$bitsPerBlock]);
2426
var_dump($palette->get(0, 0, 0));
25-
}catch(\RuntimeException $e){
27+
}catch(PalettedBlockArrayLoadException $e){
2628
echo "size + 1 is not allowed\n";
2729
}
2830

2931
try{
3032
$palette = \pocketmine\world\format\PalettedBlockArray::fromData($bitsPerBlock, "", [$bitsPerBlock]);
3133
var_dump($palette->get(0, 0, 0));
32-
}catch(\RuntimeException $e){
34+
}catch(PalettedBlockArrayLoadException $e){
3335
echo "empty payload is not allowed\n";
3436
}
3537
}

‎tests/PalettedBlockArray/validate.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ foreach($samples as $k => $values){
2323
try{
2424
PalettedBlockArray::fromData($values[0], $values[1], $values[2]);
2525
echo "$k not corrupted\n";
26-
}catch(\Exception $e){
26+
}catch(PalettedBlockArrayLoadException $e){
2727
echo "$k " . $e->getMessage() . "\n";
2828
}
2929
}

0 commit comments

Comments
 (0)