In the beginning, error handling in MySQL code was a straightforward process. There was your C function
my_error(int nr, int MyFlags, char* format, ...)that would simply put the error on a queue for you, ready to be returned to the client at the right time. The two last arguments work just like
printf. You can easily check whether the current thread threw an error by reading
THD::is_error(). It is very simple and very hard to get wrong.
Later down the line, however, the need has surfaced for something more. If module A asks module B to do something as part of the execution of module A, module B might set a critical error since this would be the right thing to do had it not been part of A's execution. Then later A might call on module C to do something, but there might be code inside C that is not being executed because it notices that somewhere something went wrong and bails out.
An example (from
Bug#35996):
When the user issues
SHOW CREATE VIEW, it might be that the view references non-existing tables. Why this is allowed is another discussion, beyond the scope of this post. There must also be an access check. The user has to have
SELECT and
SHOW CREATE privilege on the view, and
SELECT privilege on all table columns that the view references.
The execution of
SHOW CREATE VIEW will call on the
check_access procedure, which recurses through the view itself and all tables or views that it references. This procedure is wired to raise an error for non-existing tables, and for columns that the user has no privileges an error will be raised saying
- Table 'test.t1' doesn't exist, or
- "... command denied to user ... for column ... in table ..."
respectively.
The latter error must be hidden by the
SHOW CREATE VIEW execution since it gives out information about tables that the user has no right to know. Remember, the view may just do a
SELECT *, but this error will actually name the columns for which the user is not authorized. The error must be replaced by the more anonymous
"View ... references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them".
This is accomplished by examining the current error in the queue, then
removing that error and replace it by the other one. . Recipe for disaster? Read on. Remember that
SHOW CREATE VIEW allows the user to see the definition (and for the view to exist) of a view referencing columns or even tables that don't exist. Hence it must manipulate the errors once again. The same message may be used, but the status must be warning level and not error level. A message with an error status has the authority to stop the transfer of information to the client, while a warning will simply pop up after the wanted result is transferred. The problem is, by simply looking at the anonymous error message we don't know what the original cause is:
- If it was a missing column, all is fine, let's downgrade an error to a warning an get on with it.
- If it was a column that the user has no right to know about, there must be an error.
What to do?
Obviously, it is bad design to have functions putting up errors only so that other functions' execution paths depend on what errors are on the queue. You need to know
why that error is there in the first place and what the circumstances were when it was put there. In other words, you need to be there when it happens.
Enter
Internal_error_handler. This brilliant invention is a hook for whenever an error is raised and a caller may install such a hook before calling a function. This way an appropriate action may be taken so that the
final error message may be set on the spot. It does this using a virtual method
handle_error. (
handle_condition in 6.0) Bug#35996 was fixed by making
SHOW CREATE VIEW, prior to calling
check_access, install an
Internal_error_handler that would:
- If the view itself was access checked when the error was raised, do nothing
- If a column or a table was access checked when the error was raised, hide the details of it.
- If the error was raised due to a missing column or table, hide the details of it and make it a warning.
Now is the time to take this approach one step further. The code that anonymized error messages is (for some reason) put in
TABLE_LIST:
void TABLE_LIST::hide_view_error(THD *thd)Post-bug#35996-fix the function is only called once, and in an awkward way. The fix to replace the code is trivial with lessons now learned. But a great amount of fragile code can get removed, spanning tens of files, so I will list the details below. Whenever a command is executed out on a view (e.g.
SELECT,
INSERT etc) referencing either non-allowed ot invalid columns/tables/databases, the error message gets anonymized. This is accomplished by a C-style function pointer in
Name_resolution_context called
process_error. This function constitues a hook for
TABLE_LIST::hide_view_error. The hook is called from various places in the name resolution code, probably only the ones that happen to be passed in most current scenarios. Most likely you can find a number of bugs here. The hook is installed in mysql_make_view, however.
All of this code can be removed and replaced with the more general Internal_error_handler. The benefits are obvious: We replace an unsafe C pointer with polymorphism, we substitue a general approach for an ad-hoc one, we remove unrelated functionality from TABLE_LIST and we don't have to sprinkle the code with easily-forgettable calls to the process_error hook.
