planetscale / database-js

A Fetch API-compatible PlanetScale database driver
https://planetscale.com/docs/tutorials/planetscale-serverless-driver
Apache License 2.0
1.17k stars 35 forks source link

feat(sanitization): support `bigint` params by not wrapping them in quotes #159

Closed jkomyno closed 10 months ago

jkomyno commented 10 months ago

Hi 👋🏻, Alberto from @prisma here.

This PR fixes a surprising behavior we at @prisma found while adding new tests for an internal PR related to our new Driver Adapters feature.

We noticed that the JavaScript PlanetScale driver currently doesn't support bigint query parameters: these parameters are treated as strings by the client-side sanitization routine, causing SQL syntax errors at runtime. For instance, SELECT * FROM user LIMIT ? with parameters [1n] is validated as SELECT * FROM user LIMIT '1', which of course results in a SQL error.

Here is the relevant output of pnpm test after adding this new test:

 FAIL  __tests__/sanitization.test.ts
  ● sanitization › format › formats bigint values

    expect(received).toEqual(expected) // deep equality

    Expected: "select 1 from user where id=12 and id2=42 and id3=9223372036854775807"
    Received: "select 1 from user where id='12' and id2='42' and id3='9223372036854775807'"

      40 |       const query = 'select 1 from user where id=? and id2=? and id3=?'
      41 |       const expected = 'select 1 from user where id=12 and id2=42 and id3=9223372036854775807'
    > 42 |       expect(format(query, [12n, 42n, 9223372036854775807n])).toEqual(expected)
         |                                                               ^
      43 |     })
      44 |
      45 |     test('formats string values', () => {

      at Object.<anonymous> (__tests__/sanitization.test.ts:42:63)

If we apply the same sanitization logic currently applied on number query parameters, the tests pass.

 PASS  __tests__/sanitization.test.ts
 PASS  __tests__/index.test.ts
 PASS  __tests__/text.test.ts

Test Suites: 3 passed, 3 total
Tests:       54 passed, 54 total
Snapshots:   0 total
Time:        0.604 s, estimated 1 s
Ran all test suites

I'm of course available for feedback concerning this PR :)