feat: Support encoding/emitting nested structures with binary fields #1
Conversation
I've pulled in jsoniter to support passing a context into a json serialization, so we can stop walking and rewriting the entire passed structure to replace with placeholders. Instead, for the registered types, we replace them with placeholders inline as they're serialized, writing their attachments to a stream-context-passed attachments list, so it's multiple-simultaneous-encoding-safe.
|
Thanks for your PR, I need to review this request. |
|
|
||
| jsoniter "github.com/json-iterator/go" |
There was a problem hiding this comment.
The last update to the standard library was two years ago, and it seems to be out of maintenance.
There was a problem hiding this comment.
Yeah, I don't love this fact, but it was the only high-performance json library I could find that supported passing context through each individual serialization versus having to do some horrible thing with finding pointers to all arrays in a first pass, storing them in a global lookup, and then using that to back-calculate the "current stream" from inside the custom marshal functions. It's well-regarded across the internet though, has 13k stars, etc., so I'm not that worried, but your call about how worried you are about this.
There was a problem hiding this comment.
For a rapidly developing network, stopping updates means many security risks.
parser/is-binary.go
Outdated
| return false | ||
| } | ||
| dv := reflect.ValueOf(data) | ||
| if dv.Kind() == reflect.Struct { |
There was a problem hiding this comment.
Maybe consider adding reflect.Ptr support.
There was a problem hiding this comment.
Fair enough! I wasn't sure how well updated this library was and whether you'd like the PR at all since it feels a little hacky to me, so wanted to get something basic out first. :D Will add this...
There was a problem hiding this comment.
Well, given this feature, we can even do more, such as support for Array and Map.
parser/encoder.go
Outdated
| @@ -79,8 +79,13 @@ func (e *encoder) encodeAsString(packet *Packet) types.BufferInterface { | |||
| } | |||
| // json data | |||
| if nil != packet.Data { | |||
| if b, err := json.Marshal(_encodeData(packet.Data)); err == nil { | |||
| str.Write(b) | |||
| if pds, is := packet.Data.(string); is { | |||
There was a problem hiding this comment.
This behavior seems inconsistent with nodejs socket.io-parser, which will cause unpredictable problems in other clients:
test golang:
package main
import (
"fmt"
"github.com/zishang520/socket.io-go-parser/v2/parser"
)
func main() {
en := parser.NewEncoder()
id := uint64(1)
Attachments := uint64(1)
fmt.Println(en.Encode(&parser.Packet{
Type: parser.EVENT,
Nsp: "/a",
Data: "any",
Id: &id,
Attachments: &Attachments,
}))
}output: [2/a,1any]
test nodejs (latest):
const {
Encoder,
PacketType
} = require("socket.io-parser");
let en = new Encoder;
console.log(en.encode({
type: PacketType.EVENT,
nsp: "/a",
data: "any",
id: 1,
attachments: 1,
}))
output: [ '2/a,1"any"' ]
There was a problem hiding this comment.
Ah, wasn't aware you could use it that way. Adding a private field to store it instad...
parser/binary.go
Outdated
| stream.Attachment = append(bufList, bb) | ||
| }, nil) | ||
|
|
||
| jsoniter.RegisterTypeEncoderFunc("[]byte", func(ptr unsafe.Pointer, stream *jsoniter.Stream) { |
There was a problem hiding this comment.
[]byte doesn't seem to be working as expected, maybe there's something wrong with my testing?
package main
import (
"fmt"
"github.com/zishang520/socket.io-go-parser/v2/parser"
)
func main() {
en := parser.NewEncoder()
id := uint64(1)
Attachments := uint64(1)
fmt.Println(en.Encode(&parser.Packet{
Type: parser.EVENT,
Nsp: "/a",
Data: []any{"any", []byte{1, 2, 3}},
Id: &id,
Attachments: &Attachments,
}))
}need: [51-/a,1["any",{"_placeholder":true,"num":0}] ╔╗╚]
output: [50-/a,1["any","AQID"]]
|
I plan to improve the unit testing in the short term (: |
| @@ -79,9 +79,9 @@ func (e *encoder) encodeAsString(packet *Packet) types.BufferInterface { | |||
| } | |||
| // json data | |||
| if nil != packet.Data { | |||
| if pds, is := packet.Data.(string); is { | |||
| if packet.preSerializedData != "" { | |||
There was a problem hiding this comment.
I need to run more test cases to ensure that this request is OK. It may take longer.
There was a problem hiding this comment.
There's no rush. I've already taken the encoder from this branch and I'm using it in my project that way via the config.Encoder.
|
BTW, just to be clear, feel free to hack this PR up. I'm not attached to this implementation, it just worked for my use case and looked reasonably generalized. :) |
|
Due to the reason of |
I've pulled in jsoniter to support passing a context into a json serialization, so we can stop walking and rewriting the entire passed structure to replace with placeholders. Instead, for the registered types, we replace them with placeholders inline as they're serialized, writing their attachments to a stream-context-passed attachments list, so it's multiple-simultaneous-encoding-safe. Now you can use structures instead of typeless maps for passing arraybuffers.
Example: