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

Refactor socketioxide parsing #376

Merged
merged 87 commits into from
Oct 20, 2024
Merged

Refactor socketioxide parsing #376

merged 87 commits into from
Oct 20, 2024

Conversation

Totodore
Copy link
Owner

@Totodore Totodore commented Sep 29, 2024

Motivation

Currently, the serialization/deserialization system of socketioxide works as follows:

Deserialization

  • First, the packet is parsed, and a payload is extracted.
  • This payload ([event, ...data]) is immediately parsed into a dynamic value (serde_json::Value). We do this to:
    • Read the event string and route it to the correct handler.
    • If the data consists of a variadic number of arguments, convert it into a [serde_json::Value::Array] and skip the first element (the event).
    • If the data is a single value, extract it from the array.
  • The payload is then deserialized into the user-provided type before calling the handler.

Serialization

  • The data provided by the user is serialized into a serde_json::Value.
  • If the data is an array, we insert the event name at the front of the array (shifting all the elements).
  • If the data is a single value, we wrap the provided value in an array with the event at the front.

Drawbacks

This approach is the simplest way to handle the data payload, but it has several drawbacks:

Solution

First, the codebase is split into multiple crates:

  • The socketioxide_core crate contains all the core types used by the other crates (mainly the parser implementations): Packet, Parse, Sid, Str.
  • The parser_common crate contains all the parsing code for the default parser.
  • The parser_msgpack crate contains all the parsing code for the new msgpack parser.
  • The socketioxide crate contains the rest of the codebase.

The core crate defines a Parse trait that all parsers must implement. The Parse design focuses on deferred parsing and keeping things as simple as possible. Here is the new serde flow with this solution:

Deserialization

  • The packet is parsed, and the entire payload is retained as-is (no deserialization).
  • When socketioxide needs to call the correct handler, it calls the read_event method to simply retrieve a string reference to the event in the payload.
  • Using a custom serde implementation, we check if the user-provided value is a tuple based on the serde model. If it is, we deserialize it directly. Otherwise, we deserialize the first element of the array (solves Emitting with an array/vector data incorrectly serialized as separate arguments. #225).
  • The payload is then deserialized into the user-provided value. Both parser implementations (common and msgpack) contain custom implementations of serde::{Serialize, Deserialize}, which wrap serde_json and rmp_serde. These wrappers handle the following:

Serialization

Drawbacks of this solution

  • For the common parser, binary payloads are currently cloned during both serialization and deserialization because the serde model uses Vec<u8> rather than Bytes or other structs that are cheap to clone. This is the primary issue with the current solution, though it only affects serde_json; msgpack handles binary data natively.
  • It is not possible to deserialize/serialize an unknown variadic number of arguments. You must know the number of arguments you are sending and receiving, whereas in JavaScript you can emit any unknown number of arguments (e.g., socket.emit(...new Array(Math.random(1000)))).

To Do

  • Add documentation
  • Test and document behavior for using serde_json::Value when deserializing payloads with binaries
  • Add CI/CD pipelines for msgpack parser
  • Add more unit testing for parsers (read_event, ...)
  • Add fuzzy testing ?
  • Add benchmarks for the msgpack parser.
  • Add a feature flag for msgpack parser

Closes :

crates/socketioxide-core/src/parser.rs Fixed Show fixed Hide fixed
crates/socketioxide-core/src/parser.rs Fixed Show fixed Hide fixed
crates/socketioxide-core/src/parser.rs Fixed Show fixed Hide fixed
crates/socketioxide-core/src/parser.rs Fixed Show fixed Hide fixed
@Totodore Totodore merged commit c5e8ae2 into main Oct 20, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
1 participant