-
Notifications
You must be signed in to change notification settings - Fork 36
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
base64 compatibility #49
Comments
Chiming in: beyond being outdated and not recommended for new projects, By using these methods, the package is naively considering the values as being encoded in ASCII formatting, thus destroying characters outside the aforementioned encoding. The following TEXT value would be butchered: “Ô meu amigo, nós precisamos reunir o pessoal para o aniversário de José” I have confirmed that the decoding issue happens locally by forcing |
Do you have an example where this fails with a table we can reproduce with? I have a bunch of test data that tests various encodings and Unicode data without any issues currently, so I'm curious what fails in practice here so we can construct a test case.
From my understanding tho, our usage is safe and correct since the data being decoded is binary bytes, which we explicitly encode to utf8 if/when strings are needed.
So within the context of our usage, I don't believe we corrupt any data.
|
Hey @mattrobenolt! You are right to assume the data received is indeed a correctly encoded binary UTF-8 string. The thing with Here's a simple example using my test above: {
"headers": [
":vtg1 /* VARCHAR */"
],
"types": {
":vtg1 /* VARCHAR */": "VARCHAR"
},
"fields": [
{
"name": ":vtg1 /* VARCHAR */",
"type": "VARCHAR",
"charset": 255
}
],
"rows": [
{
":vtg1 /* VARCHAR */": "� meu amigo, nós precisamos reunir o pessoal para o aniversário de José"
}
],
"rowsAffected": 0,
"insertId": "0",
"size": 1,
"statement": "SELECT \"Ô meu amigo, nós precisamos reunir o pessoal para o aniversário de José\"",
"time": 1.217693
} Now, if I monkeypatch
The response is correctly parsed: {
"headers": [
":vtg1 /* VARCHAR */"
],
"types": {
":vtg1 /* VARCHAR */": "VARCHAR"
},
"fields": [
{
"name": ":vtg1 /* VARCHAR */",
"type": "VARCHAR",
"charset": 255
}
],
"rows": [
{
":vtg1 /* VARCHAR */": "Ô meu amigo, nós precisamos reunir o pessoal para o aniversário de José"
}
],
"rowsAffected": 0,
"insertId": "0",
"size": 1,
"statement": "SELECT \"Ô meu amigo, nós precisamos reunir o pessoal para o aniversário de José\"",
"time": 2.099751
} |
I'll spend a little time with this later but I think this is something different. I'll explain if I'm correct. |
@mattrobenolt it is indeed something different. There was a misunderstanding in my part about the In my thought process, I though |
Ah, yeah, that checks out. It's definitely expected for the raw data to be binary encoded data, so is entirely safe to do |
We currently rely on
btoa
andatob
, because that works in browsers and worked in versions of Node we tested against, but that's not as universal as we had hoped.We should instead, provide some compatibility shim against the
Buffer
API that should more likely exist? We might need to provide a fallback for even that if neitherBuffer
norbtoa
and friends exist.But for now, it seems either way, it'd be more favorable to use the
Buffer
API when available overbtoa
.I propose something like this, granted I dunno the best way to do this in JavaScript/TypeScript these days:
I think this is still reasonably applicable since we don't simply use it for the Authorization header. While that's the only use of
btoa
,atob
is used to decode binary row data, which we do for every row in a QueryResult.Refs: #47
The text was updated successfully, but these errors were encountered: