-
Notifications
You must be signed in to change notification settings - Fork 72
Fix LOCAL INFILE options in mysql driver handshake #204
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
base: master
Are you sure you want to change the base?
Conversation
@@ -513,6 +520,11 @@ defmodule MyXQL.Protocol do | |||
{:cont, :initial, {:many, [resultset | results]}} | |||
end | |||
|
|||
defp decode_resultset(<<0xFB, rest::binary>>, _next_data, :initial, _row_decoder) do | |||
{filename, ""} = take_string_nul(rest) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Base on the changes to take_string_nul
, it seems you are not expecting a nul
here? So maybe we should not call/change said function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, we shouldn't change the function!
lib/myxql/protocol.ex
Outdated
@@ -331,6 +332,12 @@ defmodule MyXQL.Protocol do | |||
{:halt, decode_ok_packet_body(rest)} | |||
end | |||
|
|||
def decode_com_query_response(<<0xFB, rest::binary>>, "", :initial) do | |||
{filename, _remaining} = take_string_nul(rest) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we can discard the rest here? Or should we assert it is an empty string as well? Or nul
even there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According here. The LOCAL INFILE request packet is
- 1 byte: 0xFB and
- N bytes: the filename as a NUL-terminated string
So after parsing the filename, there should be nothing left in the packet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we can match on the empty string as second tuple element as well!
def take_string_nul(binary) do | ||
[string, rest] = :binary.split(binary, <<0>>) | ||
{string, rest} | ||
def take_string_nul(binary) when is_binary(binary) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change still necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I tested it without the changes to the method, an exception occurred when using the client. So its necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But why? We said the filename is always followed by a nul byte, right? That will always return a two element list:
iex(2)> :binary.split(<<"/", 0>>, [<<0>>])
["/", ""]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I change it I got this error:
stopped: ** (MatchError) no match of right hand side value: ["/Users/iagocavalcante/Workspaces/i9Amazon/i9_processador/25332012000225-pdv_caixa_lancamento-20250618_210733-T-110.txt"]
(myxql 0.8.0-dev) lib/myxql/protocol/types.ex:74: MyXQL.Protocol.Types.take_string_nul/1
closes #203