LoginPacketHandler: stop bailing on unexpected JSON properties

the intent of this was noble (to make sure nothing was missed), but in practice throwing
errors on this stuff is just a pain in the ass. We don't actually need to care if the
properties are not used, since it doesn't affect the decoding (like it would for
a missing packet field), so the only reasons to complain are for BedrockProtocol to
have a complete picture of the protocol, and to make sure we're not discarding useful
information.

Log a warning in these cases instead, which should be noticed by developers without
being an unnecessary problem for users.

closes #6816
This commit is contained in:
Dylan K. Taylor
2025-09-26 23:04:23 +01:00
parent d6e05c90fe
commit 1c92d66ca4
2 changed files with 20 additions and 8 deletions

View File

@@ -114,7 +114,7 @@ class LoginPacketHandler extends PacketHandler{
throw new PacketHandlingException("Unexpected type for self-signed certificate chain: " . gettype($chainData) . ", expected object");
}
try{
$chain = $this->defaultJsonMapper()->map($chainData, new LegacyAuthChain());
$chain = $this->defaultJsonMapper("Self-signed auth chain JSON")->map($chainData, new LegacyAuthChain());
}catch(\JsonMapper_Exception $e){
throw PacketHandlingException::wrap($e, "Error mapping self-signed certificate chain");
}
@@ -132,7 +132,7 @@ class LoginPacketHandler extends PacketHandler{
}
try{
$claims = $this->defaultJsonMapper()->map($claimsArray["extraData"], new LegacyAuthIdentityData());
$claims = $this->defaultJsonMapper("Self-signed auth JWT 'extraData'")->map($claimsArray["extraData"], new LegacyAuthIdentityData());
}catch(\JsonMapper_Exception $e){
throw PacketHandlingException::wrap($e, "Error mapping self-signed certificate extraData");
}
@@ -244,7 +244,7 @@ class LoginPacketHandler extends PacketHandler{
throw new PacketHandlingException("Unexpected type for auth info data: " . gettype($authInfoJson) . ", expected object");
}
$mapper = $this->defaultJsonMapper();
$mapper = $this->defaultJsonMapper("Root authentication info JSON");
try{
$clientData = $mapper->map($authInfoJson, new AuthenticationInfo());
}catch(\JsonMapper_Exception $e){
@@ -258,7 +258,7 @@ class LoginPacketHandler extends PacketHandler{
* @throws PacketHandlingException
*/
protected function mapXboxTokenHeader(array $headerArray) : XboxAuthJwtHeader{
$mapper = $this->defaultJsonMapper();
$mapper = $this->defaultJsonMapper("OpenID JWT header");
try{
$header = $mapper->map($headerArray, new XboxAuthJwtHeader());
}catch(\JsonMapper_Exception $e){
@@ -272,7 +272,7 @@ class LoginPacketHandler extends PacketHandler{
* @throws PacketHandlingException
*/
protected function mapXboxTokenBody(array $bodyArray) : XboxAuthJwtBody{
$mapper = $this->defaultJsonMapper();
$mapper = $this->defaultJsonMapper("OpenID JWT body");
try{
$header = $mapper->map($bodyArray, new XboxAuthJwtBody());
}catch(\JsonMapper_Exception $e){
@@ -291,7 +291,7 @@ class LoginPacketHandler extends PacketHandler{
throw PacketHandlingException::wrap($e);
}
$mapper = $this->defaultJsonMapper();
$mapper = $this->defaultJsonMapper("ClientData JWT body");
try{
$clientData = $mapper->map($clientDataClaims, new ClientData());
}catch(\JsonMapper_Exception $e){
@@ -329,12 +329,21 @@ class LoginPacketHandler extends PacketHandler{
$this->server->getAsyncPool()->submitTask(new ProcessLegacyLoginTask($legacyCertificate, $clientDataJwt, rootAuthKeyDer: null, authRequired: $authRequired, onCompletion: $this->authCallback));
}
private function defaultJsonMapper() : \JsonMapper{
private function defaultJsonMapper(string $logContext) : \JsonMapper{
$mapper = new \JsonMapper();
$mapper->bExceptionOnMissingData = true;
$mapper->bExceptionOnUndefinedProperty = true;
$mapper->undefinedPropertyHandler = $this->warnUndefinedJsonPropertyHandler($logContext);
$mapper->bStrictObjectTypeChecking = true;
$mapper->bEnforceMapType = false;
return $mapper;
}
/**
* @phpstan-return \Closure(object, string, mixed) : void
*/
protected function warnUndefinedJsonPropertyHandler(string $context) : \Closure{
return fn(object $object, string $name, mixed $value) => $this->session->getLogger()->warning(
"$context: Unexpected JSON property for " . (new \ReflectionClass($object))->getShortName() . ": " . $name . " = " . var_export($value, return: true)
);
}
}

View File

@@ -5,6 +5,9 @@ class JsonMapper_Exception extends \Exception{}
class JsonMapper{
/** @var ?\Closure(object, string, mixed) : void */
public $undefinedPropertyHandler = null;
/**
* @template TModel of object
*