Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typing in chat causes client to disconnect #24

Closed
bananasov opened this issue Sep 5, 2022 · 8 comments · Fixed by #30
Closed

Typing in chat causes client to disconnect #24

bananasov opened this issue Sep 5, 2022 · 8 comments · Fixed by #30
Labels
good first issue Good for newcomers

Comments

@bananasov
Copy link

Using commit cac348a4b6e788f3ecce24c7559c1789f4c8b402, running example combat.

Server log

[2022-09-05T13:03:17Z TRACE valence::server] entering accept loop
[2022-09-05T13:04:22Z ERROR valence::server] connection to 127.0.0.1:64121 ended: error during play: packet contents were not read completely

How to reproduce

  • Join the server
  • Type a message
  • Get disconnected

This bug occurs in all examples and non-examples.

@rj00a
Copy link
Member

rj00a commented Sep 5, 2022

This is an issue because serverbound chat is not implemented (see #8).

At the very least the packet should be defined correctly so the clients are not disconnected. I'll leave this issue open until that is fixed.

@rj00a rj00a added the good first issue Good for newcomers label Sep 5, 2022
@dyc3

This comment was marked as outdated.

dyc3 added a commit to dyc3/valence that referenced this issue Sep 6, 2022
@XBagon
Copy link
Contributor

XBagon commented Sep 6, 2022

At the very least the packet should be defined correctly so the clients are not disconnected. I'll leave this issue open until that is fixed.

I'm having problems finding the wrongly/not defined packet? It just seems to have 2 additional 0x0 bytes and I can't figure out why. Using this.

@dyc3
Copy link
Collaborator

dyc3 commented Sep 6, 2022

I logged out the decompressed bytes and the parsed packet, and here's what I got

[2022-09-06T22:10:49Z TRACE valence::protocol::codec] decompressed: [5, 4, 116, 101, 115, 116, 0, 0, 1, 131, 20, 219, 36, 224, 163, 58, 17, 30, 198, 29, 85, 35, 128, 2, 132, 226, 255, 248, 83, 63, 9, 114, 115, 179, 171, 65, 138, 224, 100, 85, 36, 232, 47, 90, 15, 87, 225, 250, 162, 208, 244, 241, 18, 239, 88, 37, 50, 72, 126, 96, 98, 114, 44, 108, 127, 251, 80, 108, 157, 30, 249, 188, 74, 99, 16, 212, 248, 123, 13, 71, 61, 17, 226, 222, 44, 48, 35, 80, 137, 201, 119, 236, 164, 106, 132, 196, 156, 18, 67, 159, 17, 85, 29, 170, 220, 84, 250, 206, 224, 16, 248, 34, 155, 124, 134, 175, 38, 249, 60, 235, 86, 93, 62, 236, 111, 55, 44, 209, 27, 107, 68, 126, 148, 239, 179, 43, 110, 59, 72, 70, 211, 91, 16, 179, 27, 113, 38, 215, 14, 135, 99, 137, 43, 161, 70, 17, 197, 178, 155, 40, 173, 85, 12, 39, 131, 97, 45, 114, 67, 181, 129, 135, 177, 48, 240, 114, 14, 214, 218, 169, 78, 208, 150, 255, 164, 97, 62, 206, 32, 42, 26, 40, 206, 37, 17, 54, 194, 60, 96, 65, 48, 50, 89, 77, 96, 108, 38, 92, 3, 1, 117, 131, 116, 64, 211, 18, 6, 220, 143, 113, 241, 148, 22, 180, 225, 164, 60, 37, 25, 13, 212, 210, 198, 7, 82, 18, 237, 73, 98, 98, 61, 89, 146, 79, 129, 141, 27, 122, 4, 161, 174, 208, 100, 152, 147, 226, 166, 147, 0, 58, 155, 23, 227, 191, 81, 49, 68, 40, 131, 182, 126, 15, 251, 101, 133, 127, 13, 56, 107, 243, 0, 0, 0]
[2022-09-06T22:10:49Z TRACE valence::protocol::codec] packet: C2sPlayPacket(ChatMessage { message: BoundedString("test"), timestamp: 1662502249696, salt: 11761732200701973795, signature: [132, 226, 255, 248, 83, 63, 9, 114, 115, 179, 171, 65, 138, 224, 100, 85, 36, 232, 47, 90, 15, 87, 225, 250, 162, 208, 244, 241, 18, 239, 88, 37, 50, 72, 126, 96, 98, 114, 44, 108, 127, 251, 80, 108, 157, 30, 249, 188, 74, 99, 16, 212, 248, 123, 13, 71, 61, 17, 226, 222, 44, 48, 35, 80, 137, 201, 119, 236, 164, 106, 132, 196, 156, 18, 67, 159, 17, 85, 29, 170, 220, 84, 250, 206, 224, 16, 248, 34, 155, 124, 134, 175, 38, 249, 60, 235, 86, 93, 62, 236, 111, 55, 44, 209, 27, 107, 68, 126, 148, 239, 179, 43, 110, 59, 72, 70, 211, 91, 16, 179, 27, 113, 38, 215, 14, 135, 99, 137, 43, 161, 70, 17, 197, 178, 155, 40, 173, 85, 12, 39, 131, 97, 45, 114, 67, 181, 129, 135, 177, 48, 240, 114, 14, 214, 218, 169, 78, 208, 150, 255, 164, 97, 62, 206, 32, 42, 26, 40, 206, 37, 17, 54, 194, 60, 96, 65, 48, 50, 89, 77, 96, 108, 38, 92, 3, 1, 117, 131, 116, 64, 211, 18, 6, 220, 143, 113, 241, 148, 22, 180, 225, 164, 60, 37, 25, 13, 212, 210, 198, 7, 82, 18, 237, 73, 98, 98, 61, 89, 146, 79, 129, 141, 27, 122, 4, 161, 174, 208, 100, 152, 147, 226, 166, 147, 0, 58, 155, 23, 227, 191, 81, 49, 68, 40, 131, 182, 126, 15, 251, 101, 133, 127, 13, 56, 107, 243], signed_preview: false })

Notice that the signature field occurs 2nd to last, but theres a remaining 2 0x0 bytes at the end of the packet. I think it might just be padding from the compression, which would mean it's safe to ignore, or extra padding for the signature field, which needs to be appended to the signature. For the chat message packet specifically, it seems like its consistently 2 bytes remaining after parsing.

@rj00a
Copy link
Member

rj00a commented Sep 7, 2022

I'm having problems finding the wrongly/not defined packet? It just seems to have 2 additional 0x0 bytes and I can't figure out why. Using this.

The packet definition on wiki.vg is likely outdated.

@alice39
Copy link

alice39 commented Sep 7, 2022

I found that those 2 remaining bytes is related with last signed messages according to Velocity Player Chat Decoding for versions >=1.19.1

@dyc3
Copy link
Collaborator

dyc3 commented Sep 7, 2022

Here is a screenshot from fabric's enigma:

image

Working on figuring out what exactly this "Acknowledgement" type is. Good to know that this was introduced >= 1.19.1

@dyc3
Copy link
Collaborator

dyc3 commented Sep 7, 2022

This is the decompiled code for that Acknowledgement type.

/**
     * A record of messages acknowledged by a client.
     * 
     * <p>This holds the messages the client has recently seen, as well as the last
     * message they received, if any.
     */
    public record LastSeenMessageList.Acknowledgment(LastSeenMessageList lastSeen, Optional<LastSeenMessageList.Entry> lastReceived) {
        public LastSeenMessageList.Acknowledgment(PacketByteBuf buf) {
            this(new LastSeenMessageList(buf), buf.readOptional(LastSeenMessageList.Entry::LastSeenMessageList.Entry));
        }

        public void write(PacketByteBuf buf) {
            this.lastSeen.write(buf);
            buf.writeOptional(this.lastReceived, (buf2, lastReceived) -> lastReceived.write((PacketByteBuf)((Object)buf2)));
        }
    }

So the last 2 bytes we are seeing is a list of entries, and an optional last seen entry. Putting up a PR now.

dyc3 added a commit to dyc3/valence that referenced this issue Sep 7, 2022
dyc3 added a commit to dyc3/valence that referenced this issue Sep 7, 2022
dyc3 added a commit to dyc3/valence that referenced this issue Sep 7, 2022
rj00a pushed a commit that referenced this issue Sep 7, 2022
* Update ChatMessage definition, prevent typing in chat from kicking players

fixes #24

* Update definition for CommandExecution
@rj00a rj00a closed this as completed in #30 Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
5 participants