Uploaded image for project: 'MariaDB Connector/C'
  1. MariaDB Connector/C
  2. CONC-515

Implement safe LOAD DATA LOCAL INFILE method

Details

    • Task
    • Status: Stalled (View Workflow)
    • Major
    • Resolution: Unresolved
    • None
    • 3.5
    • Security
    • None

    Description

      Implement safe "LOAD DATA LOCAL INFILE" method

      Prerequisites

      1. Clients should be capabable to check if a SQL statement is a LOAD DATA LOCAL INFILE statement and parse the specified filename.

      2. Client and Server indicate by extended capability flag MARIADB_CLIENT_SAFE_LOCAL_INFILE that they can handle LOAD DATA LOCAL INFILE in a safely manner.

      Workflow

      Client will parse the SQL statement and in case it was a LOAD DATA LOCAL INFILE statement parse the specified filename.

      If the filename does not exist or is not readable by client an error will be returned.

      In case both server and client have MARIADB_CLIENT_SAFE_LOCAL_FILE capability flag set, the server will not send parsed filename anymore to client. The client itself sends the content of the file immediately after sending the LOAD DATA LOCAL INFILE statement to the server.

      If only client is capable to handle LOAD DATA LOCAL INFILE in a safely manner, client will read server response and check if the filename sent by server will match the parsed file name.
      Additionally the client will check if server sent a file sending request, if the previously executed command was a LOAD DATA LOCAL INFILE statement.

      Unsetting CLIENT_LOCAL_FILES flag (disabling LOAD DATA LOCAL INFILE) will not have any effect since the client will be able to safely handle LOAD DATA LOCAL INFILE.

      Attachments

        Issue Links

          Activity

            diego dupin Diego Dupin added a comment - - edited

            Current proposition is not the best fit :

            • driver must NOT parse command.
            • forbid having multiple exchanges commands.

            Load local infile is the only exchange with multiples request-answer in the protocol after authentication.

            The problem is that this block async driver to work the best possible way.

            After authentication, if load local infile capability is disabled, async driver can fully use the possibility that non blocking socket offer : pipelining request then read response orderly. This is what async offer, no possibility of socket hanging because of socket buffer full.

            So when all exchanges are only request - answer, that's perfect, driver can just send command, and when corresponding answer comes, deliver result.

            If there is even one command that can have multiple exchanges, then driver will have to ensure having the response of previous request before sending next command. The alternative is to parse command then lock all other commands until completion if command was a load local infile.

            Actual LOAD LOCAL INFILE command are blocked by default by recent server.
            2 of our drivers : node.js and R2DBC disable LOAD LOCAL INFILE capability client side, to ensure permit pipelining by default without needing to parse commands.
            That's will be history in some time, and this is a good thing.

            If this functionality is essential, this will have to be done in a one exchange command, and probably have a dedicated API to send that special command.
            maybe a new specific COM_SEND_FILE, that permit to send the file, then the COM_QUERY with LOAD LOCAL INFILE command without file name

            diego dupin Diego Dupin added a comment - - edited Current proposition is not the best fit : driver must NOT parse command. forbid having multiple exchanges commands. Load local infile is the only exchange with multiples request-answer in the protocol after authentication. The problem is that this block async driver to work the best possible way. After authentication, if load local infile capability is disabled, async driver can fully use the possibility that non blocking socket offer : pipelining request then read response orderly. This is what async offer, no possibility of socket hanging because of socket buffer full. So when all exchanges are only request - answer, that's perfect, driver can just send command, and when corresponding answer comes, deliver result. If there is even one command that can have multiple exchanges, then driver will have to ensure having the response of previous request before sending next command. The alternative is to parse command then lock all other commands until completion if command was a load local infile. Actual LOAD LOCAL INFILE command are blocked by default by recent server. 2 of our drivers : node.js and R2DBC disable LOAD LOCAL INFILE capability client side, to ensure permit pipelining by default without needing to parse commands. That's will be history in some time, and this is a good thing. If this functionality is essential, this will have to be done in a one exchange command, and probably have a dedicated API to send that special command. maybe a new specific COM_SEND_FILE, that permit to send the file, then the COM_QUERY with LOAD LOCAL INFILE command without file name

            diego dupin, this is the task of what a connector can do without any changes to the server and with breaking as few existing applications as possible. Under those constraints this approach works quite good, perhaps it's even the best fit.

            If it's allowed to break existing applications or to modify the protocol then you can do larger changes, of course.

            serg Sergei Golubchik added a comment - diego dupin , this is the task of what a connector can do without any changes to the server and with breaking as few existing applications as possible. Under those constraints this approach works quite good, perhaps it's even the best fit. If it's allowed to break existing applications or to modify the protocol then you can do larger changes, of course.
            georg Georg Richter added a comment -

            serg I don't think that introducing a new capability flag (CLIENT_SAFE_LOCAL_INFILE) would be much overhead: if both client and server have this capability flag set, the server doesn't need to send the filename anymore to the client - and it will not break existing applications. Additionally the configuration variable local_infile=ON/OFF would only apply if client isn't able to handle safe load data local infile.

            However on client side both variants will require either parsing, or to specify a filename e.g. via mysql_optionsv() before executing the statement.

            georg Georg Richter added a comment - serg I don't think that introducing a new capability flag (CLIENT_SAFE_LOCAL_INFILE) would be much overhead: if both client and server have this capability flag set, the server doesn't need to send the filename anymore to the client - and it will not break existing applications. Additionally the configuration variable local_infile=ON/OFF would only apply if client isn't able to handle safe load data local infile. However on client side both variants will require either parsing, or to specify a filename e.g. via mysql_optionsv() before executing the statement.
            georg Georg Richter added a comment -

            A special case are multi statements (statements concatenated by semicolon) and the init_command which can be send after connection was established. Either we need to parsecompletely, or we need to document that LOAD DATA LOCAL INFILE might fail if used inside multi statements.

            georg Georg Richter added a comment - A special case are multi statements (statements concatenated by semicolon) and the init_command which can be send after connection was established. Either we need to parsecompletely, or we need to document that LOAD DATA LOCAL INFILE might fail if used inside multi statements.

            People

              georg Georg Richter
              georg Georg Richter
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.