xtrabackup arguments are not validated

Bug #717256 reported by Kenny Gryp on 2011-02-11
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Percona XtraBackup
Medium
Unassigned
2.0
Medium
Unassigned
2.1
Medium
Unassigned
2.2
Medium
Unassigned
2.3
Medium
Unassigned

Bug Description

Have a look at the arguments I used. A typo in the arguments can cause wrong things to happen (--incremental-dir in this case)
root@lesktop:/data/backups# xtrabackup --prepare --apply-log-only --target-dir=/data/backups/base_xtrabackup/ --incremaaaal-dir=/data/backups/inc1/ --i-can-add --whatever-i-want
xtrabackup Ver 1.4 Rev 193 for 5.1.47 unknown-linux-gnu (x86_64)
xtrabackup: cd to /data/backups/base_xtrabackup/
xtrabackup: This target seems to be already prepared.
xtrabackup: notice: xtrabackup_logfile was already used to '--prepare'.
xtrabackup: Temporary instance for recovery is set as followings.
xtrabackup: innodb_data_home_dir = ./
xtrabackup: innodb_data_file_path = ibdata1:10M:autoextend
xtrabackup: innodb_log_group_home_dir = ./
xtrabackup: innodb_log_files_in_group = 2
xtrabackup: innodb_log_file_size = 5242880
xtrabackup: Starting InnoDB instance for recovery.
xtrabackup: Using 104857600 bytes for buffer pool (set by --use-memory parameter)
InnoDB: The InnoDB memory heap is disabled
InnoDB: Mutexes and rw_locks use GCC atomic builtins
InnoDB: Compressed tables use zlib 1.2.3
110211 17:25:22 InnoDB: highest supported file format is Barracuda.

[notice (again)]
  If you use binary log and don't use any hack of group commit,
  the binary log position seems to be:
InnoDB: Last MySQL binlog file position 0 61013, file name /var/log/mysql/mysql-bin.000117

xtrabackup: starting shutdown with innodb_fast_shutdown = 1
110211 17:25:22 InnoDB: Starting shutdown...
110211 17:25:22 InnoDB: Shutdown completed; log sequence number 4960382476

Expected behavior: bail out when invalid arguments are added.

Changed in percona-xtrabackup:
assignee: nobody → Valentine Gostev (core-longbow)
Alexey Kopytov (akopytov) wrote :

I hit this bug too, but then realized it is an intended behavior. Since xtrabackup reads some of its options from my.cnf, it has to ignore all options it doesn't know.

On the other hand, silently ignoring typos in command line options is definitely not a good idea for a backup utility. I am not sure how this can be resolved. One idea is to parse my.cnf with a custom parser rather than my_getopt.

Alexey,

Should not we build a list of options which it needs to ignore ?

On Fri, Feb 11, 2011 at 8:47 AM, Alexey Kopytov
<email address hidden>wrote:

> I hit this bug too, but then realized it is an intended behavior. Since
> xtrabackup reads some of its options from my.cnf, it has to ignore all
> options it doesn't know.
>
> On the other hand, silently ignoring typos in command line options is
> definitely not a good idea for a backup utility. I am not sure how this
> can be resolved. One idea is to parse my.cnf with a custom parser rather
> than my_getopt.
>
> --
> You received this bug notification because you are a member of Percona
> developers, which is the registrant for Percona-XtraBackup.
> https://bugs.launchpad.net/bugs/717256
>
> Title:
> xtrabackup arguments are not validated
>
> Status in Open source backup tool for InnoDB and XtraDB:
> New
>
> Bug description:
> Have a look at the arguments I used. A typo in the arguments can cause
> wrong things to happen (--incremental-dir in this case)
> root@lesktop:/data/backups# xtrabackup --prepare --apply-log-only
> --target-dir=/data/backups/base_xtrabackup/
> --incremaaaal-dir=/data/backups/inc1/ --i-can-add --whatever-i-want
> xtrabackup Ver 1.4 Rev 193 for 5.1.47 unknown-linux-gnu (x86_64)
> xtrabackup: cd to /data/backups/base_xtrabackup/
> xtrabackup: This target seems to be already prepared.
> xtrabackup: notice: xtrabackup_logfile was already used to '--prepare'.
> xtrabackup: Temporary instance for recovery is set as followings.
> xtrabackup: innodb_data_home_dir = ./
> xtrabackup: innodb_data_file_path = ibdata1:10M:autoextend
> xtrabackup: innodb_log_group_home_dir = ./
> xtrabackup: innodb_log_files_in_group = 2
> xtrabackup: innodb_log_file_size = 5242880
> xtrabackup: Starting InnoDB instance for recovery.
> xtrabackup: Using 104857600 bytes for buffer pool (set by --use-memory
> parameter)
> InnoDB: The InnoDB memory heap is disabled
> InnoDB: Mutexes and rw_locks use GCC atomic builtins
> InnoDB: Compressed tables use zlib 1.2.3
> 110211 17:25:22 InnoDB: highest supported file format is Barracuda.
>
> [notice (again)]
> If you use binary log and don't use any hack of group commit,
> the binary log position seems to be:
> InnoDB: Last MySQL binlog file position 0 61013, file name
> /var/log/mysql/mysql-bin.000117
>
> xtrabackup: starting shutdown with innodb_fast_shutdown = 1
> 110211 17:25:22 InnoDB: Starting shutdown...
> 110211 17:25:22 InnoDB: Shutdown completed; log sequence number
> 4960382476
>
>
> Expected behavior: bail out when invalid arguments are added.
>
>
>

--
Peter Zaitsev, CEO, Percona Inc.
Tel: +1 888 401 3401 ext 501 Skype: peter_zaitsev
24/7 Emergency Line +1 888 401 3401 ext 911

Join us for Percona Live Event, San Francisco, Feb 16
http://www.percona.com/events/percona-live-san-francisco-2011/

Vadim Tkachenko (vadim-tk) wrote :

Peter,

I am afraid it is almost full list of mysqld options, which is about 150 names.

On Fri, Feb 11, 2011 at 12:24 PM, Peter Zaitsev <email address hidden> wrote:
> Alexey,
>
> Should not we build a list of options which it needs to ignore ?
>
> On Fri, Feb 11, 2011 at 8:47 AM, Alexey Kopytov
> <email address hidden>wrote:
>
>> I hit this bug too, but then realized it is an intended behavior. Since
>> xtrabackup reads some of its options from my.cnf, it has to ignore all
>> options it doesn't know.
>>
>> On the other hand, silently ignoring typos in command line options is
>> definitely not a good idea for a backup utility. I am not sure how this
>> can be resolved. One idea is to parse my.cnf with a custom parser rather
>> than my_getopt.
>>
>> --
>> You received this bug notification because you are a member of Percona
>> developers, which is the registrant for Percona-XtraBackup.
>> https://bugs.launchpad.net/bugs/717256
>>
>> Title:
>>  xtrabackup arguments are not validated
>>
>> Status in Open source backup tool for InnoDB and XtraDB:
>>  New
>>
>> Bug description:
>>  Have a look at the arguments I used. A typo in the arguments can cause
>> wrong things to happen (--incremental-dir in this case)
>>  root@lesktop:/data/backups# xtrabackup --prepare --apply-log-only
>> --target-dir=/data/backups/base_xtrabackup/
>> --incremaaaal-dir=/data/backups/inc1/ --i-can-add --whatever-i-want
>>  xtrabackup  Ver 1.4 Rev 193 for 5.1.47 unknown-linux-gnu (x86_64)
>>  xtrabackup: cd to /data/backups/base_xtrabackup/
>>  xtrabackup: This target seems to be already prepared.
>>  xtrabackup: notice: xtrabackup_logfile was already used to '--prepare'.
>>  xtrabackup: Temporary instance for recovery is set as followings.
>>  xtrabackup:   innodb_data_home_dir = ./
>>  xtrabackup:   innodb_data_file_path = ibdata1:10M:autoextend
>>  xtrabackup:   innodb_log_group_home_dir = ./
>>  xtrabackup:   innodb_log_files_in_group = 2
>>  xtrabackup:   innodb_log_file_size = 5242880
>>  xtrabackup: Starting InnoDB instance for recovery.
>>  xtrabackup: Using 104857600 bytes for buffer pool (set by --use-memory
>> parameter)
>>  InnoDB: The InnoDB memory heap is disabled
>>  InnoDB: Mutexes and rw_locks use GCC atomic builtins
>>  InnoDB: Compressed tables use zlib 1.2.3
>>  110211 17:25:22  InnoDB: highest supported file format is Barracuda

Alexey Kopytov (akopytov) wrote :

Peter,

On 11.02.11 23:24, Peter Zaitsev wrote:
> Should not we build a list of options which it needs to ignore ?
>

That would be a list of all server options except a few ones that are
recognized by xtrabackup. And that list would have to be updated
whenever a new server option is added. Looks like a fragile solution to me.

Peter Zaitsev (pz-percona) wrote :
Download full text (4.8 KiB)

Yes... I understand. Though it can be generated automatically is not it ?

On Fri, Feb 11, 2011 at 12:34 PM, Vadim Tkachenko <email address hidden> wrote:

> Peter,
>
> I am afraid it is almost full list of mysqld options, which is about 150
> names.
>
>
> On Fri, Feb 11, 2011 at 12:24 PM, Peter Zaitsev <email address hidden> wrote:
> > Alexey,
> >
> > Should not we build a list of options which it needs to ignore ?
> >
> > On Fri, Feb 11, 2011 at 8:47 AM, Alexey Kopytov
> > <email address hidden>wrote:
> >
> >> I hit this bug too, but then realized it is an intended behavior. Since
> >> xtrabackup reads some of its options from my.cnf, it has to ignore all
> >> options it doesn't know.
> >>
> >> On the other hand, silently ignoring typos in command line options is
> >> definitely not a good idea for a backup utility. I am not sure how this
> >> can be resolved. One idea is to parse my.cnf with a custom parser rather
> >> than my_getopt.
> >>
> >> --
> >> You received this bug notification because you are a member of Percona
> >> developers, which is the registrant for Percona-XtraBackup.
> >> https://bugs.launchpad.net/bugs/717256
> >>
> >> Title:
> >> xtrabackup arguments are not validated
> >>
> >> Status in Open source backup tool for InnoDB and XtraDB:
> >> New
> >>
> >> Bug description:
> >> Have a look at the arguments I used. A typo in the arguments can cause
> >> wrong things to happen (--incremental-dir in this case)
> >> root@lesktop:/data/backups# xtrabackup --prepare --apply-log-only
> >> --target-dir=/data/backups/base_xtrabackup/
> >> --incremaaaal-dir=/data/backups/inc1/ --i-can-add --whatever-i-want
> >> xtrabackup Ver 1.4 Rev 193 for 5.1.47 unknown-linux-gnu (x86_64)
> >> xtrabackup: cd to /data/backups/base_xtrabackup/
> >> xtrabackup: This target seems to be already prepared.
> >> xtrabackup: notice: xtrabackup_logfile was already used to '--prepare'.
> >> xtrabackup: Temporary instance for recovery is set as followings.
> >> xtrabackup: innodb_data_home_dir = ./
> >> xtrabackup: innodb_data_file_path = ibdata1:10M:autoextend
> >> xtrabackup: innodb_log_group_home_dir = ./
> >> xtrabackup: innodb_log_files_in_group = 2
> >> xtrabackup: innodb_log_file_size = 5242880
> >> xtrabackup: Starting InnoDB instance for recovery.
> >> xtrabackup: Using 104857600 bytes for buffer pool (set by --use-memory
> >> parameter)
> >> InnoDB: The InnoDB memory heap is disabled
> >> InnoDB: Mutexes and rw_locks use GCC atomic builtins
> >> InnoDB: Compressed tables use zlib 1.2.3
> >> 110211 17:25:22 InnoDB: highest supported file format is Barracuda
>
> --
> You received this bug notification because you are a member of Percona
> developers, which is the registrant for Percona-XtraBackup.
> https://bugs.launchpad.net/bugs/717256
>
> Title:
> xtrabackup arguments are not validated
>
> Status in Open source backup tool for InnoDB and XtraDB:
> New
>
> Bug description:
> Have a look at the arguments I used. A typo in the arguments can cause
> wrong things to happen (--incremental-dir in this case)
> root@lesktop:/data/backups# xtrabackup --prepare --apply-log-only
> --target-dir=/data/backups/base_xtrabackup/
...

Read more...

Peter Zaitsev (pz-percona) wrote :

Alexey,

Yes... At the same time complaining about valid option looks better than
doing absolutely wrong stuff
and corrupting 1TB backup because of Typo and requiring hours to recover :)

On Fri, Feb 11, 2011 at 12:38 PM, Alexey Kopytov <<email address hidden>
> wrote:

> Peter,
>
> On 11.02.11 23:24, Peter Zaitsev wrote:
> > Should not we build a list of options which it needs to ignore ?
> >
>
> That would be a list of all server options except a few ones that are
> recognized by xtrabackup. And that list would have to be updated
> whenever a new server option is added. Looks like a fragile solution to me.
>
> --
> You received this bug notification because you are a member of Percona
> developers, which is the registrant for Percona-XtraBackup.
> https://bugs.launchpad.net/bugs/717256
>
> Title:
> xtrabackup arguments are not validated
>
> Status in Open source backup tool for InnoDB and XtraDB:
> New
>
> Bug description:
> Have a look at the arguments I used. A typo in the arguments can cause
> wrong things to happen (--incremental-dir in this case)
> root@lesktop:/data/backups# xtrabackup --prepare --apply-log-only
> --target-dir=/data/backups/base_xtrabackup/
> --incremaaaal-dir=/data/backups/inc1/ --i-can-add --whatever-i-want
> xtrabackup Ver 1.4 Rev 193 for 5.1.47 unknown-linux-gnu (x86_64)
> xtrabackup: cd to /data/backups/base_xtrabackup/
> xtrabackup: This target seems to be already prepared.
> xtrabackup: notice: xtrabackup_logfile was already used to '--prepare'.
> xtrabackup: Temporary instance for recovery is set as followings.
> xtrabackup: innodb_data_home_dir = ./
> xtrabackup: innodb_data_file_path = ibdata1:10M:autoextend
> xtrabackup: innodb_log_group_home_dir = ./
> xtrabackup: innodb_log_files_in_group = 2
> xtrabackup: innodb_log_file_size = 5242880
> xtrabackup: Starting InnoDB instance for recovery.
> xtrabackup: Using 104857600 bytes for buffer pool (set by --use-memory
> parameter)
> InnoDB: The InnoDB memory heap is disabled
> InnoDB: Mutexes and rw_locks use GCC atomic builtins
> InnoDB: Compressed tables use zlib 1.2.3
> 110211 17:25:22 InnoDB: highest supported file format is Barracuda.
>
> [notice (again)]
> If you use binary log and don't use any hack of group commit,
> the binary log position seems to be:
> InnoDB: Last MySQL binlog file position 0 61013, file name
> /var/log/mysql/mysql-bin.000117
>
> xtrabackup: starting shutdown with innodb_fast_shutdown = 1
> 110211 17:25:22 InnoDB: Starting shutdown...
> 110211 17:25:22 InnoDB: Shutdown completed; log sequence number
> 4960382476
>
>
> Expected behavior: bail out when invalid arguments are added.
>
>
>

--
Peter Zaitsev, CEO, Percona Inc.
Tel: +1 888 401 3401 ext 501 Skype: peter_zaitsev
24/7 Emergency Line +1 888 401 3401 ext 911

Join us for Percona Live Event, San Francisco, Feb 16
http://www.percona.com/events/percona-live-san-francisco-2011/

Alexey Kopytov (akopytov) wrote :

What about parsing the xtrabackup and mysqld options separately, in the way it's currently done in innobackupex? This way we can ignore all the unknown options from the [mysqld] section in my.cnf, but don't allow typos in the command line or the [xtrabackup] section in my.cnf.

Peter Zaitsev (pz-percona) wrote :

Alexey,

I think this means it will complicate use for the users.

Right now you do not need any configuration with xtrabackup in most cases ;
you have your server and you just back it up

If you require duplication of the options this means what configuration has
to be adjusted before it can run.

On Sat, Feb 12, 2011 at 1:43 AM, Alexey Kopytov
<email address hidden>wrote:

> What about parsing the xtrabackup and mysqld options separately, in the
> way it's currently done in innobackupex? This way we can ignore all the
> unknown options from the [mysqld] section in my.cnf, but don't allow
> typos in the command line or the [xtrabackup] section in my.cnf.
>
> --
> You received this bug notification because you are a member of Percona
> developers, which is the registrant for Percona-XtraBackup.
> https://bugs.launchpad.net/bugs/717256
>
> Title:
> xtrabackup arguments are not validated
>
> Status in Open source backup tool for InnoDB and XtraDB:
> New
>
> Bug description:
> Have a look at the arguments I used. A typo in the arguments can cause
> wrong things to happen (--incremental-dir in this case)
> root@lesktop:/data/backups# xtrabackup --prepare --apply-log-only
> --target-dir=/data/backups/base_xtrabackup/
> --incremaaaal-dir=/data/backups/inc1/ --i-can-add --whatever-i-want
> xtrabackup Ver 1.4 Rev 193 for 5.1.47 unknown-linux-gnu (x86_64)
> xtrabackup: cd to /data/backups/base_xtrabackup/
> xtrabackup: This target seems to be already prepared.
> xtrabackup: notice: xtrabackup_logfile was already used to '--prepare'.
> xtrabackup: Temporary instance for recovery is set as followings.
> xtrabackup: innodb_data_home_dir = ./
> xtrabackup: innodb_data_file_path = ibdata1:10M:autoextend
> xtrabackup: innodb_log_group_home_dir = ./
> xtrabackup: innodb_log_files_in_group = 2
> xtrabackup: innodb_log_file_size = 5242880
> xtrabackup: Starting InnoDB instance for recovery.
> xtrabackup: Using 104857600 bytes for buffer pool (set by --use-memory
> parameter)
> InnoDB: The InnoDB memory heap is disabled
> InnoDB: Mutexes and rw_locks use GCC atomic builtins
> InnoDB: Compressed tables use zlib 1.2.3
> 110211 17:25:22 InnoDB: highest supported file format is Barracuda.
>
> [notice (again)]
> If you use binary log and don't use any hack of group commit,
> the binary log position seems to be:
> InnoDB: Last MySQL binlog file position 0 61013, file name
> /var/log/mysql/mysql-bin.000117
>
> xtrabackup: starting shutdown with innodb_fast_shutdown = 1
> 110211 17:25:22 InnoDB: Starting shutdown...
> 110211 17:25:22 InnoDB: Shutdown completed; log sequence number
> 4960382476
>
>
> Expected behavior: bail out when invalid arguments are added.
>
>
>

--
Peter Zaitsev, CEO, Percona Inc.
Tel: +1 888 401 3401 ext 501 Skype: peter_zaitsev
24/7 Emergency Line +1 888 401 3401 ext 911

Join us for Percona Live Event, San Francisco, Feb 16
http://www.percona.com/events/percona-live-san-francisco-2011/

Alexey Kopytov (akopytov) wrote :

Peter,

I think you misunderstood me, I'm not suggesting to duplicate the options.

The idea is to process the [mysqld] section of my.cnf differently than other sources. I think we can ignore unrecognized options in that section (and thus, skip all server-specific options), but fail with an error when reading the options from the command line or from the [xtrabackup] section (and thus disallow typos in xtrabackup-specific options). Does this sound acceptable?

Peter Zaitsev (pz-percona) wrote :

Alexey,

Yes. This is great idea indeed !

On Sat, Feb 12, 2011 at 9:28 AM, Alexey Kopytov
<email address hidden>wrote:

> Peter,
>
> I think you misunderstood me, I'm not suggesting to duplicate the
> options.
>
> The idea is to process the [mysqld] section of my.cnf differently than
> other sources. I think we can ignore unrecognized options in that
> section (and thus, skip all server-specific options), but fail with an
> error when reading the options from the command line or from the
> [xtrabackup] section (and thus disallow typos in xtrabackup-specific
> options). Does this sound acceptable?
>
>
>

--
Peter Zaitsev, CEO, Percona Inc.
Tel: +1 888 401 3401 ext 501 Skype: peter_zaitsev
24/7 Emergency Line +1 888 401 3401 ext 911

Join us for Percona Live Event, San Francisco, Feb 16
http://www.percona.com/events/percona-live-san-francisco-2011/

Changed in percona-xtrabackup:
status: New → Confirmed
assignee: Valentine Gostev (core-longbow) → Alexey Kopytov (akopytov)
importance: Undecided → High
Stewart Smith (stewart) wrote :

Agree is important, and would be great to have this implemented, but marking as Medium.

Changed in percona-xtrabackup:
importance: High → Medium
status: Confirmed → Triaged
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers