accepts broken OPENBATTLE messages

[cleanrock]

I see a few BATTLEOPENED messages which dont have the required modName-sentence at end:

failed to process server msg:'BATTLEOPENED 7265 0 0 [GJL]autohost 94.23.171.71 8499 16 0 0 0 Small_Divide-Remake-v04 cursed autohost ' (extractSentence failed)
failed to process server msg:'BATTLEOPENED 7264 0 0 PlanetWars 94.23.171.71 8496 32 1 0 -373207118 FolsomDamFinal PlanetWars online campaign ' (extractSentence failed)
failed to process server msg:'BATTLEOPENED 7267 0 0 Bromine 94.23.171.71 8498 16 0 0 0 Small_Divide-Remake-v04Conflict Terra (test) Autohost ' (extractSentence failed)
failed to process server msg:'BATTLEOPENED 7276 0 0 Oxygen1 94.23.171.71 8507 16 1 0 0 Small_Divide-Remake-v04Latest test version PASSWORD:test ' (extractSentence failed)

I suspect this is because the hosts dont send the required modName (or perhaps they have a newline before the modName).
After looking at the uberserver code i think it does not catch these bad messages:

if sentence_args.count('\t') > 1:
map, title, modname = sentence_args.split('\t',2)

modname is probably an empty string after this.

[lunixbochs]

what are you using to parse the netcode?

Ryan Hileman

On Tuesday, February 7, 2012 at 8:56 AM, cleanrock wrote:

I see a few BATTLEOPENED messages which dont have the required modName-sentence at end:

failed to process server msg:'BATTLEOPENED 7265 0 0 [GJL]autohost 94.23.171.71 8499 16 0 0 0 Small_Divide-Remake-v04 cursed autohost ' (extractSentence failed)
failed to process server msg:'BATTLEOPENED 7264 0 0 PlanetWars 94.23.171.71 8496 32 1 0 -373207118 FolsomDamFinal PlanetWars online campaign ' (extractSentence failed)
failed to process server msg:'BATTLEOPENED 7267 0 0 Bromine 94.23.171.71 8498 16 0 0 0 Small_Divide-Remake-v04Conflict Terra (test) Autohost ' (extractSentence failed)
failed to process server msg:'BATTLEOPENED 7276 0 0 Oxygen1 94.23.171.71 8507 16 1 0 0 Small_Divide-Remake-v04Latest test version PASSWORD:test ' (extractSentence failed)

I suspect this is because the hosts dont send the required modName (or perhaps they have a newline before the modName).
After looking at the uberserver code i think it does not catch these bad messages:

if sentence_args.count('\t') > 1:
map, title, modname = sentence_args.split('\t',2)

modname is probably an empty string after this.

---

Reply to this email directly or view it on GitHub:
#7

[cleanrock]

My own lobby.
Btw, i am pretty sure there are not embedded newlines in message because my code would print an error on next message in that case.
Do you agree uberserver accepts incomplete BATTLEOPENED messages ?
To me it looks like that and i think the hole should be plugged.

[lunixbochs]

if you split that on tab, you'll get an empty string for the modname. I don't see how that's a problem unless your string parsing lib doesn't behave well

Ryan Hileman

On Saturday, February 11, 2012 at 5:30 AM, cleanrock wrote:

My own lobby.
Btw, i am pretty sure there are not embedded newlines in message because my code would print an error on next message in that case.
Do you agree uberserver accepts incomplete BATTLEOPENED messages ?
To me it looks like that and i think the hole should be plugged.

---

Reply to this email directly or view it on GitHub:
#7 (comment)

[cleanrock]

I cant see how OPENBATTLE messages with empty modName makes any sense, i think uberserver should send OPENBATTLEFAILED in that case.
Correct me if i am wrong here, i.e. having an empty modName in OPENBATTLE is useful.

[shaun0x00]

I have the same problem with my unreleased lobby. It is behaving well because it is notifying me that a non-optional parameter is missing. It discards the server message because it is not following specification which, unless proven otherwise, is the right thing to do.

The protocol spec is clear on this, modname is not optional. A token with 0 word is not a sentence, so the sentence is missing and since it is not optional, it is wrong.

Relevant parts of the spec:

{} = sentence = several words (at least one!) separated by space characters

Please dont ask us to make sloppy client implementations.
Changing the specification to make this field optional or fixing the server to make it specification compliant are the only sensible options here.

[lunixbochs]

You need to be able to handle one of the sentence args being empty (because it's feasible for the description to be blank, for example). The spec does not specify a minimum word length inside sentences, only the word count. I don't see anything wrong with allowing empty strings at a protocol level from an abstract perspective.

However - as it makes no sense in the context of Spring to have a zero-length mod or map name (which you should be able to parse with your sentence parser, nothing to do with the specific command), I've added a check for non-empty that'll throw OPENBATTLEFAILED otherwise.

[cleanrock]

I saw this bad BATTLEOPENED message again, when investigating i noticed the function in_OPENBATTLEEX which is not fixed and i guess this msg was used.
'BATTLEOPENED 3273 0 0 Nickel1 94.23.171.71 8475 16 1 0 0 Small_Divide-Remake-v04 my game '
The last char after "my game" is a tab.

[lunixbochs]

thanks.

also, licho said he fixed this in springie :D so apparently he didn't.