surrealdb / surrealdb.go

SurrealDB SDK for Golang
https://surrealdb.com
Apache License 2.0
231 stars 62 forks source link

Combinations of characters are wrongly transformed #62

Closed KadoBOT closed 1 year ago

KadoBOT commented 1 year ago

Hey team, there's something odd with the client. I printed the query right before passing it to the Query method to make sure this wasn't on me:

fmt.Println("Executing query: ", c.query)
resp, err := c.db.Query(c.query, nil)

Later:

Executing query:
                        CREATE club:`MyNewClub` CONTENT {
                                club_name: "MyNewClub",
                                owner: clubber:`d395c589-c8bb-4f21-b38f-705dcc805af1`,
                                settings: {
                                        is_open: false,
                                        vip_chat: false,
                                        battle_mode: false
                                }
                        };
                        RELATE clubber:`d395c589-c8bb-4f21-b38f-705dcc805af1`->owns->club:`MyNewClub` SET time.bought = time::now();
                        SELECT * FROM club:`MyNewClub` FETCH owner;

2023-04-25T06:55:37.056+0200    ERROR   club_server     club/handlers.go:181    could not create club      {"CreateClub": "", "clubName": "MyNewClub", "identityID": "d395c589-c8bb-4f21-b38f-705dcc805af1", "error": "query error: sending request failed for method 'query': There was a problem with the database: Parse error on line 11 at character 3 when parsing 'RELATE clubber:`d395c589-c8bb-4f21-b38f-705dcc805af1`->owns-Ϭlub:`MyNewClub` SET time.bought = time:'"}

I'm running v0.2.1 of the client, and the latest docker image.

The interesting part: ->owns-Ϭlub I have added a space between the > and the c, and this seems to fix the issue. So somewhere in the go client, it is transforming the >c into Ϭ.

This is the only transformation I've found so far.

KadoBOT commented 1 year ago

I've tested it outside the Go client, and it works. So it looks like it's indeed an issue with the Go library: image

KadoBOT commented 1 year ago

The issue is happening during the marshalling of the query:

func (ws *WebSocket) write(v interface{}) error {
    data, err := json.Marshal(v)
    if err != nil {
        return err
    }
    fmt.Println("data", string(data))

since it converts the > to \u003e and the next word is club, when unmarshalling, it tries to unmarshall \u003ec which probably is Ϭ.

data {"id":"dzodWgboQwt4Y5OV","method":"query","params":["\n\t\t\tCREATE club:`MyNewClub2` CONTENT {\n\t\t\t\tclub_name: \"MyNewClub2\",\n\t\t\t\towner: clubber:`d395c589-c8bb-4f21-b38f-705dcc805af1`,\n\t\t\t\tsettings: {\n\t\t\t\t\tis_open: false,\n\t\t\t\t\tvip_chat: false,\n\t\t\t\t\tbattle_mode: false\n\t\t\t\t}\n\t\t\t};\n\t\t\tRELATE clubber:`d395c589-c8bb-4f21-b38f-705dcc805af1`-\u003eowns-\u003eclub:`MyNewClub2` SET time.bought = time::now();\n\t\t\tSELECT * FROM club:`MyNewClub2` FETCH owner;\n\t\t",null]}

Edit: cannot replicate this on MacOS, but got the same output on two Linux machines (Arch and Mint)

ElecTwix commented 1 year ago

The Issue has been verified on my end too. OS: Arch

The Transformation

RELATE clubber:abc->owns->club:MyNewClub -> RELATE clubber:abc->owns-Ϭlub:MyNewClub

@KadoBOT could you share benchmark stats, it can be good to know the difference in CPU time.

KadoBOT commented 1 year ago

The Issue has been verified on my end too. OS: Arch

The Transformation

RELATE clubber:abc->owns->club:MyNewClub -> RELATE clubber:abc->owns-Ϭlub:MyNewClub

@KadoBOT could you share benchmark stats, it can be good to know the difference in CPU time.

Sure, I can do that. Where do I get it? Or, how do I run the benchmark?

ElecTwix commented 1 year ago

The Issue has been verified on my end too. OS: Arch

The Transformation

RELATE clubber:abc->owns->club:MyNewClub -> RELATE clubber:abc->owns-Ϭlub:MyNewClub @KadoBOT could you share benchmark stats, it can be good to know the difference in CPU time.

Sure, I can do that. Where do I get it? Or, how do I run the benchmark?

right now. there is no benchmark I had PR for that. I think it will merge soon if that merges you could get from there if it is not you can benchmark the db.Query and get the ns as diff.

KadoBOT commented 1 year ago

(base) ➜ benchmark git:(main) go test -bench=. goos: linux goarch: amd64 pkg: github.com/surrealdb/surrealdb.go/internal/benchmark cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz BenchmarkCreate-8 11149136 130.5 ns/op BenchmarkSelect-8 10311582 100.7 ns/op PASS ok github.com/surrealdb/surrealdb.go/internal/benchmark 6.869s

plally commented 1 year ago

After looking into this I believe the issue is with how surrealdb is decoding unicode escape sequences, It seems to expect 6 character escape sequences and not 4. I think it should happen if you have any 4 character unicode escape sequence followed by a character 0-F, this may be an issue that should be brought up in the surrealdb project itself.

You can find the code related to that issue here. https://github.com/surrealdb/surrealdb/blob/eeb46468aa96977edd7a3dd2e2c5b50133555a54/lib/src/sql/strand.rs#L167

KadoBOT commented 1 year ago

@plally can you check the PR I've opened? By not escaping the characters, it fixes the issue. I'm not sure tho if there are cases that it needs to be escaped.

plally commented 1 year ago

This issue was fixed in https://github.com/surrealdb/surrealdb/pull/1888