benbaptist / minecraft-wrapper

A simple & intuitive Minecraft Server wrapper. Supports IRC, backups, a plugin system, and more.
http://wrapper.benbaptist.com/
Other
73 stars 20 forks source link

server.properties temporary modification system #525

Open benbaptist opened 6 years ago

benbaptist commented 6 years ago

That title pretty much makes no sense. Allow me to explain!

While my knowledge of the modern vanilla server.jar is a bit out-of-date, I believe the server.properties file is still only read once: when the server first boots. We can use this to our leverage to modify certain properties for the vanilla server to see, but then revert those changes as soon as the server is fully up-and-running.

In other words, we can make a much simpler, easier-to-setup proxy system (among other things that could benefit from this).

Instead of making the user configure both wrapper.properties.json (setting the "accessable" port that clients connect to, etc.) and server.properties (disabling online-mode, changing server-port to something arbitrary, turning off packet compression, etc.), Wrapper can simply leverage the port already set in server.properties for the Proxy to bind to (BEFORE starting the server!), then temporarily overwrite the existing server.properties with a modified one containing a randomized, open port, online-mode set to false, etc. and by the time the server is fully launched, putting the old server.properties back. The server.jar won't care that the server.properties is different, and thus, the original settings are safe and no problems should arise from this.

It should be noted that every time the server restarts, the server.properties should be re-loaded back into Wrapper's memory (in case the user made adjustments), and the overwriting of server.properties needs to take place just before the actual server.jar is started for this to work.

For extra safety, the old server.properties can always be placed as something like server.properties.bak, and we can have a flag in the temporary, modified one that indicates to Wrapper that this was a modified, temporary file. Then, if something is interrupted DURING the server start and the server.properties remains overwritten, we can perform a check and restore the proper server.properties (with the user-set settings, not the randomized port).

Apologies if this is rather confusingly written. I can elaborate as needed. Also, my apologies if this has already been suggested someplace else. I just did a quick check of the issues tab and didn't see anything like this. But this is one of the features I had been intending to implement since the days when I was still actively maintaining this project.

suresttexas00 commented 6 years ago

That sounds nice. I will say that one change made while you were gone was that "server port" is no longer used in the proxy setup. You set the server port in the wrapper.properties and proxy port in the wrapper.properties.json. Proxy gets the server port during the server startup automatically.

suresttexas00 commented 6 years ago

Also, I don't turn off packet compression. As far as I know, it still performs fine.

benbaptist commented 6 years ago

Yeah, disabling compression is totally optional. I just figure it's a waste, since it's being compressed twice at that point - once by the server, and once by Wrapper. And it has to decompress it between those two, so it's just using extra CPU. That being said, this was how it worked in the old days - are we just passing through chunks now? If so, nevermind.

And indeed, I did notice the port that the proxy connects to automatically adapts to whatever the server is bound to. This can remain the same, and I didn't mean to reference this port. I meant changing the proxy's bind port via the server.properties port, then temporarily swapping the server.properties port for something random and unused, starting the server (and proxy noticing the random port being used), and so on, etc.

suresttexas00 commented 6 years ago

I understood. I marked it as a future item for a post-stable release. I'd rather not tackle something like this right now. There are too many other truly broken items to attend to first.

benbaptist commented 6 years ago

Yeah, I agree. That's a good idea.

suresttexas00 commented 6 years ago
Yeah, disabling compression is totally optional. I just figure it's a waste, since it's being compressed
 twice at that point - once by the server, and once by Wrapper. And it has to decompress it between
 those two, so it's just using extra CPU. That being said, this was how it worked in the old days - 
are we just passing through chunks now? If so, nevermind.

That is a good point; we Should be passing raw packets for stuff we don't parse! That could get us a real speed boost!

My slower connection people have fits with wrapper at times. This double compression affect may be the source. However, Minecraft packets have continued to bloat as new types types of metadata continue to become long strings (versus varint codes, for example), so not compressing them may be just as bad these days.

suresttexas00 commented 6 years ago

Well, so far the only kink in this could be things list /whitelist which will reset the server properties to whatever the server thought the contents were (i.e., what it was started with...). Not a real big deal, I think, since we are now intercepting the whitelist command (in proxy mode). It is something we need to be mindful of, though.

suresttexas00 commented 6 years ago

Instead of a random server port, I think we need some standardization. I propose the server port be set two hundred above the proxy port: Proxy 25565; server 25765. With a predictable port pattern (versus random), the user can ensure that they don't inadvertently expose the offline server port to the whole world.

suresttexas00 commented 6 years ago

I must say, this was a brilliant idea!! The next wrapper build will now only run a decompress on packets that come in (can't get around that because you have to decompress to get the packet ID). However, if a packet is not being parsed, the whole raw packet (compressed or not) is just re-transmitted, eliminating the re-compress! The only packets needing (re)compression will be those that get changed by a parser.