socketioxide
socketioxide copied to clipboard
Emitting with an array/vector data incorrectly serialized as separate arguments.
Describe the bug As described in title, emitting an event with an array or vector like:
let v = vec![1,2,3];
socket.emit("message", v).ok();
will split elements in the vector and send them as separate arguments. Clients receive that many arguments instead of the intact array:
To Reproduce Run a server using socketioxide in rust and start a Javascript or python client. Emitting an event from server with an array/vector payload.
socket.on("message", (arg)=>{
// will print 1
console.log(arg);
// error
console.log(arg[1]);
})
socket.on("message", (...arg)=>{
// will print
// 1
// 2
// 3
for (const a of arg) {
console.log(a);
}
})
Expected behavior The expected resulte would be:
socket.on("message", (arg)=>{
// will print [1,2,3]
console.log(arg);
// will print 2
console.log(arg[1]);
})
Same behavior for tuple and array.
Versions (please complete the following information):
- Socketioxide version: 0.10.0
- Http lib: axum 0.7
- Socket.io client version js v4.7.2
Additional context If I wrap the array/vector with an additional array, the results will be fine:
let v = vec![1,2,3];
socket.emit("message", [v]).ok();
That is an interesting find out.
Unfortunately this is the only way I found to handle multi arguments.
When multi arguments are received they are "compacted" into an array and to send multi arguments you just provide an array or tuple. The best behaviour may be to have tuples for multi-arguments and keep arrays "as-is". But it is impossible to differentiate them when everything is serialized to a serde_json::Value.
We should document that case though.
Unfortunately this is the only way I found to handle multi arguments. When multi arguments are received they are "compacted" into an array and to send multi arguments you just provide an array or tuple. The best behaviour may be to have tuples for multi-arguments and keep arrays "as-is". But it is impossible to differentiate them when everything is serialized to a
serde_json::Value.We should document that case though.
Could we just check for vector/tuple payloads and wrap them before sending in the crate?
serde_json::Value doesn't make the difference (in both case it is a Value::Array variant) and we must support multi arguments to have full compatibility. Solving this issue would require to be able to detect if we provided a tuple or an array either at compile time or at runtime.
We could use a macro fn to check that but then it would not be a method of the socket struct.
Perhaps it would make sense to always require a tuple in emit()? You would have to write socket.emit("single", (value,)) to emit a single argument, which is less pretty, but it's more clear and cannot give you an unexpected result. I already write all my emits like that because it already works today 😛 The only problem I see with it is that a variable number of arguments would not be supported, but that could be in a separate socket.emit_variable() method, as it's hopefully uncommon.
Perhaps it would make sense to always require a tuple in
emit()? You would have to writesocket.emit("single", (value,))to emit a single argument, which is less pretty, but it's more clear and cannot give you an unexpected result. I already write all my emits like that because it already works today 😛 The only problem I see with it is that a variable number of arguments would not be supported, but that could be in a separatesocket.emit_variable()method, as it's hopefully uncommon.
I don't really want to force people to put tuples inside socket.emit for the reasons you gave. However adding a second emit_array or emit_variable (I don't have a precise name for the moment) would be a good idea to support the case of @NessajCN without having to do socket.emit([array]). Internally it would be just an alias of socket.emit([arg]).
If someone want to work on this don't hesitate :). Things to do are :
- Document this special case in the
socket.emitfn and in theEmitting datacategory in the main lib.rs doc. - Add a
emit_arrayfn inSocketandOperatorswith an explanation on why this is needed and the difference withsocket.emit - Update examples where the syntax
socket.emit([args])is used.
https://github.com/Totodore/socketioxide/blob/d890ba4040efe5b0376d9a68a20fc20733d96cc8/socketioxide/src/packet.rs#L313-L315
Value::Array(v) if !v.is_empty() => serde_json::to_string(&vec![
(*e).to_string(),
serde_json::to_string(v).unwrap(),
]),
Maybe we can modify it like this.
https://github.com/Totodore/socketioxide/blob/d890ba4040efe5b0376d9a68a20fc20733d96cc8/socketioxide/src/packet.rs#L313-L315
Value::Array(v) if !v.is_empty() => serde_json::to_string(&vec![ (*e).to_string(), serde_json::to_string(v).unwrap(), ]),Maybe we can modify it like this.
There are two issues:
- The inner
serde_json::to_string(v)would be escaped and therefore the result woul be [string, string] and not [string, ...T]. - It would be impossible to do multi-argument emission.
Value::Array(v) if !v.is_empty() => {
serde_json::to_string(&json!([(*e).to_string(), v]))
}
- It would be impossible to do multi-argument emission.
Yes, there is still a need for a method that supports multiple parameters.
There is another way to solve this problem. We can add a few notes to the document.
let _ = socket
.within(data.room.clone())
.emit("message", json!([[1, 2, 3, 4]]));
let _ = socket.within(data.room).emit("message", response);
IMHO, whichever way it goes, there should be separate methods for emitting single or multiple values, and emit shouldn't do both like it does today. I think .emit(data) where data always arrives as a single argument, even if it is an array, and an .emit_multiple([1, 2, 3]) that arrives as multiple arguments would also be great. .emit(data) could just be implemented as .emit_multiple([data]).
A small issue with adding more methods is that they need to be added on 3 types and also have ack/non-ack variants. It's OK for this but if there are more variations in the future, you'd end up with lots and lots of emit_x_y_z methods.
What if someone tries to emit multiple arrays in emit_variable()? Then we have to wrap those arrays with additional [] right?
My input is, we should document these cases, and keep the only emit method. Otherwise, imo, it will create inconsistency.
If you wanted to emit two arrays as separate arguments, you'd do emit_variable((array_a, array_b)). If you want to emit an array that contains arrays, you'd do emit_variable((array_of_arrays,)) or just emit(array_of_arrays).
The current situation is inconsistent, because if you write emit(data), you never know if it will be emitted as one argument or multiple. It depends on data's serialization implementation. If you have an opaque JSON value already, and you want to emit it as a single argument, you have to do this:
let data: serde_json::Value = todo!("get a value from somewhere");
let Some(array) = data.as_array() {
socket.emit([array])?
} else {
socket.emit(data)?
}
Tbh I'd still argue for a single emit() method that takes a tuple. On second thought I think that can actually handle all cases with an IntoArguments trait. See:
// Single argument
socket.emit((1,))?;
// Multiple arguments of same type
socket.emit([1, 2, 3])?;
// Multiple arguments of different types
socket.emit((1, "2", serde_json::json!({ "wrapped": "by tuple" })))?;
// Variable number of arguments
let array = vec![0; random_size];
socket.emit(array);
// Variable number of arguments of different types
let mut array: Vec<serde_json::Value> = vec![];
array.push(1.into());
if some_condition { array.push("2".into()); }
socket.emit(array);
With a trait definition like this:
trait IntoArguments {
fn into_arguments(self) -> Vec<serde_json::Value>;
}
// Implement this for tuples of many different sizes using a macro
impl <T: Serialize> IntoArguments for (T,) {
// pseudocode implementation, would need error handling
fn into_arguments(self) -> Vec<serde_json::Value> { vec![serde_json::to_value(self)] }
}
impl <T: Serialize> IntoArguments for &[T] {
// pseudocode implementation, would need error handling
fn into_arguments(self) -> Vec<serde_json::Value> { self.iter().map(serde_json::to_value).collect() }
}
impl SocketIo {
fn emit(&self, event: &str, args: impl IntoArguments) -> // etc
}
In my opinion writing emit((value,)) is really not that bad (it's 3 extra characters that help clarity), and that would be the only thing that would need to be explicitly documented, and if you omit it you'd get a compile error.
I'm still hesitant on the direction to take to fix this issue.
Tbh I'd still argue for a single emit() method that takes a tuple. On second thought I think that can actually handle all cases with an IntoArguments trait. See:
A solution with an IntoArgument trait is a good idea but I'm not a big fan of this syntax emit((value,)).
For the moment I'm going to simply document this issue on the emit fn.