List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:March 14 2011 10:48am
Subject:Re: bzr commit into mysql-5.5 branch (Dmitry.Shulga:3362) Bug#11764168
View as plain text  
Hi Dmitry,

On 3/14/11 2:52 AM, Dmitry Shulga wrote:
> #At file:///Users/shulga/projects/mysql/mysql-5.5/ based on
> revid:marc.alff@stripped
>
>   3362 Dmitry Shulga	2011-03-14 [merge]
>        Manual merge from mysql-5.1 for Bug#11764168 (56976: Severe denial
>        of service in prepared statements).
>

[...]

OK to push. One request below, which can pushed in a separate patch.

>
> === modified file 'sql/mysqld.h'
> --- a/sql/mysqld.h	2011-03-10 08:43:55 +0000
> +++ b/sql/mysqld.h	2011-03-14 05:51:23 +0000
> @@ -126,6 +126,7 @@ extern char *default_storage_engine;
>   extern bool opt_endinfo, using_udf_functions;
>   extern my_bool locked_in_memory;
>   extern bool opt_using_transactions;
> +extern ulong max_long_data_size;
>   extern ulong current_pid;
>   extern ulong expire_logs_days;
>   extern my_bool relay_log_recovery;
> @@ -397,7 +398,8 @@ enum options_mysqld
>     OPT_UPDATE_LOG,
>     OPT_WANT_CORE,
>     OPT_ENGINE_CONDITION_PUSHDOWN,
> -  OPT_LOG_ERROR
> +  OPT_LOG_ERROR,
> +  OPT_MAX_LONG_DATA_SIZE
>   };
>
>
>
> === modified file 'sql/sql_prepare.cc'
> --- a/sql/sql_prepare.cc	2011-03-01 14:42:37 +0000
> +++ b/sql/sql_prepare.cc	2011-03-14 05:51:23 +0000
> @@ -2784,6 +2784,34 @@ void mysql_sql_stmt_close(THD *thd)
>     }
>   }
>
> +
> +class Set_longdata_error_handler : public Internal_error_handler
> +{
> +public:
> +  Set_longdata_error_handler(Prepared_statement *statement)
> +    : stmt(statement)
> +  { }
> +
> +public:
> +  bool handle_condition(THD *thd,
> +                        uint sql_errno,
> +                        const char* sqlstate,
> +                        MYSQL_ERROR::enum_warning_level level,
> +                        const char* msg,
> +                        MYSQL_ERROR ** cond_hdl)
> +  {
> +    stmt->state= Query_arena::ERROR;
> +    stmt->last_errno= sql_errno;
> +    strncpy(stmt->last_error, msg, MYSQL_ERRMSG_SIZE);
> +
> +    return TRUE;
> +  }
> +
> +private:
> +  Prepared_statement *stmt;
> +};
> +
> +
>   /**
>     Handle long data in pieces from client.
>
> @@ -2840,16 +2868,19 @@ void mysql_stmt_get_longdata(THD *thd, c
>
>     param= stmt->param_array[param_number];
>
> +  Set_longdata_error_handler err_handler(stmt);
> +  /*
> +    Install handler that will catch any errors that can be generated
> +    during execution of Item_param::set_longdata() and propagate
> +    them to Statement::last_error.
> +  */
> +  thd->push_internal_handler(&err_handler);

Instead of pushing a internal handler, please save, set and restore the 
diagnostics area and warning info (thd->stmt_da and thd->warning_info).

It looks something like:

void mysql_stmt_get_longdata(THD *thd, ...)

   Diagnostics_area diagnostics_area, *save_diagnostics_area= thd->stmt_da;
   Warning_info warning_info(thd->query_id), *save_warning_info= 
thd->warning_info;

   save_diagnostics_area= thd->stmt_da;
   save_warning_info= thd->warning_info;

   thd->stmt_da= &diagnostics_area;
   thd->warning_info= &warning_info;

   param->set_longdata(thd->extra_data, thd->extra_length);

   if (thd->stmt_da->is_error())
   {
     stmt->state= Query_arena::ERROR;
     stmt->last_errno= thd->stmt_da->sql_errno();
     strncpy(stmt->last_error, thd->stmt_da->message(), MYSQL_ERRMSG_SIZE);
   }

   thd->stmt_da= save_diagnostics_area;
   thd->warning_info= save_warning_info;

This fits better into the error handling model and allow us to extend 
things further if necessary. Also, it's slightly better if there are 
multiple errors as we will get the first error which caused the DA 
status to transition.

Regards,

Davi
Thread
bzr commit into mysql-5.5 branch (Dmitry.Shulga:3362) Bug#11764168Dmitry Shulga14 Mar
  • Re: bzr commit into mysql-5.5 branch (Dmitry.Shulga:3362) Bug#11764168Davi Arnaut14 Mar