search

TownyAdvanced / Towny

Towny Advanced Minecraft plugin for Bukkit/Spigot.
https://townyadvanced.github.io
Other
519 stars 353 forks source link

MojangAPI sending response 204 on accounts which still exist. #4611

Closed Klawdek closed 2 years ago

Klawdek commented 3 years ago

What steps will reproduce the problem?

  1. Upgrade from 0.96.0.0 to Towny-0.96.5.0
  2. Then watch as it removes residents from the database. It does not seem to stop doing it. It keeps removing them.

What is the expected output?

  1. Resident to not be removed

What is your Towny version?

Do not say "latest". Towny-0.96.5.0

What is your spigot/craftbukkit version?

Do not say "latest". git-Paper-335 (MC: 1.16.4) (Implementing API version 1.16.4-R0.1-SNAPSHOT)

Please use Pastebin.com to link the following files:

Skipping these files is not an option. It is the easiest way to diagnose an issue ticket properly. If you do skip this step then your ticket will most likely be deleted and you will be asked to resubmit.

  1. Your full server startup from the logs\latest.log : https://pastebin.com/VUTeyhMM
  2. Your Towny config.yml (if using MySQL please remove password,) : https://pastebin.com/QMh6Krtn
  3. Your townyperms.yml : https://pastebin.com/11HHeW6B
  4. Your log's error : https://pastebin.com/fK1vfj7d

I had just updated and allowed the config to regenerate so it is the default config. I cannot run my server until this is fixed as it looks like it is going to delete all the citizens eventually so my update is now on hold until this bug can be fixed.

It is unbelievable that that many people somehow no longer have an account. Who buys a game and then deletes their account? These people have most likely changed their names.

(optional) If this is to do with permissions,

  1. Your permissions file :

(optional) If this is to do with chat,

  1. Your chatconfig.yml :
  2. Your channels.yml :
Klawdek commented 3 years ago

I just saw that this is fixed with the dev builds. This bad version needs to be removed from spigot immediately. People are going to have all their residents deleted.

Klawdek commented 3 years ago

Problem is NOT fixed with the latest dev version Towny v0.96.5.6 It still deletes the resident. And that version adds another problem it spams the console with the change log: https://pastebin.com/EkT1SCps Please note the last line (line 505): 21.12 00:17:18 [Multicraft] Skipped 856 lines due to rate limit (500/s) So it was trying to put 1,361 lines of changelog into the console. This is probably the worst case of console spam in minecraft history. My server is now stuck offline until this problem is fixed or I make the decision to scrap towny. My server has been a towny server for three years. I would hate to loose this plugin it was a great plugin in its day.

Klawdek commented 3 years ago

I just took 14 names that towny said no longer had accounts and put them into https://namemc.com/ and in every case it was a player who had changed their name.

Klawdek commented 3 years ago

I am also wondering why Towny ever used player names to store data about the player rather than UUIDs? This is bound to cause problems. Like the one that is happening now with residents being deleted from their towns which has to do with players changing their names and no other player taking the name. When you try to migrate to UUIDs all hell is going to break loose. Some players will not be identifiable others will be identified wrong. For instance Alice changes their name to Bob. Another player takes the name Alice. When you go to migrate it will get the new Alice's UUID.

LlmDl commented 3 years ago

So it was trying to put 1,361 lines of changelog into the console. This is probably the worst case of console spam in minecraft history. My server is now stuck offline until this problem is fixed or I make the decision to scrap towny. My server has been a towny server for three years. I would hate to loose this plugin it was a great plugin in its day.

Your server should also have backups, made by Towny, but if you were updating from 1.15 to 1.16 I can assume you made full server backups as well. Roll back to your backup (towny or full server) and set the gathering uuid task to false in the config. There's nothing stopping you from using Towny for another 3 years.

I am also wondering why Towny ever used player names to store data about the player rather than UUIDs?

Towny is older than Minecraft accounts having UUIDs, the entire backend is being overhauled to support UUIDs directly. The gathering task which started to remove accounts is actually there to make it possible to convert your database to use UUIDs.

I'll be re-evaluating the connection to the mojang api, might be interesting to know more about your setup, re: is there a reason you couldn't reach Mojang's API or if you have any insight as to why it would be sending the response code 204, which is stated to be: If there is no player with the given username an HTTP status code 204 (No Content) is sent without any HTTP body.

For instance Alice changes their name to Bob. Another player takes the name Alice. When you go to migrate it will get the new Alice's UUID.

We actually check their username at the time they last logged in (for the gathering task,) so we are getting the original Alice's UUID from before they changed their name to Bob.

LlmDl commented 3 years ago

Here's what I've gotten through a bit of testing this morning, using a new MojangException, specific to only response 204:

1st 204: 99999FPS who was last online in the database on January 24, 2020, 4 days before they changed their name.

2nd 204: Devoinmonk6582 who was last online March 8, 2020, but has absolutely deleted their account.

3rd 204: Dewukk who was last online March 12, 2020, many months before they changed their name.

4th 204: Gronyak125 who was last online January 24, 2020, a couple months before they changed their name.

5-10 were all also accounts that are not deleted, according to namemc.com.

So for whatever reason mojang is actually responding with the wrong response for 9 of these 10 accounts.

I am going to remove the deleting-aspect of this until there's more is uncovered.

LlmDl commented 3 years ago

After doing some further testing with the new version that doesn't delete residents who return 204, I've found that mojang's API will return 204 for the same players each startup. Very strange stuff.

Klawdek commented 3 years ago

Thank you for the prompt and informative reply.

Towny is older than Minecraft accounts having UUIDs, the entire backend is being overhauled to support UUIDs directly. The gathering task which started to remove accounts is actually there to make it possible to convert your database to use UUIDs.

OK that explains it. Mojang screwed up to begin with by using player picked names rather than IDs assigned by an algorithm that insures no dupes and the ID is never changed. (There actually is such a thing as name dupes in minecraft https://www.youtube.com/watch?v=eWitJ-vWBdE)

I'll be re-evaluating the connection to the mojang api, might be interesting to know more about your setup, re: is there a reason you couldn't reach Mojang's API or if you have any insight as to why it would be sending the response code 204, which is stated to be: If there is no player with the given username an HTTP status code 204 (No Content) is sent without any HTTP body.

I have no idea if this could be it and I tend to doubt it, but just in case I will mention that my connection drops sometimes. It is usually not noticeable, however while uploading my upgraded (optimized) worlds to the server I did get a bunch of messages from FileZilla about overwriting files when no such error should occur as those files were not on the server to begin with. This happens when a connection is lost while a file is being transferred. When it tries again a 0 byte or lesser byte file is there. I found posts on this going back several years and I have seen it before and it does not cause any file corruptions. The day I did the upload my connection was doing it a lot and I was getting freezes on streaming videos.

However the the next day when I had the 204 problems with towny my connection seemed pretty good. Also I tried twice installing towny from backups and I am not sure but I think it was the same names both times that got 204 errors. I also noticed one player whose UUID was updated and listed in the resident files. That player has not changed their name in 2 1/2 years.

We actually check their username at the time they last logged in (for the gathering task,) so we are getting the original Alice's UUID from before they changed their name to Bob.

You lost me on that one. In my case these players may have last logged in long before I upgraded to the current version of towny that is attempting to migrate.

I do like the fact that the UUID is placed in the resident file while the names are still in other places in the file as that makes it more human readable. I rarely have to look at these files except to find the location of a deleted town (it would be nice if that info was retained when the file is moved to the deleted folder, not a big deal though with all the backups towny makes), however it is nice that it is human readable. The only problem would be when they change names all those files would have to be updated with the new name.

Again thanks for your help. I will set the gathering uuid setting to false and try again.

LlmDl commented 3 years ago

Once 0.96.5.7 is out you can actually run the gathering task, but if it gets a 204 response it will just skip to the next resident missing a UUID.

Klawdek commented 3 years ago

OK thanks. I was planning on using the stable 0.96.5.0 build with the workaround you mentioned I guess I could try the DEV 0.96.5.7 build, I didn't think it would be out that fast. I am currently working on the newly generated 0.96.5.0 config file. I will have to run the program get the new config file and start over putting my changes in.

Articdive commented 3 years ago

To solve this issue we may be able to search for the username (no timestamp) and check if we receive only 1 UUID back, we should probably use this on multiple users at once though.

LlmDl commented 3 years ago

@Articdive in my testing (other than legitimately deleted accounts,) the account is always one which has changed their name at least once. What you're talking about might work.

Edit: Actually I'm not sure this would work. Since each account that is having this issue has more than one username, we'll never be getting the proper UUID for that username via the API.