diff --git a/src/pocketmine/level/format/io/region/RegionLoader.php b/src/pocketmine/level/format/io/region/RegionLoader.php index 12d7750f6..35b2480ff 100644 --- a/src/pocketmine/level/format/io/region/RegionLoader.php +++ b/src/pocketmine/level/format/io/region/RegionLoader.php @@ -119,9 +119,6 @@ class RegionLoader{ */ public function readChunk(int $x, int $z) : ?string{ $index = self::getChunkOffset($x, $z); - if($index < 0 or $index >= 4096){ - throw new \InvalidArgumentException("Invalid chunk position in region, expected x/z in range 0-31, got x=$x, z=$z"); - } $this->lastUsed = time(); @@ -140,13 +137,13 @@ class RegionLoader{ if($length >= self::MAX_SECTOR_LENGTH){ $this->locationTable[$index][0] = ++$this->lastSector; $this->locationTable[$index][1] = 1; - throw new CorruptedChunkException("Corrupted chunk header detected (sector count larger than max)"); + throw new CorruptedChunkException("Corrupted chunk header detected (sector count $length larger than max " . self::MAX_SECTOR_LENGTH . ")"); } return null; } if($length > ($this->locationTable[$index][1] << 12)){ //Invalid chunk, bigger than defined number of sectors - \GlobalLogger::get()->error("Corrupted bigger chunk detected (bigger than number of sectors given in header)"); + \GlobalLogger::get()->error("Chunk x=$x,z=$z length mismatch (expected " . ($this->locationTable[$index][1] << 12) . " sectors, got $length sectors)"); $this->locationTable[$index][1] = $length >> 12; $this->writeLocationIndex($index); } @@ -164,10 +161,25 @@ class RegionLoader{ return substr($chunkData, 1); } + /** + * @param int $x + * @param int $z + * + * @return bool + * @throws \InvalidArgumentException + */ public function chunkExists(int $x, int $z) : bool{ return $this->isChunkGenerated(self::getChunkOffset($x, $z)); } + /** + * @param int $x + * @param int $z + * @param string $chunkData + * + * @throws ChunkException + * @throws \InvalidArgumentException + */ public function writeChunk(int $x, int $z, string $chunkData) : void{ $this->lastUsed = time(); @@ -197,14 +209,40 @@ class RegionLoader{ } } + /** + * @param int $x + * @param int $z + * + * @throws \InvalidArgumentException + */ public function removeChunk(int $x, int $z) : void{ $index = self::getChunkOffset($x, $z); $this->locationTable[$index][0] = 0; $this->locationTable[$index][1] = 0; } + /** + * @param int $x + * @param int $z + * + * @return int + * @throws \InvalidArgumentException + */ protected static function getChunkOffset(int $x, int $z) : int{ - return $x + ($z << 5); + if($x < 0 or $x > 31 or $z < 0 or $z > 31){ + throw new \InvalidArgumentException("Invalid chunk position in region, expected x/z in range 0-31, got x=$x, z=$z"); + } + return $x | ($z << 5); + } + + /** + * @param int $offset + * @param int &$x + * @param int &$z + */ + protected static function getChunkCoords(int $offset, ?int &$x, ?int &$z) : void{ + $x = $offset & 0x1f; + $z = ($offset >> 5) & 0x1f; } /** @@ -235,18 +273,23 @@ class RegionLoader{ } $data = unpack("N*", $headerRaw); + /** @var int[] $usedOffsets */ $usedOffsets = []; for($i = 0; $i < 1024; ++$i){ $index = $data[$i + 1]; $offset = $index >> 8; if($offset !== 0){ - fseek($this->filePointer, $offset << 12); + self::getChunkCoords($i, $x, $z); + $fileOffset = $offset << 12; + + fseek($this->filePointer, $fileOffset); if(fgetc($this->filePointer) === false){ //Try and read from the location - throw new CorruptedRegionException("Region file location offset points to invalid location"); + throw new CorruptedRegionException("Region file location offset x=$x,z=$z points to invalid file location $fileOffset"); }elseif(isset($usedOffsets[$offset])){ - throw new CorruptedRegionException("Found two chunk offsets pointing to the same location"); + self::getChunkCoords($usedOffsets[$offset], $existingX, $existingZ); + throw new CorruptedRegionException("Found two chunk offsets (chunk1: x=$existingX,z=$existingZ, chunk2: x=$x,z=$z) pointing to the file location $fileOffset"); }else{ - $usedOffsets[$offset] = true; + $usedOffsets[$offset] = $i; } } diff --git a/tests/phpunit/level/format/io/region/RegionLoaderTest.php b/tests/phpunit/level/format/io/region/RegionLoaderTest.php index 5bc00de63..39b69bc4f 100644 --- a/tests/phpunit/level/format/io/region/RegionLoaderTest.php +++ b/tests/phpunit/level/format/io/region/RegionLoaderTest.php @@ -25,28 +25,94 @@ namespace pocketmine\level\format\io\region; use PHPUnit\Framework\TestCase; use pocketmine\level\format\ChunkException; +use function file_exists; +use function random_bytes; +use function str_repeat; +use function sys_get_temp_dir; +use function unlink; class RegionLoaderTest extends TestCase{ - public function testChunkTooBig() : void{ - $r = new RegionLoader(sys_get_temp_dir() . '/chunk_too_big.testregion_' . bin2hex(random_bytes(4))); - $r->open(); + /** @var string */ + private $regionPath; + /** @var RegionLoader */ + private $region; + public function setUp(){ + $this->regionPath = sys_get_temp_dir() . '/test.testregion'; + if(file_exists($this->regionPath)){ + unlink($this->regionPath); + } + $this->region = new RegionLoader($this->regionPath); + $this->region->open(); + } + + public function tearDown(){ + $this->region->close(); + if(file_exists($this->regionPath)){ + unlink($this->regionPath); + } + } + + public function testChunkTooBig() : void{ $this->expectException(ChunkException::class); - $r->writeChunk(0, 0, str_repeat("a", 1044476)); + $this->region->writeChunk(0, 0, str_repeat("a", 1044476)); } public function testChunkMaxSize() : void{ $data = str_repeat("a", 1044475); - $path = sys_get_temp_dir() . '/chunk_just_fits.testregion_' . bin2hex(random_bytes(4)); - $r = new RegionLoader($path); - $r->open(); + $this->region->writeChunk(0, 0, $data); + $this->region->close(); - $r->writeChunk(0, 0, $data); - $r->close(); - - $r = new RegionLoader($path); + $r = new RegionLoader($this->regionPath); $r->open(); self::assertSame($data, $r->readChunk(0, 0)); } + + public function outOfBoundsCoordsProvider() : \Generator{ + yield [-1, -1]; + yield [32, 32]; + yield [-1, 32]; + yield [32, -1]; + } + + /** + * @dataProvider outOfBoundsCoordsProvider + * @param int $x + * @param int $z + * + * @throws ChunkException + * @throws \InvalidArgumentException + */ + public function testWriteChunkOutOfBounds(int $x, int $z) : void{ + $this->expectException(\InvalidArgumentException::class); + $this->region->writeChunk($x, $z, str_repeat("\x00", 1000)); + } + + public function testReadWriteChunkInBounds() : void{ + $dat = random_bytes(1000); + for($x = 0; $x < 32; ++$x){ + for($z = 0; $z < 32; ++$z){ + $this->region->writeChunk($x, $z, $dat); + } + } + for($x = 0; $x < 32; ++$x){ + for($z = 0; $z < 32; ++$z){ + self::assertSame($dat, $this->region->readChunk($x, $z)); + } + } + } + + /** + * @dataProvider outOfBoundsCoordsProvider + * @param int $x + * @param int $z + * + * @throws \InvalidArgumentException + * @throws \pocketmine\level\format\io\exception\CorruptedChunkException + */ + public function testReadChunkOutOfBounds(int $x, int $z) : void{ + $this->expectException(\InvalidArgumentException::class); + $this->region->readChunk($x, $z); + } }