SpigotMC / BungeeCord

BungeeCord, the 6th in a generation of server portal suites. Efficiently proxies and maintains connections and transport between multiple Minecraft servers.
https://www.spigotmc.org/go/bungeecord
Other
1.56k stars 1.1k forks source link

Bungee can not connect to Forge/MCPC in 1.7 #899

Closed mindforger closed 9 years ago

mindforger commented 10 years ago

Using Bungee #847 and MCPC 1.7 B74 i can not connect via Bungee to the MCPC

i got stuck during login and get a netty redtimeout and on the mcpc i only get "user disconnected", no more errors or warnings or any logs

as already said in IRC i asked for forge support and got told there were some changes that may have not been considered in bungee implementation.

I am sorry that i can not provide more details, but i really hope it will be availabe again soon.

mindforger commented 10 years ago

Please, if i can help finding the problem in any way tell me how, i do not want to let this awesome feature from 1.6 die! i want to make an hybrid server(i had a working testsetup in 1.6.4) for both types of players, the one who like a challenge and the one who like vanilla!

Al3XKOoL commented 10 years ago

Same problem. I am not able to find a way. I've tried all the latest versions of MCPC+ and BungeeCord but none seem to work. We can only hope that the magnificent md_5 have some time to watch this.

Greetings and thanks.

ninja- commented 10 years ago

forge support was removed for 1.7 because of protocol changes. if anyone is aware what forge changed in 1.7 protocol, maybe this can be added again

mindforger commented 10 years ago

i got an answer https://github.com/MinecraftForge/FML/issues/391 maybe this can help you

i did not know that cpw quit programming :( i hope this will not impact future development of bungee too hard.

Thank you for your attention on my issue!

jeffreykog commented 10 years ago

The only thing FML changes in vanilla's packets is that it adds an additional json entry to the serverinfo packet (Protocol: status, ID: 00) which tells the client what kind of server this is.

Json structure of vanilla server:

{"status":{
    "description": ...,
    "players": ...,
    "version": ...,
    "favicon": ...
}}

Json structure of an FML server:

{"status":{
    "description": ...,
    "players": ...,
    "version": ...,
    "favicon": ...,
    "modinfo":{
        "type":"FML" ,  //This can be "FML", "BUKKIT" and "VANILLA". The client will react in adding a logo for it in the multiplayer list. Required
        "modList":[       //This is just a list of all serverside mods. Required but may be empty
            {"modid":..., "version":...}
        ],
        "clientModsAllowed": true   //This field is optional and defaults to true. When set to false, FML clients will refuse to login
    }
}}

If more information is needed, ask me here or on IRC. I have a good understanding of FML's new network system and i hope we can work something out regarding BungeeCord support.

jeffreykog commented 10 years ago

The only problem that exists is that FML servers kick every non-FML client. This is easy to hack. We have to identify bungeecord as an FML client to FML servers, so we have to do the handshake with it. After that, we can connect to FML. By default FML waits 10 hours for this handshake, after that kicking the player. This is what causes the readtimeout @mindforger spoke about.

mindforger commented 10 years ago

sooooo ... bungee needs some way to pass through the handshake or do an pseudoandshake with the client first to get all necessary informations !? sounds like a chance to get rid of the cryptic "you are missing the following stuff" message and replace it with "hit the update button" i really hope the problem is not getting any deeper, owe you a big one jk-5 :+1:

jeffreykog commented 10 years ago

I'm going to try implementing this today. There are a few things that could give us some troubles. When a FML client joins an FML server, the handshake will occur and the server will send block name to blockid mappings to the client, we need to pass this thru, that's not a problem. When a FML client switches from one FML server to another FML server using BungeeCord, we need to have a way to tell the client "forget your current stuff" and we need to resend the data. That could give us some problems and i think i need to do a PR for FML to add support for that. When that comes thru, i'm sure we could fix this.

mindforger commented 10 years ago

i already asked in the issue https://github.com/MinecraftForge/FML/issues/391 just that, but got no answer ... but you can fork this issue, it's linked with this one here

Allnoob007 commented 10 years ago

Hello, i wanted to ask how the fix is going, as i am also waiting for it on my server ?

jeffreykog commented 10 years ago

Sorry for the radio silence. Currently adding forge support would require us to identify ourselves as FML to each client handshaking with us and sending all mods that could potentially be on a sub-server to the client. This would be impossible for us. We have to add some hooks in FML that allows the server to send mod information to the client while being connected. I will look into that how this would work in FML and try some stuff.

jeffreykog commented 10 years ago

Small update on this. Working on it. FML handshake between client and server is almost working

jeffreykog commented 10 years ago

Screenshot

jeffreykog commented 10 years ago

https://github.com/jk-5/BungeeCord/commit/ad5fec14c6705c61a537e9c25c473542f44f6efc

It works, but in almost the half of the login attempt a race condition prevents the client to join the server. I think it is because the FML handshakes are PluginMessages which are sent as early as possible. Perhaps someone can take a look why this is not working, i don't know where in the code would be a good point to send them

iAd4m commented 10 years ago

Could you upload a dev version of this? I know that it will deny the access sometimes but it works!

mindforger commented 10 years ago

i am not familiar with github at all but shouldn't it be possible (for you and me iAd4m) to checkout the original src + jk-5's addition somehow? I want to look into that too now as my modpack is starting to feel like it is ready to use :P and i have more time the next weeks

jeffreykog commented 10 years ago

It's not 'denying sometimes' but accepting sometimes. I think i'm doing the handshake way to early. I'll try fixing that this afternoon.

It's possible to check my source out and to build it. Make sure you get the 'fml' branch

i am not familiar with github at all but shouldn't it be possible (for you and me iAd4m) to checkout the original src + jk-5's addition somehow? I want to look into that too now as my modpack is starting to feel like it is ready to use :P and i have more time the next weeks

— Reply to this email directly or view it on GitHub.

iAd4m commented 10 years ago

ok ill check it out soon

mindforger commented 10 years ago

i have tried to review my protocol(kind of hard to strip the headerinformation and assemble the whole stream) i sent you the other day and looking at your omplementation i would suggest to put the actual handshake behind the connect, as it looks like the session data will be transmitted first and then the handshake ... i did not manage to check out the project yet so i am not aware what userCon.connect( server, null, true ); is doing, i am just guessing!

                         userCon.connect( server, null, true );
                         //Start FML handshake. This is ignored by vanilla clients
                         unsafe.sendPacket( PacketConstants.FML_REGISTER );
                         unsafe.sendPacket(PacketConstants.FML_START_CLIENT_HANDSHAKE);

i will check back when i got the whole thing running

iAd4m commented 10 years ago

ok glad to see someone thats taking the time to fix this

mindforger commented 10 years ago

another more refined version of my login capture

http://paste.md-5.net/ropujokuti.1c left original (some edits), right your version as far as my checkout is working :P

Halornek commented 10 years ago

Just wondering if there is any new information relating to this. Thanks.

dualspiral commented 10 years ago

I took a look at the work @jk-5 did, and I spotted the problem. The race condition was caused by Bungee not waiting for the Server Hello message and just sending the client response to this. Usually, the client would send the response before the server got a chance to say "Hello", and thus, the server login didn't accept the connection - because it wasn't ready to!

The fix is to just provide another packet state to check for, namely, the "ServerHello" state (which the server sends first), and start the handshake from there. With this update, I've not encountered these race conditions upon connection.

You can see the code at https://github.com/dualspiral/BungeeCord/commits/fml. It has also been updated to include the latest Bungee code.

I would not call this code complete - there is still a fair amount of work to be done, and there might be other coding issues that currently means that they would not be accepted into the master repo. More importantly, though, if you try to mix vanilla and forge servers, you are going to have problems connecting to the forge servers from a Vanilla server (your clients will get NullPointerExceptions) - the current code I have just doesn't handle that transition at all. I may get to looking at that, but I most likely will not, as I won't be using this myself, I only tried to get it working to this point for a friend!

Essentially, if you do build and use this code, use it for all Forge servers, (or all Vanilla servers but then, if you have all Vanilla servers, then you should just use the Vanilla BungeeCord!).

md-5 commented 10 years ago

@dualspiral I've created a PR: https://github.com/SpigotMC/BungeeCord/pull/1091 to track your code (since you are the only one really looking into it). Let me know when you think the bugs are ironed out / its ready for review. Ideally (if possible) both forge and not forge servers should be supported for the same user on the same connection.

dualspiral commented 10 years ago

Alright. I'll take a look.

Just had another quick play this morning, it turns out, in it's current state, you can have both forge and vanilla servers IF your initial lobby server is a Forge server. I think some work needs to be done properly responding to the FML handshake between user client and Bungee. It works when connecting to a Forge server initially, as the handshake data from the server is handed back to the client (except for the hello), but when connecting to a vanilla server first off, the handshake isn't completed, so I think the client acts as a vanilla client - and thus crashes when suddenly it's sent to a modded server.

I'll have a go when I get some time to do so.

md-5 commented 10 years ago

What we used to do is make bungee itself pretend to be forge.

dualspiral commented 10 years ago

Ultimately, I think that's what will have to happen here too. The plan is to send an empty mod list and then some sort of server ID if the client responds to a forge hello message to complete the handshake, and then accept all connections, letting the Bungee <-> Server bridge decide if the mod list is acceptable. I'm hoping that this will resolve the issue, and I'll then be able to look at cleaning up the code and more bug hunting.

For reference - the way the handshake is intended to work is described at https://github.com/MinecraftForge/FML/blob/master/src/main/java/cpw/mods/fml/common/network/handshake/FMLHandshakeClientState.java#L15 - that's what I'll be working towards.

ericboakye commented 10 years ago

When do you think this will be fixed? I have a pixelmon server to run :P

mindforger commented 10 years ago

what the heck? i just lost track of this issue as i gave up getting the spigot into my eclipse ... god damnit lombok ... i see there a problem, if the initial lobby is a bungee, no unmodded user will be able to join any server related with that bungee ? or am i thinking too complicated? if bungee itself could deal with that stuff and remembers if the user is modded or not, an option for "modded: yes" in the server config to automatically block non modded clients detected by bungee would be nice. maybe add a message option to return a link for example to tell people where to get the mod pack

dualspiral commented 10 years ago

@XenoJava It'll be done when it's done. We can try to be be as quick as we can - but the quickest way to get this fixed is to help fix the bugs yourself! If you can't do that, then just sit tight please - asking for when it'll be done just distracts us from actually doing it. :)

@mindforger I'm struggling to see what you mean, and if you are understanding it fully or not. Let me explain what the current issue is:

Imagine you have two servers, a Vanilla one, and a Forge one. Your lobby is the Vanilla server. A player can join that no problem with their Forge client, but will get disconnected due to a NullPointerException when they try to switch to the Forge server. I'm looking at fixing this problem. An all Forge server setup works fine with the code that @jk-5 and I have written.

If you just want to block non-modded clients, just have a Forge lobby server. There will be no need to add more code to Bungee to do that - vanilla clients will just get kicked by the server if they don't have the required mods.

mindforger commented 10 years ago

yeah i see what you mean, my initial intetion was a hybrid server, and as far as i understand forge, it should be possible to resend the handshake whenever you want to (thats what i got told in forge channel)

i want to have both modded clients and vanilla clients to play on 2 subserver one modded one not modded ... so both can chat with each other and the modded people could switch seamless to the vanilla server if they want or if for example a game event is starting they could join it without the need to switch the server. And for those people who yet do not have the modpack but try to join the modded server however get filtered by the "modded" flag and get a "message" what they need to do to join this area for example

i know this is not the place to talk about that, but i really would like to get into this stuff and help but i did not get lombok working with my eclipse and i did not understand how to work IntelliJ thing ... it's so confusing

i do not understand the statemachine for the handshake in bungee but i got told in the forge channel : passing through all forge packages would be enough, i just had to ensure to send an vanilla style or "empty" mod list and the relevant default item IDs to the client when switching back to vanilla

the only needed effort is to create a "vanilla" forge handshake or to redirect the client over an unmodded server to reset it's config to default before entering the real vanilla server... i can not tell what the real truth is and what the problem are

i just can thell what i got informed about when doing research about this and what i've learned from my package analysis

dualspiral commented 10 years ago

Actually @mindforger - that was useful information, and confirmed my thinking, so thank you for that. I've pushed a commit to the PR (dualspiral/BungeeCord@3d521e0) - which seems to fix this particular issue. The only downside is that I have had to include the vanilla block list message in Bungee, which I'm not sure I'd be keen on, but at least it shows that it fixes the issue at hand - and alternative solutions can now be sought (if that's what is wanted.)

Attempting to use an item that doesn't exist on the server (for example, in Creative) will cause the server to throw you off (due to a NullPointerException), but it's possible there is no way we can control that.

Zero-Gravity commented 10 years ago

Really Excited to see this come out. I own a pixelmon hub and was unaware of the 1.7 issue and cant really do much until its fixed. But take your time and make it good. It seems like a great thing so far

floh22 commented 10 years ago

Got an error with the fork @dualspiral made: http://pastebin.com/emAakPmH

dualspiral commented 10 years ago

@floh22 Thanks, but I've just downloaded and installed Pixelmon, and I'm unable to reproduce this error. Could you tell me what mods you were running - and does it happen to you all the time?

floh22 commented 10 years ago

Im using Pixelmon and CustomNpcs server side. It happens every time I connect. Client side mods are the same forge v.1121

ericboakye commented 10 years ago

@floh22 Are you using cauldron?

floh22 commented 10 years ago

@XenoJava yes. I just realised that this might be the cause

dualspiral commented 10 years ago

It's not. I'm running Cauldron.

floh22 commented 10 years ago

what version? Im using the latest one available

ericboakye commented 10 years ago

@floh22 I use that version and never had an error like that.

floh22 commented 10 years ago

@XenoJava thats so odd.. Il set up a new clean server and see if it still happens. Im probably being stupid though. xD

edit: just tried it with a clean setup. I got a read timed out error and couldnt connect to the server.

dualspiral commented 10 years ago

@floh22 My thought is that it's your CustomNpcs mod. It's probably sending huge packets over the network, causing the negative array you are seeing. I'll test it when I get a moment.

As far as I can see, Cauldron is essentially Forge that can run plugins. There should be nothing different between that and Forge when it comes to Bungee setup.

Finally, just as a side note, I really really really want to stress that this Bungee code is not official, not reviewed, not fully tested. I urge people not to test this in a live environment, as it may cause data loss, computer crashes, drive you insane, annoy your cat and upset your dog*. However, error reports are appreciated from test environments, so thank you for that!

*(all of these might not be true. Best not take a risk, though, eh?)

floh22 commented 10 years ago

@dualspiral any way to filter out the packets during the login process? Il try without in a sec. I set up 2 more testing enviroments so il see what I can find out

edit: seems like you were right. removing customNpcs fixed it.

edit 2: aha now we are getting somewhere. Only CustomNpcs works. Only Pixelmon works as well. Together they seem to overload the network like you said.

edit 3: It just worked... I set connection_throttle to -1 and timeout to 300000. Not sure if that did it but....

edit 4: now it doesnt work again. Its a 50/50 chance it seems..

dualspiral commented 10 years ago

@floh22 The best thing to do to confirm this would be to just remove the mod.

I've got the error now, and it is the CustomNpcs mod sending a packet bigger than 32kiB over the network. The datatype that checks how big it is can only count to 32767, and then thinks the next number is -32768. It then tries to make a sequence of bytes that is smaller than zero, which can never happen! Thus, you get that crash.

In my case, and I expect yours, the length of the packet is just short of 34kiB. This would require some changes in Bungee's packet handlers.

floh22 commented 10 years ago

@dualspiral is it doable? xD This isnt my area of expertise xD I can do a bit of java, but not this haha. Anyway, I appreciate the work you've done already, Itl really help a lot! The only thing which doesnt make sense though is why it works without Pixelmon. it works 100% of the time alone, with Pixelmon it doesnt.

dualspiral commented 10 years ago

@floh22 In theory yes, I have had a look at the protocol, and locally converted the offending short into a int that gets the short value as an unsigned value (Line 42 of DefinedPacket.java in the protocol project, replace with int len = buf.readUnsignedShort();, or do a bitwise AND with 0xffff), which should (I think) have given me the correct length. Unfortunately, at least with this mod, it actually comes up 1 byte short, which causes another error as Bungee detects that it hasn't read the whole packet.

I don't know of any other mods that send packets over 32kiB, so I can't check to see if this "off by one error" affects other mods that use large packets or not. If it indeed was universal, I'm not sure that Bungee should adopt incorrect behaviour. Either that, or I'm doing or understanding something wrong...

(For reference, the reverse for the writeArray method is to replace line 36 of the same file with buf.writeShort( b.length & 0xff );, to get the last 2 bytes of the int into a signed short correctly.)

floh22 commented 10 years ago

Hm so its not possible to fix at the moment? Could I rewrite anything in the mod to make the packets smaller without having to rewrite the clientside as well? As far as I know its not possible but I just wanted to check.

jeffreykog commented 10 years ago

We know if a client is modded or not. When it is not modded, just filter out the packets > 32kb

Hm so its not possible to fix at the moment? Could I rewrite anything in the mod to make the packets smaller without having to rewrite the clientside as well? As far as I know its not possible but I just wanted to check.

— Reply to this email directly or view it on GitHub.

floh22 commented 10 years ago

@jk-5 Well my client is modded so I dont know how that would help... I need to be able to connect with CustomNpcs and Pixelmon, not just one or the other. Ofc both are installed on the client as well. I just dont want to have to change the mod clientside