Workaround for MC-52974 ? (局域网联机时其他玩家无法看见房主的皮肤) about authlib-injector

[yushijinhun]

我和 @NekoCaffeine 进行了讨论，找到了不兼容1.7.10的原因：

1. net.minecraft.util.Session负责创建GameProfile对象，每次进服（包括本地游戏）时，它就创建一个。其创建的GameProfile不包含properties。
2. 创建的GameProfile被放入C00PacketLoginStart中，不经过序列化直接被传递到本地服务器。
3. 本地服务器取出C00PacketLoginStart中的GameProfile对象。因为进入本地游戏时不经过KEY阶段，直接跳到READY_TO_ACCEPT阶段，所以不会去调用hasJoined接口，所以本地服务器使用的GameProfile对象直接就是一开始Session所创建的对象。
4. 因为GameProfile不包含properties，所以发送的S0CPacketSpawnPlayer中也不包含properties。
5. SkinManager中存在逻辑：加载皮肤时，如果properties为空，并且加载的是自己的皮肤，就去查询皮肤。所以Host可以看到自己的皮肤，而Guest不能看到Host皮肤。

解决方法：

1. 修改Session，对其创建的GameProfile对象进行标记。
2. 修改S0CPacketSpawnPlayer，当包被序列化时，如果其中的GameProfile是被标记过的，就查询properties并加入其中。

[xfl03]

Mojang对于这个的解决方案是在服务器没有回发texture时，使用uuid前往mojang获取皮肤
不知道ai准备怎么修复

[andylizi]

Jira 上 bug 描述最后一段写的

I assume this could be fixed by having the method net.minecraft.client.Minecraft.getProfileProperties() call the method com.mojang.authlib.minecraft.MinecraftSessionService.fillProfileProperties(GameProfile, boolean) with true as requireSecure parameter value. There should probably not be any downsides.

不行吗？

[xfl03]

getProfileProperties仅对玩家自己有效，无法用这个方法补全房主皮肤

[yushijinhun]

数字签名我觉得是有必要的，还是不要绕过为好。

我发现在 https://sessionserver.mojang.com/session/minecraft/profile/ 里强行返回数字签名就可以解决这个问题，现在我在寻找检测 MC 版本的方法（只对特定版本开启这个 workaround）。

[andylizi]

getProfileProperties仅对玩家自己有效，无法用这个方法补全房主皮肤

我检查了一下 mcp，这个 bug 的原理看起来的确是如 Jira 里描述的那样，是因为单人模式下玩家的 GameProfile 中皮肤信息没有签名，当其他玩家加入游戏时（net.minecraft.server.management.ServerConfigurationManager.playerLoggedIn()）发送的 Player List Item 包（S38PacketPlayerListItem）里的数据也就不会有签名，因此不会被客户端正常显示。结论：问题的症结在于单人游戏下获取 GameProfile 的 properties 时没有包括签名。

而 GameProfile 里的皮肤信息确实是通过 YggdrasilMinecraftSessionService.fillProfileProperties(GameProfile profile, boolean requireSecure) 方法获取的，而该方法会请求 https://sessionserver.mojang.com/session/minecraft/profile/ 这个 API。结论：当第二个参数传入 true 的时候，API 返回的结果就会带上签名，因此 GameProfile 里的数据也就会有签名。

这个 fillProfileProperties() 方法我唯一搜索到的调用点就是在 SkinManager 里。综上所述，只要把这个方法调用的参数从 false 改为 true，我实在看不出为什么这样不能解决问题……用 ASM 改个 boolean 参数应该很简单才对（虽然实际上定位到这个方法在哪就是另一说了，考虑到混淆的问题），而且既然唯一的区别就是多了个签名，我也看不出会导致啥副作用。

我第一时间能想到的修改方法是类似于这样子的：

@Override
public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) {
    if (opcode == INVOKEINTERFACE && 
            "com/mojang/authlib/minecraft/MinecraftSessionService".equals(owner) && 
            "fillProfileProperties".equals(name)) {
        visitInsn(POP);        // 把原来已经在栈上的参数值 false 给 pop 掉
        visitInsn(ICONST_1);   // 然后替换为 true
    }
    super.visitMethodInsn(opcode, owner, name, desc, itf);  // 再进行方法调用
}

[yushijinhun]

@andylizi 我已经写了一个 workaround，不过我的代码是类似这样的：

diff --git a/src/main/java/moe/yushi/authlibinjector/httpd/QueryProfileFilter.java b/src/main/java/moe/yushi/authlibinjector/httpd/QueryProfileFilter.java
index 34a65e8..de77fa0 100644
--- a/src/main/java/moe/yushi/authlibinjector/httpd/QueryProfileFilter.java
+++ b/src/main/java/moe/yushi/authlibinjector/httpd/QueryProfileFilter.java
@@ -24,6 +24,9 @@ public class QueryProfileFilter implements URLFilter {
        private YggdrasilClient mojangClient;
        private YggdrasilClient customClient;
 
+       // see <https://github.com/yushijinhun/authlib-injector/issues/30>
+       private boolean mc52974WorkaroundEnabled;
+
        public QueryProfileFilter(YggdrasilClient mojangClient, YggdrasilClient customClient) {
                this.mojangClient = mojangClient;
                this.customClient = customClient;
@@ -55,6 +58,10 @@ public class QueryProfileFilter implements URLFilter {
                        withSignature = true;
                }
 
+               if (mc52974WorkaroundEnabled) {
+                       withSignature = true;
+               }
+
                Optional<GameProfile> response;
                if (QueryUUIDsFilter.isMaskedUUID(uuid)) {
                        response = mojangClient.queryProfile(QueryUUIDsFilter.unmaskUUID(uuid), withSignature);
@@ -73,4 +80,11 @@ public class QueryProfileFilter implements URLFilter {
                }
        }
 
+       public boolean isMc52974WorkaroundEnabled() {
+               return mc52974WorkaroundEnabled;
+       }
+
+       public void setMc52974WorkaroundEnabled(boolean mc52974WorkaroundEnabled) {
+               this.mc52974WorkaroundEnabled = mc52974WorkaroundEnabled;
+       }
 }

我觉得可能你的方法更好。

[yushijinhun]

@andylizi 我的想法是只对 1.7.9 到 1.12.2 之间的版本开启这个 workaround。

目前我是通过拦截 main 方法中传入的参数来判断当前版本的，因此只有执行了 main 方法才能获取到 MC 版本。而在此之前，要修改的类可能已经加载了，但那时候我无法判定我需不需要 workaround。

所以我觉得可能我的方法更好一些，因为我可以实时控制要不要返回数字签名。

[andylizi]

其实我在想，即使 1.13 没有这个问题，这个 workaround 启用了又会怎么样？反正又不会有啥副作用，还能省去检查版本的麻烦。

因为我可以实时控制要不要返回数字签名。

又没有哪种情况是“必须不能有数字签名”的。在任何时候都带上签名又不是啥缺点。

[yushijinhun]

在任何时候都带上签名又不是啥缺点。

我觉得真正的缺点，是程序没有按照约定工作。因此如果没有必要，就不要随意改变 API 的行为。对于 1.13 而言，既然它没有这个问题，就不需要去 workaround 了。如果改了的话，分明调用者没有指定 requireSecure=true，但 API 却返回了数字签名，这算不算 bug？

[yushijinhun]

@andylizi 我开了一个PR #31 ，望您能 review。

[andylizi]

我觉得真正的缺点，是程序没有按照约定工作。因此如果没有必要，就不要随意改变 API 的行为。

好吧，这个的确有道理。

我本来想参考一下 1.13 是怎么修复这个问题的，结果发现 1.13 的 MCP 还没发布……
关于 PR 的话，除了为一个小功能写了一大堆逻辑这一点感觉有点别扭外（毕竟代码越多潜在的 bug 越多），其他就没什么了。

[yushijinhun]

@andylizi 不知您是否熟悉 1.7.10 的局域网进服流程？我发现我这个 workaround 对于 1.7.10 无效，还没能找出原因。

[andylizi]

查了一下 MCP 和 1.7 最后一个版本的 Protocol Specification，发现在 1.7 里携带 GameProfile 的数据包不是 Player List Item 而是 Spawn Player（class S0CPacketSpawnPlayer） 负责的。这个包不像 Player List Item 一样是在有新玩家加入服务器时发送，而是在玩家实体进入客户端可视范围时发送的。

S0CPacketSpawnPlayer 构造器唯一被调用了的地方是在 net.minecraft.entity.EntityTrackerEntry 类的 func_151260_c() 方法（根据代码来看这方法原名应该是 addPacket，用途是构造这个 tracker entry 类所负责的实体对应的 spawn 包）：

private Packet func_151260_c() {
    if (this.myEntity.isDead) {
        logger.warn("Fetching addPacket for removed entity");
    }
    if (this.myEntity instanceof EntityPlayerMP) {
        return new S0CPacketSpawnPlayer((EntityPlayer) this.myEntity);
    }
    // ...
}

构造的数据包将会在同一类下的 tryStartWachingThis() 方法中被发送出去。

所以虽然网络协议这部分跟 1.8 以后有挺大的不同，但关于 GameProfile 的部分还是一样的——在数据包里携带 EntityPlayer.getGameProfile() 所返回的信息。

而我又搜了一下 fillProfileProperties() 方法的调用点，发现跟 1.8 一样仍然只有三处： SkinManager、领域相关代码，和 TileEntitySkull（多半是用来显示骷髅头的自定义皮肤）。SkinManager 甚至代码都没怎么变化，因此没理由认为是获取带签名的 properties 这一步出了问题。

[andylizi]

至于究竟是什么问题导致的，我现在能想出以下两种假说：

1. bug 的产生原理在 1.7.10 跟在 1.8 以上是不一样的，并不是（或者并不仅仅是）因为签名的原因，因此通过加签名的方法无法修复。
2. 构造 S0CPacketSpawnPlayer 时使用的 GameProfile （来自 EntityPlayer.getGameProfile()），跟 SkinManager 里 fillProfileProperties() 里填充的不是同一个，因此在 fillProfileProperties() 里填充签名没效果。

其他可能性好像都属于假说 1 的变体，在没有更多信息的情况下还真想不到还能是因为啥了…

排查方法：

1. Jira 的 bug 报告不是说客户端会有日志输出 Signature is missing from textures payload 么。这行 log 是在 YggdrasilMinecraftSessionService.getTextures() 方法产生的，所以如果真是签名问题的话 1.7.10 也会产生这行日志。那么如果看到了这个输出，就能确定以下两点：
   • bug 至少有一部分是因为签名导致的。但这并不能完全排除假说1，因为签名可能只是部分原因。
   • 服务端发来的 GameProfile 里没签名。大大提升了假说 2 的可能性。
2. 通过拦截数据包的方法（无论是在客户端还是服务端），确认 Spawn Player 包有没有签名。至于怎么实现的话，测试用代码自然不需要考虑多版本兼容，因此硬编码进 notch name 都没事。但仔细想想，这种方法能够证实/证伪的可能性，方法 1 好像也能做到…所以好像没必要这么麻烦的样子…
3. 这个 bug 不是只有房主的皮肤不能显示，而其他人的可以么？也许能通过某种方法对比房主的玩家实体产生的相关数据包（保险起见最好不止 Spawn Player，谁知道旧版本协议里会不会有啥奇奇怪怪的逻辑我们没有发现），和其他能正常显示皮肤的玩家的相关数据包，看看区别究竟在哪里。这个方法可以确认假说 1 和假说 2。但实现起来恐怕会很麻烦。

[yushijinhun]

我昨天在测试的时候没有看到 Signature is missing from textures payload 这条信息（已 workaround）。等我下下周期末考试之后，我弄一个 MCP 调试下吧。

[yushijinhun]

除 1.7.9 及 1.7.10 外，已在 #31 中修复。

[yushijinhun]

我为不兼容 1.7.10 的问题新开了一个 issue #35 。目前不打算解决不兼容 1.7.9 的问题。