Fixed resource packs/login sequence fail, added basic safety restrictions for packet sending before clients are logged in

close #452

src/pocketmine/Player.php

@@ -1063,6 +1063,11 @@ class Player extends Human implements CommandSender, InventoryHolder, ChunkLoade
 			return false;
 		}
 
+		//Basic safety restriction. TODO: improve this
+		if(!$this->loggedIn and !$packet->canBeSentBeforeLogin()){
+			throw new \InvalidArgumentException("Attempted to send " . get_class($packet) . " to " . $this->getName() . " too early");
+		}
+
 		$timings = Timings::getSendDataPacketTimings($packet);
 		$timings->startTiming();
 
@@ -1096,6 +1101,11 @@ class Player extends Human implements CommandSender, InventoryHolder, ChunkLoade
 			return false;
 		}
 
+		//Basic safety restriction. TODO: improve this
+		if(!$this->loggedIn and !$packet->canBeSentBeforeLogin()){
+			throw new \InvalidArgumentException("Attempted to send " . get_class($packet) . " to " . $this->getName() . " too early");
+		}
+
 		$timings = Timings::getSendDataPacketTimings($packet);
 		$timings->startTiming();
 		$this->server->getPluginManager()->callEvent($ev = new DataPacketSendEvent($this, $packet));
@@ -1734,38 +1744,40 @@ class Player extends Human implements CommandSender, InventoryHolder, ChunkLoade
 			if($p !== $this and strtolower($p->getName()) === strtolower($this->getName())){
 				if($p->kick("logged in from another location") === false){
 					$this->close($this->getLeaveMessage(), "Logged in from another location");
+
 					return;
 				}
 			}elseif($p->loggedIn and $this->getUniqueId()->equals($p->getUniqueId())){
 				if($p->kick("logged in from another location") === false){
 					$this->close($this->getLeaveMessage(), "Logged in from another location");
+
 					return;
 				}
 			}
 		}
 
-		$nbt = $this->server->getOfflinePlayerData($this->username);
+		$this->namedtag = $this->server->getOfflinePlayerData($this->username);
 
-		$this->playedBefore = ($nbt["lastPlayed"] - $nbt["firstPlayed"]) > 1; // microtime(true) - microtime(true) may have less than one millisecond difference
+		$this->playedBefore = ($this->namedtag["lastPlayed"] - $this->namedtag["firstPlayed"]) > 1; // microtime(true) - microtime(true) may have less than one millisecond difference
-		if(!isset($nbt->NameTag)){
+		if(!isset($this->namedtag->NameTag)){
-			$nbt->NameTag = new StringTag("NameTag", $this->username);
+			$this->namedtag->NameTag = new StringTag("NameTag", $this->username);
 		}else{
-			$nbt["NameTag"] = $this->username;
+			$this->namedtag["NameTag"] = $this->username;
 		}
-		$this->gamemode = $nbt["playerGameType"] & 0x03;
+		$this->gamemode = $this->namedtag["playerGameType"] & 0x03;
 		if($this->server->getForceGamemode()){
 			$this->gamemode = $this->server->getGamemode();
-			$nbt->playerGameType = new IntTag("playerGameType", $this->gamemode);
+			$this->namedtag->playerGameType = new IntTag("playerGameType", $this->gamemode);
 		}
 
 		$this->allowFlight = (bool) ($this->gamemode & 0x01);
 
-		if(($level = $this->server->getLevelByName($nbt["Level"])) === null){
+		if(($level = $this->server->getLevelByName($this->namedtag["Level"])) === null){
 			$this->setLevel($this->server->getDefaultLevel());
-			$nbt["Level"] = $this->level->getName();
+			$this->namedtag["Level"] = $this->level->getName();
-			$nbt["Pos"][0] = $this->level->getSpawnLocation()->x;
+			$this->namedtag["Pos"][0] = $this->level->getSpawnLocation()->x;
-			$nbt["Pos"][1] = $this->level->getSpawnLocation()->y;
+			$this->namedtag["Pos"][1] = $this->level->getSpawnLocation()->y;
-			$nbt["Pos"][2] = $this->level->getSpawnLocation()->z;
+			$this->namedtag["Pos"][2] = $this->level->getSpawnLocation()->z;
 		}else{
 			$this->setLevel($level);
 		}
@@ -1773,19 +1785,29 @@ class Player extends Human implements CommandSender, InventoryHolder, ChunkLoade
 		$this->achievements = [];
 
 		/** @var ByteTag $achievement */
-		foreach($nbt->Achievements as $achievement){
+		foreach($this->namedtag->Achievements as $achievement){
 			$this->achievements[$achievement->getName()] = $achievement->getValue() > 0 ? true : false;
 		}
 
-		$nbt->lastPlayed = new LongTag("lastPlayed", floor(microtime(true) * 1000));
+		$this->namedtag->lastPlayed = new LongTag("lastPlayed", floor(microtime(true) * 1000));
 		if($this->server->getAutoSave()){
-			$this->server->saveOfflinePlayerData($this->username, $nbt, true);
+			$this->server->saveOfflinePlayerData($this->username, $this->namedtag, true);
 		}
 
-		parent::__construct($this->level, $nbt);
+		$this->sendPlayStatus(PlayStatusPacket::LOGIN_SUCCESS);
+
 		$this->loggedIn = true;
 		$this->server->addOnlinePlayer($this);
 
+		$pk = new ResourcePacksInfoPacket();
+		$manager = $this->server->getResourceManager();
+		$pk->resourcePackEntries = $manager->getResourceStack();
+		$pk->mustAccept = $manager->resourcePacksRequired();
+		$this->dataPacket($pk);
+	}
+
+	protected function completeLoginSequence(){
+		parent::__construct($this->level, $this->namedtag);
 		$this->server->getPluginManager()->callEvent($ev = new PlayerLoginEvent($this, "Plugin reason"));
 		if($ev->isCancelled()){
 			$this->close($this->getLeaveMessage(), $ev->getKickMessage());
@@ -1910,13 +1932,7 @@ class Player extends Human implements CommandSender, InventoryHolder, ChunkLoade
 
 		//TODO: add JWT verification, add encryption
 
-		$this->sendPlayStatus(PlayStatusPacket::LOGIN_SUCCESS);
+		$this->processLogin();
-
-		$pk = new ResourcePacksInfoPacket();
-		$manager = $this->server->getResourceManager();
-		$pk->resourcePackEntries = $manager->getResourceStack();
-		$pk->mustAccept = $manager->resourcePacksRequired();
-		$this->dataPacket($pk);
 
 		return true;
 	}
@@ -1989,7 +2005,7 @@ class Player extends Human implements CommandSender, InventoryHolder, ChunkLoade
 				$this->dataPacket($pk);
 				break;
 			case ResourcePackClientResponsePacket::STATUS_COMPLETED:
-				$this->processLogin();
+				$this->completeLoginSequence();
 				break;
 		}
 

src/pocketmine/Server.php

@@ -2207,7 +2207,9 @@ class Server{
 
 	private function checkTickUpdates($currentTick, $tickTime){
 		foreach($this->players as $p){
-			if($this->alwaysTickPlayers){
+			if(!$p->loggedIn and ($tickTime - $p->creationTime) >= 10){
+				$p->close("", "Login timeout");
+			}elseif($this->alwaysTickPlayers){
 				$p->onUpdate($currentTick);
 			}
 		}

src/pocketmine/network/mcpe/protocol/BatchPacket.php

@@ -35,6 +35,10 @@ class BatchPacket extends DataPacket{
 		return false;
 	}
 
+	public function canBeSentBeforeLogin() : bool{
+		return true;
+	}
+
 	public function decode(){
 		$this->payload = $this->getString();
 	}

src/pocketmine/network/mcpe/protocol/ClientToServerHandshakePacket.php

@@ -29,6 +29,10 @@ use pocketmine\network\mcpe\NetworkSession;
 class ClientToServerHandshakePacket extends DataPacket{
 	const NETWORK_ID = ProtocolInfo::CLIENT_TO_SERVER_HANDSHAKE_PACKET;
 
+	public function canBeSentBeforeLogin() : bool{
+		return true;
+	}
+
 	public function decode(){
 		//No payload
 	}

src/pocketmine/network/mcpe/protocol/DataPacket.php

@@ -44,6 +44,10 @@ abstract class DataPacket extends BinaryStream{
 		return true;
 	}
 
+	public function canBeSentBeforeLogin() : bool{
+		return false;
+	}
+
 	abstract public function encode();
 
 	abstract public function decode();

src/pocketmine/network/mcpe/protocol/DisconnectPacket.php

@@ -32,6 +32,10 @@ class DisconnectPacket extends DataPacket{
 	public $hideDisconnectionScreen = false;
 	public $message;
 
+	public function canBeSentBeforeLogin() : bool{
+		return true;
+	}
+
 	public function decode(){
 		$this->hideDisconnectionScreen = $this->getBool();
 		$this->message = $this->getString();

src/pocketmine/network/mcpe/protocol/LoginPacket.php

@@ -48,6 +48,10 @@ class LoginPacket extends DataPacket{
 		return false;
 	}
 
+	public function canBeSentBeforeLogin() : bool{
+		return true;
+	}
+
 	public function decode(){
 		$this->protocol = $this->getInt();
 

src/pocketmine/network/mcpe/protocol/PlayStatusPacket.php

@@ -42,6 +42,10 @@ class PlayStatusPacket extends DataPacket{
 
 	}
 
+	public function canBeSentBeforeLogin() : bool{
+		return true;
+	}
+
 	public function encode(){
 		$this->reset();
 		$this->putInt($this->status);

src/pocketmine/network/mcpe/protocol/ServerToClientHandshakePacket.php

@@ -32,6 +32,10 @@ class ServerToClientHandshakePacket extends DataPacket{
 	public $publicKey;
 	public $serverToken;
 
+	public function canBeSentBeforeLogin() : bool{
+		return true;
+	}
+
 	public function decode(){
 		$this->publicKey = $this->getString();
 		$this->serverToken = $this->getString();