Adapt to Cloudflare Workers environment #2289

Mini256 · 2023-11-21T08:15:08Z

try to close #2179

Summary

To fix the error window is not found, replace node Timers with web timers
To fix the error Code generation from strings disallowed for this context, using the static parser in PR Add "interpreting" parser #2099
To fix the error crypto.hash() is not a function, polyfill node crypto withwebcrypto, and modify the functions which directly/indirectly called the crypto library as async function.

TODOs:

mysql_native_password auth plugin
caching_sha2_password auth plugin
sha256_password auth plugin
static_text_parser (will be finished in PR Add "interpreting" parser #2099)
static_binary_parser (will be finished in PR Add "interpreting" parser #2099)
add integration tests
repalce pg-cloudflare with mysql2-cloudflare?

Usage

For users, they need to:

Add node_compat = true flag to polyfill the missing node modules
Add useStaticParser flag to force mysql2 to use static parser.

For example: https://github.com/Mini256/mysql2-cloudflare-test/blob/1f339a85f4457b696f122503051991326dbc23dc/src/index.ts#L30

lib/connection.js

shiyuhang0 · 2023-12-25T07:38:47Z

This PR can run on Cloudflare Workers with the following database:

MySQL 5
MySQL 8
TiDB Serverless Cluster
PlanetScale

@sidorares could you please approve to run the GitHub action?

sidorares · 2023-12-25T09:23:17Z

@shiyuhang0 @Mini256 thanks for working on this! Could you document simple steps I can follow to try it myself?

lib/utils/nodecrypto.js

shiyuhang0 · 2023-12-25T11:08:41Z

@shiyuhang0 @Mini256 thanks for working on this! Could you document simple steps I can follow to try it myself?

cc3458f

This commit adds a step-by-step tutorial. Since it's user-facing, it needs a new release of MySQL2 including this PR.
it's better to follow the develop guide in the doc to test locally without a new release.

shiyuhang0 · 2023-12-27T03:11:40Z

CI is green. Maybe we can ignore the failure of CodeQL action, because the error is reported in the code that originally existed (just the location has changed)

As for the static parser. We'd like to merge your PR #2099 rather than cherry-pick it into this PR. What's your opinion? @sidorares
The static binary parser is also needed to make things work. Code generation from strings disallowed for this context still occurs when code goes through the binary parser.

sidorares · 2023-12-28T01:40:14Z

As for the static parser. We'd like to merge your PR #2099 rather than cherry-pick it into this PR. What's your opinion? @sidorares The static binary parser is also needed to make things work. Code generation from strings disallowed for this context still occurs when code goes through the binary parser.

I can probably merge it first, together with added binary protocol non-jit parser.
I don't like the flag name "useStaticParser", it reflects more implementation details rather than intention, but don't have better ideas. Can you think of a better flag name? "disableEvals" ? "strictMode" ( could be an umbrella flag, also prohibits cleartext plugin etc ). disableGenFunction ( again, too focused on implementation ). Naming things is hard 🤷‍♂️

re testing - thanks a lot for dev documentation. Are you aware of any way to test workers locally or in actions runtime? something similar to localstack. Ideally with the same checks, e.i errors when code tries to eval. Any container we can use for that?

Mini256 · 2023-12-28T03:19:12Z

Are you aware of any way to test workers locally or in actions runtime?

Cloudflare Workers provide an unstable_dev API for E2E / integration tests:

https://developers.cloudflare.com/workers/wrangler/api/#unstable_dev

Test code in node-postgres:

https://github.com/brianc/node-postgres/blob/6cd0aeb212d1672edd33499b2f4f858cf7ed9a79/packages/pg/test/worker/src/index.test.js#L13

shiyuhang0 · 2023-12-28T03:51:56Z

I can probably merge it first, together with added binary protocol non-jit parser.
I don't like the flag name "useStaticParser", it reflects more implementation details rather than intention, but don't have better ideas. Can you think of a better flag name? "disableEvals" ? "strictMode" ( could be an umbrella flag, also prohibits cleartext plugin etc ). disableGenFunction ( again, too focused on implementation ). Naming things is hard 🤷‍♂️

I prefer disableEvals among these names. You can also use other names, not good at naming for me :(

sidorares · 2023-12-28T05:08:15Z

I prefer disableEvals among these names. You can also use other names, not good at naming for me :(

I like it more than useStaticParser, I think I'll update in my branch ( together with a version for no evals binary parser ) and comment here when ready

advancedtw · 2024-01-23T17:06:16Z

I have tested this pr on multiple MySQL server ranging from old 5.7 to more recent MySQL and MariaDB version locally and on deployed workers so far it works flawlessly. The only issue is having to set node_compat = true instead of compatibility_flags = [ "nodejs_compat" ] but that's a small price to pay.
Also it's currently not possible to use drizzle with msyql driver but kysely works ok.

elithrar · 2024-01-24T13:54:07Z

FYI:

Other drivers (like node-postgres and Postgres.js) also require node_compat = true today
We have plans to allow nodejs_compat to work: this will likely require us to better audit specific packages and conditionally load/exclude Node.js APIs that we don't have support for in Workers, or that don't make sense in a serverless environment like Workers.

Otherwise, we'd love to see this merged + we're bringing MySQL support to Hyperdrive: https://developers.cloudflare.com/hyperdrive/

…arser

shiyuhang0 · 2024-03-06T03:38:05Z

I have resolved the conflict with master branch.
@sidorares Hi, what's the process of no evals parser? I'd like to remove the related codes in the PR after that.

wellwelwel · 2024-03-06T03:46:10Z

Hi, @shiyuhang0 🙋🏻‍♂️

If it's okay, I'd like to make a few simple suggestions in the documentation:

Change from .md to .mdx
Change the title from Run on Cloudflare Workers to Cloudflare Workers
I think it fits in the Examples section, what do you think?

shiyuhang0 · 2024-03-06T04:00:53Z

Hi, @shiyuhang0 🙋🏻‍♂️

If it's okay, I'd like to make a few simple suggestions in the documentation:

Change from .md to .mdx

Change the title from Run on Cloudflare Workers to Cloudflare Workers

I think it fits in the Examples section, what do you think?

already doing this, will push a commit soon

sidorares · 2024-03-06T11:11:41Z

@shiyuhang0 what do you think about useStaticParser flag name? Should we rename it to disableEval before its too late and somebody else depends on this name?

shiyuhang0 · 2024-03-07T05:46:21Z

@shiyuhang0 what do you think about useStaticParser flag name? Should we rename it to disableEval before its too late and somebody else depends on this name?

Both it is fine for me. But we shouldn't worry about modifying parameter names in an unmerged PR. For me, I think it can even be modified before release.

I will adjust this PR after the name is determined

m430 · 2024-03-27T03:12:31Z

I really need this feature. I need to use the node-mysql2 driver with Drizzle in Cloudflare Workers. If everything is confirmed to be correct, could you please merge this PR as soon as possible? Thanks

@sidorares

advancedtw · 2024-03-27T08:11:54Z

@m430 last time I checked Drizzle/Mysql was not compatible with cloudflare even using this updated driver. That is because of some node compatibility issue.
If you really need a mysql driver for cf today! you can use a git dependecy or @nora-soderlund cloudflare-mysql

@shiyuhang0 disableEval makes more sense to me

sidorares · 2024-03-29T00:45:42Z

@shiyuhang0 missed your message, I also prefer disableEval. Are you able to continue or you need someone to take over your work?

shiyuhang0 · 2024-05-06T02:22:02Z

@shiyuhang0 missed your message, I also prefer disableEval. Are you able to continue or you need someone to take over your work?

@sidorares I have pushed a new commit to use disableEval instead of useStaticParser . Is there anything else I need to do?

codecov · 2024-05-06T03:02:14Z

Codecov Report

Attention: Patch coverage is 55.14706% with 183 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (master@41d21eb). Click here to learn what that means.

Files	Patch %	Lines
lib/parsers/static_text_parser.js	11.45%	116 Missing ⚠️
lib/utils/nodecrypto.js	0.00%	33 Missing ⚠️
lib/stream.js	61.36%	17 Missing ⚠️
lib/auth_plugins/sha256_password.js	12.50%	7 Missing ⚠️
lib/commands/client_handshake.js	70.00%	3 Missing ⚠️
lib/connection.js	84.61%	2 Missing ⚠️
lib/utils/crypto.js	77.77%	2 Missing ⚠️
lib/auth_plugins/caching_sha2_password.js	92.30%	1 Missing ⚠️
lib/commands/change_user.js	80.00%	1 Missing ⚠️
lib/commands/query.js	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #2289   +/-   ##
=========================================
  Coverage          ?   89.47%           
=========================================
  Files             ?       76           
  Lines             ?    16021           
  Branches          ?     1363           
=========================================
  Hits              ?    14334           
  Misses            ?     1687           
  Partials          ?        0

Flag	Coverage Δ
compression-0	`89.47% <55.14%> (?)`
compression-1	`89.47% <55.14%> (?)`
tls-0	`88.96% <52.45%> (?)`
tls-1	`89.08% <46.07%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

elithrar · 2024-05-06T21:09:23Z

Add node_compat = true flag to polyfill the missing node modules

Is there a path to instead supporting our modern nodejs_compat mode by updating imports (conditionally) vs using the legacy polyfill mode?

https://developers.cloudflare.com/workers/runtime-apis/nodejs/

wellwelwel · 2024-05-06T21:51:20Z

I have pushed a new commit to use disableEval instead of useStaticParser . Is there anything else I need to do?

Just a note, while in the docs it says disableEval, in code it's useStaticParser 🙋🏻‍♂️

shiyuhang0 · 2024-05-07T08:39:59Z

I have pushed a new commit to use disableEval instead of useStaticParser . Is there anything else I need to do?

Just a note, while in the docs it says disableEval, in code it's useStaticParser 🙋🏻‍♂️

I forget push the change in code

Mini256 and others added 6 commits November 17, 2023 15:10

feat: add static text parser

44bf3e0

using pg-cloudflare

0e08207

polyfill crypto lib

a03c024

fix tests

d644cb8

support tls

5514def

fix access deny error

c4d5458

shiyuhang0 force-pushed the add-static-text-parser branch from 6954a22 to c4d5458 Compare November 21, 2023 11:10

shiyuhang0 reviewed Nov 21, 2023

View reviewed changes

lib/connection.js Outdated Show resolved Hide resolved

shiyuhang0 and others added 2 commits November 21, 2023 19:34

Update lib/connection.js

590cdef

delete useless code & verify servername

f2fe8d9

Mini256 marked this pull request as ready for review December 14, 2023 09:13

Mini256 marked this pull request as draft December 15, 2023 07:35

support cacheing-sha2

758f829

fix sha256 access deny

0b9245b

shiyuhang0 force-pushed the add-static-text-parser branch from 4760d37 to 0b9245b Compare December 25, 2023 09:04

github-advanced-security bot found potential problems Dec 25, 2023

View reviewed changes

lib/utils/nodecrypto.js Dismissed Show dismissed Hide dismissed

lib/utils/nodecrypto.js Dismissed Show dismissed Hide dismissed

lib/utils/nodecrypto.js Dismissed Show dismissed Hide dismissed

fix lint

d44c6d7

shiyuhang0 force-pushed the add-static-text-parser branch from 3034a14 to 8949aca Compare December 25, 2023 11:23

add documentation

cc3458f

shiyuhang0 force-pushed the add-static-text-parser branch from 8949aca to cc3458f Compare December 25, 2023 11:24

fix lint

96f630f

Merge remote-tracking branch 'upstream/master' into add-static-text-p…

68280b7

…arser

shiyuhang0 added 2 commits March 6, 2024 12:06

move doc

fa76765

fix lint

6da01bf

rename

da73b7a

Mini256 marked this pull request as ready for review May 6, 2024 06:58

rename to disableEval

9b78fe1

wellwelwel mentioned this pull request May 14, 2024

ci(coverage): disable Codecov patch #2674

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adapt to Cloudflare Workers environment #2289

Adapt to Cloudflare Workers environment #2289

Mini256 commented Nov 21, 2023 •

edited

shiyuhang0 commented Dec 25, 2023

sidorares commented Dec 25, 2023

shiyuhang0 commented Dec 25, 2023 •

edited

shiyuhang0 commented Dec 27, 2023 •

edited

sidorares commented Dec 28, 2023

Mini256 commented Dec 28, 2023 •

edited

shiyuhang0 commented Dec 28, 2023

sidorares commented Dec 28, 2023

advancedtw commented Jan 23, 2024

elithrar commented Jan 24, 2024

shiyuhang0 commented Mar 6, 2024

wellwelwel commented Mar 6, 2024 •

edited

shiyuhang0 commented Mar 6, 2024

sidorares commented Mar 6, 2024

shiyuhang0 commented Mar 7, 2024 •

edited

m430 commented Mar 27, 2024

advancedtw commented Mar 27, 2024 •

edited

sidorares commented Mar 29, 2024

shiyuhang0 commented May 6, 2024 •

edited

codecov bot commented May 6, 2024 •

edited

elithrar commented May 6, 2024 •

edited

wellwelwel commented May 6, 2024 •

edited

shiyuhang0 commented May 7, 2024

Adapt to Cloudflare Workers environment #2289

Are you sure you want to change the base?

Adapt to Cloudflare Workers environment #2289

Conversation

Mini256 commented Nov 21, 2023 • edited

shiyuhang0 commented Dec 25, 2023

sidorares commented Dec 25, 2023

shiyuhang0 commented Dec 25, 2023 • edited

shiyuhang0 commented Dec 27, 2023 • edited

sidorares commented Dec 28, 2023

Mini256 commented Dec 28, 2023 • edited

shiyuhang0 commented Dec 28, 2023

sidorares commented Dec 28, 2023

advancedtw commented Jan 23, 2024

elithrar commented Jan 24, 2024

shiyuhang0 commented Mar 6, 2024

wellwelwel commented Mar 6, 2024 • edited

shiyuhang0 commented Mar 6, 2024

sidorares commented Mar 6, 2024

shiyuhang0 commented Mar 7, 2024 • edited

m430 commented Mar 27, 2024

advancedtw commented Mar 27, 2024 • edited

sidorares commented Mar 29, 2024

shiyuhang0 commented May 6, 2024 • edited

codecov bot commented May 6, 2024 • edited

Codecov Report

elithrar commented May 6, 2024 • edited

wellwelwel commented May 6, 2024 • edited

shiyuhang0 commented May 7, 2024

Mini256 commented Nov 21, 2023 •

edited

shiyuhang0 commented Dec 25, 2023 •

edited

shiyuhang0 commented Dec 27, 2023 •

edited

Mini256 commented Dec 28, 2023 •

edited

wellwelwel commented Mar 6, 2024 •

edited

shiyuhang0 commented Mar 7, 2024 •

edited

advancedtw commented Mar 27, 2024 •

edited

shiyuhang0 commented May 6, 2024 •

edited

codecov bot commented May 6, 2024 •

edited

elithrar commented May 6, 2024 •

edited

wellwelwel commented May 6, 2024 •

edited