MySQL Bugs: #62712: RPM-based install of the test suite is useless for non-root users.

Bug #62712	RPM-based install of the test suite is useless for non-root users.
Submitted:	13 Oct 2011 0:54	Modified:	12 Dec 2011 14:34
Reporter:	Davi Arnaut (OCA)	Email Updates:
Status:	Closed	Impact on me:	None
Category:	Tools: MTR / mysql-test-run	Severity:	S7 (Test Cases)
Version:	5.5.17	OS:	Any
Assigned to:	Bjørn Munch	CPU Architecture:	Any

Description:
Although a user with read access to the mysql-test root directory should be able to execute tests, this actually does not work in some tests because mysqltest tries to preserve the original owner and group when copying files.

For example, when the test suite is installed using the MySQL-test RPM package, the owner and group of the installed files will be set to root. If a regular non-root user attempts to run a test case that needs to copy a file, the copying (and subsequently the test) will fail because the copy routine attempts to change the owner/group of the new file to root.

This also affects all installations where the the user is not part of the group that owns the files used by test cases.

How to repeat:
1. Install MySQL-test RPM package.
3. Use a non-root user.
2. cd /usr/share/mysql-test
4. ./mtr --vardir=/tmp/var view

It should fail with:

At line 3936: command "copy_file" failed with error 1. my_errno=9

Error code 9 is permission denied.

The line in question is:

copy_file std_data/bug48449.frm $MYSQLD_DATADIR/test/v2.frm;

The copy fails because the source file is owned by root and changing the owner of the destination file to root is denied.

Suggested fix:
Add a flag to my_copy so that file ownership is not copied. Add this flag to the usage of my_copy in mysqlclient.cc where it is being used to implement the copy_file command.

Fix:

diff --git a/client/mysqltest.cc b/client/mysqltest.cc
index 77289b3..abdefb4 100644
--- a/client/mysqltest.cc
+++ b/client/mysqltest.cc
@@ -3253,7 +3253,7 @@ void do_copy_file(struct st_command *command)
 
   DBUG_PRINT("info", ("Copy %s to %s", ds_from_file.str, ds_to_file.str));
   error= (my_copy(ds_from_file.str, ds_to_file.str,
-                  MYF(MY_DONT_OVERWRITE_FILE)) != 0);
+                  MYF(MY_DONT_OVERWRITE_FILE | MY_HOLD_ORIGINAL_MODES)) != 0);
   handle_command_error(command, error);
   dynstr_free(&ds_from_file);
   dynstr_free(&ds_to_file);

I had another fix in the pipeline which would involve changing the mysys library. It has been waiting due to vacation etc.

I considered the suggested simpler fix but that would potentially change the existing behavior. Still, that quite possibly doesn't mean anything in practice so maybe we'll do that instead.

FWIW, initially I was too going for a mysys fix, but upon closer inspection I realized that MY_HOLD_ORIGINAL_MODES actually means that file modes are not changed ("my_copy() holds to file modes", although that is still a bit confusing).

Another thing that goes against a mysys change is that there is code duplication in mysys, between my_copy and my_copystat. Ideally, the stat copying in my_copy could be killed and my_copystat could be used where necessary (which seems to be a couple of places in myisampack.c).

Yes the use of this flag is confusing. BTW it cannot actually change the behavior anyway. The only case where adding the new flag would, is if the destination file exists. But since we also use MY_DONT_OVERWRITE_FILE, copy_file would fail in those cases anyway.

Fixed in 5.5.19