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.

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

Add LOCAL INFILE support #215

Merged
merged 4 commits into from
Jan 29, 2024
Merged

Conversation

mirromutth
Copy link
Contributor

@mirromutth mirromutth commented Jan 23, 2024

Motivation:

Add LOCAL INFILE support.

See also #206 .

Modification:

  • Improve envelope codec for SubsequenceClientMessage
  • Add LocalInfileRequest/LocalInfileResponse for text protocol
  • Add an option to enable/disable LOCAL INFILE support, and it should be a string to specify "safety path"
  • Add an option to specify file buffer size
  • Write docs

Result:

Query LOCAL INFILE support.

@mirromutth mirromutth added the enhancement New feature or request label Jan 23, 2024
@mirromutth mirromutth linked an issue Jan 23, 2024 that may be closed by this pull request
@mirromutth mirromutth force-pushed the 206-feature-support-local-infile branch 3 times, most recently from 9f99c07 to d59f240 Compare January 23, 2024 11:08
- Improve envelope codec for `SubsequenceClientMessage`
- Add `LocalInfileRequest`/`LocalInfileResponse` for text protocol
@mirromutth mirromutth force-pushed the 206-feature-support-local-infile branch from d59f240 to 60765f8 Compare January 24, 2024 01:36
@mirromutth
Copy link
Contributor Author

mirromutth commented Jan 24, 2024

Hi, @jchrys @JohnNiang

In JDBC-MySQL, allowLoadLocalInfileInPath will work only if allowLoadLocalInfile=false. The allowLoadLocalInfile option means "allow LOAD DATA LOCAL INFILE and allow it to load file data from any path". It seems to be for historical reasons.

For example:

  • allowLoadLocalInfile=true for allowed and unrestricted LOCAL INFILE
  • allowLoadLocalInfileInPath=/opt for allowed and restricted LOCAL INFILE
  • allowLoadLocalInfile=true&allowLoadLocalInfileInPath=/opt for allowed and UNRESTRICTED LOCAL INFILE, allowLoadLocalInfileInPath will be ignored

In my opinion, r2dbc-mysql should make allowLoadLocalInfile just mean "allow LOAD DATA LOCAL INFILE", and that can be restricted by loadLocalInfileInPath. Which means, loadLocalInfileInPath will work only if allowLoadLocalInfile=true, and all LOCAL INFILE can only read files in the path.

For example:

  • allowLoadLocalInfile=true for allowed and unrestricted LOCAL INFILE
  • allowLoadLocalInfile=true&loadLocalInfileInPath=/opt for allowed and restricted LOCAL INFILE
  • loadLocalInfileInPath=/opt, LOCAL INFILE will not be allowed

This prevents users from accidentally setting up unrestricted file access.

Or we can make it the same as JDBC.

Any ideas?

@jchrys
Copy link
Collaborator

jchrys commented Jan 24, 2024

I agree with your idea but would like to suggest a slightly different approach.

What if we consider not supporting the allowLoadLocalInfile property? (or keep (jdbc version of)allowLoadLocalInfile = false)

I think that the property allowLoadLocalInfileInPath={path} could effectively convey the user's intentions(allowLoadLocalInfileInPath=/ for unrestricted case as well).

What are your thoughts on this?

@mirromutth mirromutth force-pushed the 206-feature-support-local-infile branch from 264c4bb to bd30369 Compare January 24, 2024 10:04
@mirromutth mirromutth marked this pull request as ready for review January 24, 2024 10:05
Copy link
Collaborator

@jchrys jchrys left a comment

Choose a reason for hiding this comment

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

LGTM

@mirromutth mirromutth force-pushed the 206-feature-support-local-infile branch from 4eccf5d to 99b73c5 Compare January 29, 2024 03:50
@mirromutth mirromutth merged commit 0bf7663 into trunk Jan 29, 2024
12 checks passed
@mirromutth mirromutth deleted the 206-feature-support-local-infile branch January 29, 2024 05:10
@jchrys jchrys added this to the 1.1.0/0.10.0 milestone Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature] Support LOCAL INFILE
2 participants