itzg / rcon-cli

A little RCON cli based on james4k's RCON library for golang
Apache License 2.0
175 stars 15 forks source link

Response buffer of 4096 is too small for some commands #42

Open josef-dijon opened 11 months ago

josef-dijon commented 11 months ago

Running rcon-cli for my minecraft server I can see that the response buffer of rcon-cli is limited to 4096 as can be seen below:

$ rcon-cli help
/advancement (grant|revoke)/attribute <target> <attribute> (get|base|modifier)/execute (run|if|unless|as|at|store|positioned|rotated|facing|align|anchored|in|summon|on)/bossbar (add|remove|list|set|get)/clear [<targets>]/clone (<begin>|from)/damage <target> <amount> [<damageType>]/data (merge|get|remove|modify)/datapack (enable|disable|list)/debug (start|stop|function)/defaultgamemode <gamemode>/difficulty [peaceful|easy|normal|hard]/effect (clear|give)/me <action>/enchant <targets> <enchantment> [<level>]/experience (add|set|query)/xp -> experience/fill <from> <to> <block> [replace|keep|outline|hollow|destroy]/fillbiome <from> <to> <biome> [replace]/forceload (add|remove|query)/function <name>/gamemode <gamemode> [<target>]/gamerule (announceAdvancements|blockExplosionDropDecay|commandBlockOutput|commandModificationBlockLimit|disableElytraMovementCheck|disableRaids|doDaylightCycle|doEntityDrops|doFireTick|doImmediateRespawn|doInsomnia|doLimitedCrafting|doMobLoot|doMobSpawning|doPatrolSpawning|doTileDrops|doTraderSpawning|doVinesSpread|doWardenSpawning|doWeatherCycle|drowningDamage|fallDamage|fireDamage|forgiveDeadPlayers|freezeDamage|globalSoundEvents|keepInventory|lavaSourceConversion|logAdminCommands|maxCommandChainLength|maxEntityCramming|mobExplosionDropDecay|mobGriefing|naturalRegeneration|playersSleepingPercentage|randomTickSpeed|reducedDebugInfo|sendCommandFeedback|showDeathMessages|snowAccumulationHeight|spawnRadius|spectatorsGenerateChunks|tntExplosionDropDecay|universalAnger|universal_graves:breaking_time|universal_graves:protection_time|waterSourceConversion)/give <targets> <item> [<count>]/help [<command>]/item (replace|modify)/kick <targets> [<reason>]/kill [<targets>]/list [uuids]/locate (structure|biome|poi)/loot (replace|insert|give|spawn)/msg <targets> <message>/tell -> msg/w -> msg/particle <name> [<pos>]/place (feature|jigsaw|structure|template)/playsound <sound> (master|music|record|weather|block|hostile|neutral|player|ambient|voice)/reload/recipe (give|take)/return <value>/ride <target> (mount|dismount)/say <message>/schedule (function|clear)/scoreboard (objectives|players)/seed/setblock <pos> <block> [destroy|keep|replace]/spawnpoint [<targets>]/setworldspawn [<pos>]/spectate [<target>]/spreadplayers <center> <spreadDistance> <maxRange> (<respectTeams>|under)/stopsound <targets> [*|master|music|record|weather|block|hostile|neutral|player|ambient|voice]/summon <entity> [<pos>]/tag <targets> (add|remove|list)/team (list|add|remove|empty|join|leave|modify)/teammsg <message>/tm -> teammsg/teleport (<location>|<destination>|<targets>)/tp -> teleport/tellraw <targets> <message>/time (set|add|query)/title <targets> (clear|reset|title|subtitle|actionbar|times)/trigger <objective> [add|set]/weather (clear|rain|thunder)/worldborder (add|set|center|damage|get|warning)/jfr (start|stop)/ban-ip <target> [<reason>]/banlist [ips|players]/ban <targets> [<reason>]/deop <targets>/op <targets>/pardon <targets>/pardon-ip <target>/perf (start|stop)/save-all [flush]/save-off/save-on/setidletimeout <minutes>/stop/whitelist (on|off|list|add|remove|reload)/badge (grant|revoke|list|clear)/connectivity (packetsSummary|packetsAllPlayers|packetsPlayer|packetsClient)/invsort (sort|sortme|downloadBlacklist|blacklist|sortType|middleClickSort|doubleClickSort)/graves [modify|player|about|reload]/replacenear [<args>]//replacenear [<args>]/.s [<args>]/lrbuild [<args>]//lrbuild [<args>]//stack [<args>]//overlay [<args>]//shift [<args>]/searchitem [<args>]//searchitem [<args>]//l [<args>]//search [<args>]/toggleeditwand [<args>]/none [<args>]/gmask [<args>]//gmask [<args>]/selwand [<args>]//selwand [<args>]/removeabove [<args>]//removeabove [<args>]/pumpkins [<args>]//outset [<args>]/navwand [<args>]//navwand [<args>]//generate [<args>]//gen [<args>]//g [<args>]//replace [<args>]//re [<args>]//rep [<args>]//faces [<args>]//outline [<args>]//deform [<args>]/biomelist [<args>]/biomels [<args>]/green [<args>]//green [<args>]/extinguish [<args>]//ex [<args>]//ext [<args>]//extinguish [<args>]/ex [<args>]/ext [<args>]/unstuck [<args>]/! [<ar

Note that the help command response seems to be cut off with the last characters being [<ar. The last character corresponding to the 4096th byte in the response. This wouldn't be a huge deal as help is not the most critical command, but the more problematic issue is that the remaining bytes that are to be sent from the server pollute the buffer of any subsequent commands that are run.

I have a few mods installed that add to the server help command output, which I guess is why you might not have come up against this issue before. Namely, WorldEdit does seem to pollute the command palette significantly.

Anyway, would be worth implementing a batched read until there are no more bytes left in the response to return.

Let me know if I can help out in any way. I'm about to go on holiday, I might submit a pull request with a fix when I return :)

itzg commented 11 months ago

It looks looks it's the upstream library since that response is retrieved here

https://github.com/itzg/rcon-cli/blob/7f7f4f17150018dc375b3a1053f1b9d99c6eb178/cli/entry.go#L75

josef-dijon commented 11 months ago

Ahh yes, looking closer at https://developer.valvesoftware.com/wiki/Source_RCON_Protocol#Multiple-packet_Responses it does look like the RCON spec. limits responses to 4096 byte.

However, it does also look like it supports multi packet chunking of responses larger than 4096, which does seem to be supported by james4k's rcon lib: https://github.com/james4k/rcon/blob/34a67ca2b2d65052cacddc7a1415b568f8b932a1/rcon_test.go#L77C1-L78C1

My go knowledge is pretty much non-existent though, so I could be way off track here.

itzg commented 11 months ago

Good find; however, I'm not sure how to proceed given that unit test. It asserts it read the response sizes it expected, but in our case how would we know there are more to read? Just read until the response is an empty string?

josef-dijon commented 11 months ago

That's a pretty typical pattern, like as if you were reading from a socket, although that's not a guarantee that that's how james4k/rcon is intended to be used I guess. I'll need to have a look further.