MCL-19983 & WEB-1429 Fixed legacy skin loading & server authentication.#33
MCL-19983 & WEB-1429 Fixed legacy skin loading & server authentication.#33craftycodie wants to merge 8 commits intoMojang:masterfrom
Conversation
|
This pull request seems well coded ! |
|
I really hope Mojang gets to working on this. I've been asking for a fix for 6-7 years now. |
|
While it would be nice addition, sadly LegacyLauncher is somewhat abandoned (as it's also questionable in case of licence/ownership). I don't think it will be ever merged (but proof me wrong Mojang) |
|
👍 |
Worth a shot! |
|
I hope this gets fixed. Would like to have skins on old minecraft versions. |
|
@craftycodie Would you be OK with me using this in the MultiMC launcher startup jar? Code is here: And licensed under Apache 2.0. |
|
@peterix it can indeed, I originally wrote this fix for my own launcher for legacy versions, it might be worth you having a look at that as it's protocol contains more fixes such as online-mode. Feel free! |
src/main/java/net/minecraft/launchwrapper/protocol/SkinURLConnection.java
Show resolved
Hide resolved
|
this would also require setting minecraftArguments in the JSON to |
I think this is only required for Alpha 1.2.6. |
Did you end up adding that to MultiMC? |
craftycodie
changed the title
|
Top 10 reasons to hate Mojang: This is the most basic form of maintenance, and the fix has been sitting, ready to go for years. |
|
Any updates on this issue? |
|
At this point people should just use a mod like this. Mojang is ultimately self interested and unwilling to cooperate |
|
SkinFix was implemented in PrismLauncher 8.0 with pull request PrismLauncher/PrismLauncher#443 |
Because the community does what Ninten-- er, Mojang don't. |
|
Seems like good PR |
|
Does this fix include /whitelist add username user not found uuid endpoint error or is this only for older versions authentication & skins as stated? I know it is not called out, to be fair I did not check but hoping it does (apologies). Stellar job on these fixes by the way, thank you! It actually took a while to find ANY real info, the majority of results had a series of old issue logs on jira stating it's resolved then closed but I couldn't find an actual fix anywhere near the place. Until, perhaps, by accident, I stumbled in on an umpteenth jira post on which someone made a post that brought me here, thank you for that post too. |
maximuslotro
left a comment
maximuslotro
left a comment
There was a problem hiding this comment.
Looks good, Fixes my issues with skins when compiling and running locally!
|
LGTM: the thread |
|
@craftycodie One question. I'm pretty sure that Minecraft Beta 1.8+ is still ran through LegacyLauncher, and I don't see any mention of "session.minecraft.net" in your PR. While online mode works regardlessly on Beta 1.8+, it's still important to add the necessary layer of security to the newer versions of the game. A quick look through the code, it seems that you just need to add the second link check here, as function called afterwards works regardless of the provided URL. |
| } | ||
|
|
||
| // Server authentication is done over a newer endpoint. | ||
| if (url.toString().startsWith("http://www.minecraft.net/game/joinserver.jsp")) { |
There was a problem hiding this comment.
To add additional safety features for Beta 1.8+, this should not only support "www.minecraft.net", but also (the still working, but terrible security wise) "session.minecraft.net". Besides the different subdomain, rest of the URL is the same (and the difference doesn't matter later on, see: JoinServerURLConnection#getInputStream).
There was a problem hiding this comment.
still working, but terrible security wise
could you explain this conclusion?
There was a problem hiding this comment.
Same as with "www.minecraft.net":
- still using HTTP,
- server ID and token are still passed through the URL parameters.
Again, the only difference between "www.minecraft.net" and "session.minecraft.net" (besides the latter still working) is the subdomain, rest of the URL remains the same and rest of the code doesn't rely on the subdomain so I'm guessing it should be straight up compatible.
20 participants
Summary of Changes
MCL-19983 Skin Fix
Per MCL-19983, this pull requests fixes skins in legacy versions of Minecraft by registering a custom HTTP protocol, to essentially override requests to the URL where skins used to be back then.
The protocol uses authlib to fetch skins just like modern versions of Minecraft do.
I believe this is the only way to fix this skin issue, as skins used to be hosted at s3.amazonaws.com, it is unlikely it can be resolved server-side.
WEB-1429 Server Login Fix
As described in WEB-1429, it is currently impossible to join secured (
online-mode=true) legacy servers. The game used to authenticate via minecraft.net, but this has since been moved. I have solved this using a custom HTTP handler which intercepts the server login request before it is performed, and uses Authlib to perform this request instead.I believe this is the only suitable solution for this bug, as legacy minecraft versions are written to use HTTP rather than HTTPS, and include the access token in the URL, so handling this on the server side is insecure, and it should be done on the client side instead.
Additional Information
Since legacy Minecraft does not support the slim skin model, slim skins will be appropriately stretched out for the classic model. The screenshot below is an example of a slim skin.
I have also updated log4j, due to the infamous vulnerability, you can see #34 for more on that.
Deployment
Were this pull request accepted, to deploy the fix would require updating launcher manifest JSON files for legacy versions. The json files would need launchwrapper bumped to the latest version, and would need some additional dependencies for authlib, the same ones modern minecraft has.
The new dependencies are:
Testing Guidance
Skins
Server Login