From e52ca00348ba609ed2b23190bc06d223d238f830 Mon Sep 17 00:00:00 2001 From: Brian Fraser Date: Wed, 22 Aug 2012 16:19:43 -0300 Subject: [PATCH] Fix for 1038276: ChangeHandler doesn't quote varchar columns with hex-looking values --- lib/ChangeHandler.pm | 17 ++++++++++++---- lib/Quoter.pm | 5 +++-- t/lib/ChangeHandler.t | 46 +++++++++++++++++++++++++++++++++++++++++-- t/lib/Quoter.t | 7 +++++-- 4 files changed, 65 insertions(+), 10 deletions(-) diff --git a/lib/ChangeHandler.pm b/lib/ChangeHandler.pm index da0516ff..d72cb4ae 100644 --- a/lib/ChangeHandler.pm +++ b/lib/ChangeHandler.pm @@ -323,10 +323,13 @@ sub make_UPDATE { else { @cols = $self->sort_cols($row); } + my $types = $self->{tbl_struct}->{type_for}; return "UPDATE $self->{dst_db_tbl} SET " . join(', ', map { + my $is_char = ($types->{$_} || '') =~ m/char|text/i; $self->{Quoter}->quote($_) - . '=' . $self->{Quoter}->quote_val($row->{$_}) + . '=' . $self->{Quoter}->quote_val($row->{$_}, + is_char => $is_char); } grep { !$in_where{$_} } @cols) . " WHERE $where LIMIT 1"; } @@ -391,11 +394,15 @@ sub make_row { else { @cols = $self->sort_cols($row); } - my $q = $self->{Quoter}; + my $q = $self->{Quoter}; + my $type_for = $self->{tbl_struct}->{type_for}; return "$verb INTO $self->{dst_db_tbl}(" . join(', ', map { $q->quote($_) } @cols) . ') VALUES (' - . join(', ', map { $q->quote_val($_) } @{$row}{@cols} ) + . join(', ', map { + my $is_char = ($type_for->{$_} || '') =~ m/char|text/i; + $q->quote_val($row->{$_}, + is_char => $is_char) } @cols ) . ')'; } @@ -413,7 +420,9 @@ sub make_where_clause { my @clauses = map { my $val = $row->{$_}; my $sep = defined $val ? '=' : ' IS '; - $self->{Quoter}->quote($_) . $sep . $self->{Quoter}->quote_val($val); + my $is_char = ($self->{tbl_struct}->{type_for}->{$_} || '') =~ m/char|text/i; + $self->{Quoter}->quote($_) . $sep . $self->{Quoter}->quote_val($val, + is_char => $is_char); } @$cols; return join(' AND ', @clauses); } diff --git a/lib/Quoter.pm b/lib/Quoter.pm index 659d420f..a9916d7f 100644 --- a/lib/Quoter.pm +++ b/lib/Quoter.pm @@ -65,11 +65,12 @@ sub quote { # Returns: # Quoted value sub quote_val { - my ( $self, $val ) = @_; + my ( $self, $val, %args ) = @_; return 'NULL' unless defined $val; # undef = NULL return "''" if $val eq ''; # blank string = '' - return $val if $val =~ m/^0x[0-9a-fA-F]+$/; # hex data + return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data + && !$args{is_char}; # unless is_char is true # Quote and return non-numeric vals. $val =~ s/(['\\])/\\$1/g; diff --git a/t/lib/ChangeHandler.t b/t/lib/ChangeHandler.t index 835fdd31..2b1ba22d 100644 --- a/t/lib/ChangeHandler.t +++ b/t/lib/ChangeHandler.t @@ -9,7 +9,7 @@ BEGIN { use strict; use warnings FATAL => 'all'; use English qw(-no_match_vars); -use Test::More tests => 33; +use Test::More; use ChangeHandler; use Quoter; @@ -468,10 +468,52 @@ is_deeply( "process_rows() appends trace msg to SQL statements" ); +# ############################################################################# +# ChangeHandler doesn't quote varchar columns with hex-looking values +# https://bugs.launchpad.net/percona-toolkit/+bug/1038276 +# ############################################################################# +SKIP: { + skip 'Cannot connect to sandbox master', 1 unless $master_dbh; + $sb->load_file('master', "t/lib/samples/bug_1038276.sql"); + + @rows = (); + $tbl_struct = { + cols => [qw(id b)], + col_posn => {id=>0, b=>1}, + type_for => {id=>'int', b=>'varchar'}, + }; + $ch = new ChangeHandler( + Quoter => $q, + left_db => 'bug_1038276', + left_tbl => 'lt', + right_db => 'bug_1038276', + right_tbl => 'rt', + actions => [ sub { push @rows, $_[0]; } ], + replace => 0, + queue => 0, + tbl_struct => $tbl_struct, + ); + $ch->fetch_back($master_dbh); + + $ch->change('UPDATE', {id=>1}, [qw(id)] ); + $ch->change('INSERT', {id=>1}, [qw(id)] ); + + is_deeply( + \@rows, + [ + "UPDATE `bug_1038276`.`rt` SET `b`='0x89504E470D0A1A0A0000000D4948445200000079000000750802000000E55AD965000000097048597300000EC300000EC301C76FA8640000200049444154789C4CBB7794246779FFBBF78F7B7EBE466177677772CE3D9D667AA67BA62776CE39545557CE3974EE9EB049AB9556392210414258083' WHERE `id`='1' LIMIT 1", + "INSERT INTO `bug_1038276`.`rt`(`id`, `b`) VALUES ('1', '0x89504E470D0A1A0A0000000D4948445200000079000000750802000000E55AD965000000097048597300000EC300000EC301C76FA8640000200049444154789C4CBB7794246779FFBBF78F7B7EBE466177677772CE3D9D667AA67BA62776CE39545557CE3974EE9EB049AB9556392210414258083')", + ], + "UPDATE and INSERT quote data regardless of how it looks if tbl_struct->quote_val is true" + ); +} + # ############################################################################# # Done. # ############################################################################# $sb->wipe_clean($master_dbh); $sb->wipe_clean($slave1_dbh); ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); -exit; + +done_testing; + \ No newline at end of file diff --git a/t/lib/Quoter.t b/t/lib/Quoter.t index a843ecb9..6e3aaaae 100644 --- a/t/lib/Quoter.t +++ b/t/lib/Quoter.t @@ -9,7 +9,7 @@ BEGIN { use strict; use warnings FATAL => 'all'; use English qw(-no_match_vars); -use Test::More tests => 55; +use Test::More; use Quoter; use PerconaTest; @@ -59,6 +59,8 @@ is( $q->quote_val('\\\''), "'\\\\\\\''", 'embedded backslash'); is( $q->quote_val('123-abc'), "'123-abc'", 'looks numeric but is string'); is( $q->quote_val('123abc'), "'123abc'", 'looks numeric but is string'); is( $q->quote_val('0x89504E470'), '0x89504E470', 'hex string'); +is( $q->quote_val('0x89504E470', is_char => 0), '0x89504E470', 'hex string, with is_char => 0'); +is( $q->quote_val('0x89504E470', is_char => 1), "'0x89504E470'", 'hex string, with is_char => 1'); is( $q->quote_val('0x89504I470'), "'0x89504I470'", 'looks like hex string'); is( $q->quote_val('eastside0x3'), "'eastside0x3'", 'looks like hex str (issue 1110'); @@ -219,4 +221,5 @@ SKIP: { # Done. # ########################################################################### ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); -exit; + +done_testing;