← Back to team overview

maria-developers team mailing list archive

Re: [Commits] 5fab5b6: MDEV-8764: Wrong error when encrypted table can't be decrypted.

 

Hi, Jan!

On Sep 08, Jan Lindström wrote:

> MDEV-8764: Wrong error when encrypted table can't be decrypted.
> 
> Add a new error message when table is encrypted but decryption
> fails. Use this new error message on InnoDB/XtraDB.
> 
> diff --git a/include/my_handler_errors.h b/include/my_handler_errors.h
> index a7afcfe..9e452b0 100644
> --- a/include/my_handler_errors.h
> +++ b/include/my_handler_errors.h
> @@ -94,7 +94,8 @@ static const char *handler_error_messages[]=
>    "Operation was interrupted by end user (probably kill command?)",
>    "Disk full",
>    "Incompatible key or row definition between the MariaDB .frm file and the information in the storage engine. You have to dump and restore the table to fix this",
> -  "Too many words in a FTS phrase or proximity search"
> +  "Too many words in a FTS phrase or proximity search",
> +  "Table encrypted but decryption failed"

I'm not sure I like the wording ("table is encrypted" adds no useful
information, it's not like there can be another error "table is not encrypted
but decryption failed"). But that's up to you.

And yes, I agree that this situation deserves a dedicated error message.

>  };
>  
>  #endif /* MYSYS_MY_HANDLER_ERRORS_INCLUDED */
> diff --git a/sql/handler.cc b/sql/handler.cc
> index 1d1dae7..566db8a 100644
> --- a/sql/handler.cc
> +++ b/sql/handler.cc
> @@ -3518,6 +3519,10 @@ void handler::print_error(int error, myf errflag)
>      my_error(ER_NO_SUCH_TABLE_IN_ENGINE, errflag, table_share->db.str,
>               table_share->table_name.str);
>      DBUG_VOID_RETURN;
> +  case HA_ERR_DECRYPTION_FAILED:
> +    my_error(ER_DECRYPTION_FAILED, errflag, table_share->db.str,
> +             table_share->table_name.str);
> +    DBUG_VOID_RETURN;

Is that needed? I'd rather use the default ER_GET_ERRMSG, which would be

  Got error 192 'Table encrypted but decryption failed" from InnoDB

I think this is perfectly adequate error for internal engine errors.
ER_xxx is for upper-level errors.

>    case HA_ERR_RBR_LOGGING_FAILED:
>      textno= ER_BINLOG_ROW_LOGGING_FAILED;
>      break;
> diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt
> index 59908dc..ff72a26 100644
> --- a/sql/share/errmsg-utf8.txt
> +++ b/sql/share/errmsg-utf8.txt
> @@ -7136,3 +7136,5 @@ ER_KILL_QUERY_DENIED_ERROR
>          eng "You are not owner of query %lu"
>          ger "Sie sind nicht Eigentümer von Abfrage %lu"
>          rus "Вы не являетесь владельцем запроса %lu"
> +ER_DECRYPTION_FAILED  42S02
> +        eng "Table '%-.192s.%-.192s' encrypted but decryption failed."

In general, please prefer %`s.%`s over '%s.%s'
the former uses correct identifier quoting, the second does not

Regards,
Sergei