Compare commits

...

3 Commits

Author SHA1 Message Date
549fb923bf Release 3.26.3 2021-12-10 17:55:07 +00:00
6d5c463bdd PlayerExperienceChangeEvent: added range checks to setNewProgress()
WE FINALLY FUCKING FOUND IT

This took several years to identify because PHP's exception stack traces don't show the actual values of parameters, but rather the values of the variables they were assigned to.

This means that if the parameter variable is mutated, the exception trace will show the value of the variable inside the function, not the value that was actually passed.
2021-12-10 17:29:57 +00:00
911ad344c9 Human: do not mutate parameter variables in setXpAndProgress()
this caused a mystery that took 3 entire years to debug.
2021-12-10 17:27:28 +00:00
4 changed files with 17 additions and 13 deletions

View File

@ -17,3 +17,7 @@ Plugin developers should **only** update their required API to this version if y
- Improved error messages shown by `start.cmd`, `start.sh` and `start.ps1` when the PHP binary was not found.
- The value of PHPRC is now shown when erroring out due to unsatisfied PHP requirements.
- Removed restriction on the range of valid channels for `auto-updater.channel` in `pocketmine.yml`.
# 3.26.3
- `PlayerExperienceChangeEvent->setNewProgress()` now performs range checks. This fixes the root of a very old and confusing crash bug which took several years to identify the cause of.
- Note that the defective plugin(s) which caused this problem will still cause a server crash, but the plugin responsible will now get blamed correctly.

View File

@ -34,5 +34,5 @@ const _VERSION_INFO_INCLUDED = true;
const NAME = "PocketMine-MP";
const BASE_VERSION = "3.26.3";
const IS_DEVELOPMENT_BUILD = true;
const IS_DEVELOPMENT_BUILD = false;
const BUILD_CHANNEL = "pm3";

View File

@ -57,7 +57,6 @@ use pocketmine\network\mcpe\protocol\types\inventory\ItemStackWrapper;
use pocketmine\network\mcpe\protocol\types\PlayerListEntry;
use pocketmine\network\mcpe\protocol\types\SkinAdapterSingleton;
use pocketmine\Player;
use pocketmine\utils\AssumptionFailedError;
use pocketmine\utils\UUID;
use function array_filter;
use function array_merge;
@ -70,7 +69,6 @@ use function max;
use function min;
use function mt_rand;
use function random_int;
use function sprintf;
use function strlen;
use const INT32_MAX;
use const INT32_MIN;
@ -397,9 +395,6 @@ class Human extends Creature implements ProjectileSource, InventoryHolder{
$xpLevel = (int) $newLevel;
$xpProgress = $newLevel - (int) $newLevel;
if($xpProgress > 1.0){
throw new AssumptionFailedError(sprintf("newLevel - (int) newLevel should never be bigger than 1, but have %.53f (newLevel=%.53f)", $xpProgress, $newLevel));
}
return $this->setXpAndProgress($xpLevel, $xpProgress);
}
@ -447,24 +442,26 @@ class Human extends Creature implements ProjectileSource, InventoryHolder{
}
protected function setXpAndProgress(?int $level, ?float $progress) : bool{
$newLevel = $level;
$newProgress = $progress;
if(!$this->justCreated){
$ev = new PlayerExperienceChangeEvent($this, $this->getXpLevel(), $this->getXpProgress(), $level, $progress);
$ev = new PlayerExperienceChangeEvent($this, $this->getXpLevel(), $this->getXpProgress(), $newLevel, $newProgress);
$ev->call();
if($ev->isCancelled()){
return false;
}
$level = $ev->getNewLevel();
$progress = $ev->getNewProgress();
$newLevel = $ev->getNewLevel();
$newProgress = $ev->getNewProgress();
}
if($level !== null){
$this->getAttributeMap()->getAttribute(Attribute::EXPERIENCE_LEVEL)->setValue($level);
if($newLevel !== null){
$this->getAttributeMap()->getAttribute(Attribute::EXPERIENCE_LEVEL)->setValue($newLevel);
}
if($progress !== null){
$this->getAttributeMap()->getAttribute(Attribute::EXPERIENCE)->setValue($progress);
if($newProgress !== null){
$this->getAttributeMap()->getAttribute(Attribute::EXPERIENCE)->setValue($newProgress);
}
return true;

View File

@ -79,6 +79,9 @@ class PlayerExperienceChangeEvent extends EntityEvent implements Cancellable{
}
public function setNewProgress(?float $newProgress) : void{
if($newProgress < 0.0 || $newProgress > 1.0){
throw new \InvalidArgumentException("XP progress must be in range 0-1");
}
$this->newProgress = $newProgress;
}
}