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.

Sign up for GitHub

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

Jump to bottom

feat: Support encoding/emitting nested structures with binary fields #1

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+101 −52
Open

feat: Support encoding/emitting nested structures with binary fields #1

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4597211
feat: Support encoding/emitting nested structures with binary fields
deregtd Jul 21, 2024
111a35d
PR feedback
deregtd Jul 22, 2024
005578a
forgot to commit this
deregtd Jul 22, 2024
379e950
fixed byte arrays and arrays-that-are-binary detection
deregtd Jul 22, 2024
b80f54d
Merge branch 'main' into ddr/jsoniter-encoder
deregtd Jul 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ require (
github.com/google/pprof v0.0.0-20230821062121-407c9e7a662f // indirect
github.com/gookit/color v1.5.4 // indirect
github.com/gorilla/websocket v1.5.1 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421 // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/onsi/ginkgo/v2 v2.12.0 // indirect
github.com/quic-go/qpack v0.4.0 // indirect
github.com/quic-go/quic-go v0.44.0 // indirect
Expand Down
8 changes: 8 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,21 @@ github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg
github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
github.com/google/pprof v0.0.0-20230821062121-407c9e7a662f h1:pDhu5sgp8yJlEF/g6osliIIpF9K4F5jvkULXa4daRDQ=
github.com/google/pprof v0.0.0-20230821062121-407c9e7a662f/go.mod h1:czg5+yv1E0ZGTi6S6vVK1mke0fV+FaUhNGcd6VRS9Ik=
github.com/gookit/color v1.5.4 h1:FZmqs7XOyGgCAxmWyPslpiok1k05wmY3SJTytgvYFs0=
github.com/gookit/color v1.5.4/go.mod h1:pZJOeOS8DM43rXbp4AZo1n9zCU2qjpcRko0b6/QJi9w=
github.com/gorilla/websocket v1.5.1 h1:gmztn0JnHVt9JZquRuzLw3g4wouNVzKL15iLr/zn/QY=
github.com/gorilla/websocket v1.5.1/go.mod h1:x3kM2JMyaluk02fnUJpQuwD2dCS5NDG2ZHL0uE0tcaY=
github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM=
github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo=
github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY=
github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421 h1:ZqeYNhU3OHLH3mGKHDcjJRFFRrJa6eAM5H+CtDdOsPc=
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9Gz0M=
github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
github.com/onsi/ginkgo/v2 v2.12.0 h1:UIVDowFPwpg6yMUpPjGkYvf06K3RAiJXUhCxEwQVHRI=
github.com/onsi/ginkgo/v2 v2.12.0/go.mod h1:ZNEzXISYlqpb8S36iN71ifqLi3vVD1rVJGvWRCJOUpQ=
github.com/onsi/gomega v1.27.10 h1:naR28SdDFlqrG6kScpT8VWpu1xWY5nJRCF3XaYyBjhI=
Expand All @@ -32,6 +39,7 @@ github.com/quic-go/quic-go v0.44.0/go.mod h1:z4cx/9Ny9UtGITIPzmPTXh1ULfOyWh4qGQl
github.com/quic-go/webtransport-go v0.8.0 h1:HxSrwun11U+LlmwpgM1kEqIqH90IT4N8auv/cD7QFJg=
github.com/quic-go/webtransport-go v0.8.0/go.mod h1:N99tjprW432Ut5ONql/aUhSLT0YVSlwHohQsuac9WaM=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
Expand Down
76 changes: 35 additions & 41 deletions parser/binary.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package parser

import (
"bytes"
"errors"
"io"
"unsafe"

jsoniter "github.com/json-iterator/go"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last update to the standard library was two years ago, and it seems to be out of maintenance.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a rapidly developing network, stopping updates means many security risks.

"github.com/mitchellh/mapstructure"
"github.com/zishang520/engine.io-go-parser/types"
)
Expand All @@ -13,54 +15,46 @@ type Placeholder struct {
Num int `json:"num" mapstructure:"num" msgpack:"num"`
}

func init() {
jsoniter.RegisterTypeEncoderFunc("types.BytesBuffer", func(ptr unsafe.Pointer, stream *jsoniter.Stream) {
bb := ((*types.BytesBuffer)(ptr))

bufList := stream.Attachment.([]types.BufferInterface)
_placeholder := &Placeholder{Placeholder: true, Num: len(bufList)}
stream.WriteVal(_placeholder)
stream.Attachment = append(bufList, bb)
}, nil)

jsoniter.RegisterTypeEncoderFunc("[]byte", func(ptr unsafe.Pointer, stream *jsoniter.Stream) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[]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"]]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kk fixed this up

bb := types.NewBytesBuffer(nil)
barr := ((*[]byte)(ptr))
bb.Write(*barr)

bufList := stream.Attachment.([]types.BufferInterface)
_placeholder := &Placeholder{Placeholder: true, Num: len(bufList)}
stream.WriteVal(_placeholder)
stream.Attachment = append(bufList, bb)
}, nil)
}

// Replaces every io.Reader | []byte in packet with a numbered placeholder.
func DeconstructPacket(packet *Packet) (pack *Packet, buffers []types.BufferInterface) {
pack = packet
pack.Data = _deconstructPacket(packet.Data, &buffers)

// Run the serialization now, replacing any bytebuffers/[]byte found along the way with placeholders
buf := &bytes.Buffer{}
ns := jsoniter.NewStream(jsoniter.ConfigDefault, buf, buf.Cap())
ns.Attachment = buffers
ns.WriteVal(pack.Data)
buffers = ns.Attachment.([]types.BufferInterface)
ns.Flush()
pack.Data = buf.String()

attachments := uint64(len(buffers))
pack.Attachments = &attachments // number of binary 'attachments'
return pack, buffers
}

func _deconstructPacket(data any, buffers *[]types.BufferInterface) any {
if data == nil {
return nil
}

if IsBinary(data) {
_placeholder := &Placeholder{Placeholder: true, Num: len(*buffers)}
rdata := types.NewBytesBuffer(nil)
switch tdata := data.(type) {
case io.Reader:
if c, ok := data.(io.Closer); ok {
defer c.Close()
}
rdata.ReadFrom(tdata)
case []byte:
rdata.Write(tdata)
}
*buffers = append(*buffers, rdata)
return _placeholder
}

switch tdata := data.(type) {
case []any:
newData := make([]any, 0, len(tdata))
for _, v := range tdata {
newData = append(newData, _deconstructPacket(v, buffers))
}
return newData
case map[string]any:
newData := map[string]any{}
for k, v := range tdata {
newData[k] = _deconstructPacket(v, buffers)
}
return newData
default:
return data
}
}

// Reconstructs a binary packet from its placeholder packet and buffers
func ReconstructPacket(packet *Packet, buffers []types.BufferInterface) (*Packet, error) {
data, err := _reconstructPacket(packet.Data, &buffers)
Expand Down
13 changes: 9 additions & 4 deletions parser/encoder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ func _encodeData(data any) any {
newData[k] = _encodeData(v)
}
return newData
default:
return data
}

return data
}

// Encode packet as string.
Expand All @@ -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 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"' ]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, wasn't aware you could use it that way. Adding a private field to store it instad...

// Already serialized in the DeconstructPacket function
str.WriteString(pds)
} else {
if b, err := json.Marshal(_encodeData(packet.Data)); err == nil {
str.Write(b)
}
}
}
parser_log.Debug("encoded %v as %v", packet, str)
Expand Down
14 changes: 14 additions & 0 deletions parser/is-binary.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package parser

import (
"io"
"reflect"
"strings"

"github.com/zishang520/engine.io-go-parser/types"
Expand Down Expand Up @@ -30,12 +31,25 @@ func HasBinary(data any) bool {
return true
}
}
return false
case map[string]any:
for _, v := range o {
if HasBinary(v) {
return true
}
}
return false
}
dv := reflect.ValueOf(data)
if dv.Kind() == reflect.Struct {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe consider adding reflect.Ptr support.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, given this feature, we can even do more, such as support for Array and Map.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

for fi := range dv.NumField() {
dfv := dv.Field(fi)
if dfv.CanInterface() && HasBinary(dfv.Interface()) {
return true
}
}
return false
}

return IsBinary(data)
}
Loading