Re^2: CGI Action call

I am trying to use JSON and sending back temporarily to an Iframe to see what is returned.

#Search Records & Return Found 3 buttons
# 0 Member ID
# 1 Last Name 
# 2 Business Name 
elsif ($action eq "updatetable_167"){
    warn("Entered updatetable_167");
    my $kind = $query->param('kind');
     my $searchterm = $query->param('searchterm');
    my @row;
    warn("searchterm = '$searchterm'");
#    $searchterm = "'" + $searchterm + "'";
    my $searchfield = "";

    if ($kind == 0) {
      $searchfield = 'user_id';
    } 
    if ($kind == 1) {
      $searchfield = 'lastname';    
    }
    if ($kind == 2) {
      $searchfield = 'business';    
    }
#               SELECT * FROM users WHERE lastname = 'DeWitt'    
    my $stmt = "SELECT * FROM users WHERE $searchfield = $searchterm";

    warn("statement =  '$stmt'");
    
    #my $sth = $dbh->prepare ($stmt);
    my $sth = $dbh->prepare ($stmt) or die "Error Preparing:\n" . $stm
+t . "\nDBI returned: \n", $dbh->errstr;
    
    $sth->execute () or die "Unable to execute query: " . $sth->errstr
+;
    warn("Finished sub search record");

    generateResponseHash($sth);
}
[download]

If I intercept at this point and send to html (form for testing) I get back what I want in table format.

The problem is getting JSON result back and dealing with the error log entry:

Odd number of elements in hash assignment at update_tables.cgi line 626 (#1) (W misc) You specified an odd number of elements to initialize a hash, which is odd, because hashes come in key/value pairs.

sub generateResponseHash{
   my $sth = shift;
   my ($user_id, $username, $password, $pin, $position, $forename, $la
+stname, $business, $address1, $address2, $city, $state, $zip, $phone_
+home, $phone_cell, $email, $comments, $MJ, $MD, $DD, $DP) = @_;
   my $count = $sth->rows;
   $count = 0;
   
   $sth->execute () or die "Unable to execute query: " . $sth->errstr;
   
   while (my $ref = $sth->fetchrow_hashref ()){
#        #warn("generateResponseHash line 587 ref: '$ref'");
       hash_display_listing ($ref);
          ++$count;
      }
     $sth->finish ();
     completeResultHash(%hash_record);
  

}
[download]

sub hash_display_listing{
my $ref = shift;
warn("hash display_listing: '$ref'");

  %hash_record = (
    'user_id'   =>$ref->{"user_id"},
    'username'  =>$ref->{"username"},
    'password'  =>$ref->{"password"},
    'pin'       =>$ref->{"pin"},
    'position'  =>$ref->{"position"},
    'forename'  =>$ref->{"forename"},
    'lastname'  =>$ref->{"lastname"},
    'business'  =>$ref->{"business"},
    'address1'  =>$ref->{"address1"},
    'address2'  =>$ref->{"address2"},
    'city'      =>$ref->{"city"},
    'state'     =>$ref->{"state"},
    'zip'       =>$ref->{"zip"},
    'phone_home'=>$ref->{"phone_home"},
    'phone_cell'=>$ref->{"phone_cell"},
    'email'     =>$ref->{"email"},
    'comments'  =>$ref->{"comments"},
    'MJ'        =>$ref->{"MJ"},
    'MD'        =>$ref->{"MD"},
    'DD'        =>$ref->{"DD"},
    'DP'        =>$ref->{"DP"},
);

}
[download]

I get error or warning which ever it is stating:

[Mon Mar 12 12:38:34 2018] update_tables.cgi: statement =  'SELECT * F
+ROM users WHERE user_id = 15' at update_tables.cgi line 377.


Finished sub search record at update_tables.cgi line 383.


[Mon Mar 12 12:38:34 2018] update_tables.cgi: Finished sub search reco
+rd at update_tables.cgi line 383.
hash display_listing: 'HASH(0x287bd08)' at update_tables.cgi line 593.


[Mon Mar 12 12:38:34 2018] update_tables.cgi: hash display_listing: 'H
+ASH(0x287bd08)' at update_tables.cgi line 593.


Odd number of elements in hash assignment at update_tables.cgi line 62
+6 (#1)
    (W misc) You specified an odd number of elements to initialize a h
+ash,
    which is odd, because hashes come in key/value pairs.
    


[Mon Mar 12 12:38:34 2018] update_tables.cgi: Odd number of elements i
+n hash assignment at update_tables.cgi line 626.
completeResultHash line 631 at update_tables.cgi line 627.


[Mon Mar 12 12:38:34 2018] update_tables.cgi: completeResultHash line 
+631 at update_tables.cgi line 627.


after jason_str print at update_tables.cgi line 632.
[download]

from the code below:

sub completeResultHash{
  my %hash = shift;
  warn("completeResultHash line 631");

  my $json_str = encode_json(\%hash);
  print "$json_str";

  warn("after jason_str print");
  exit(0);
}
[download]

Comment on Re^2: CGI Action call Select or Download Code

Replies are listed 'Best First'.
Re^3: CGI Action call by Corion (Patriarch) on Mar 12, 2018 at 18:57 UTC
You are reading untrusted user input and use that to create an SQL statement. This is highly unsafe: `... my $searchterm = $query->param('searchterm'); ... my $stmt = "SELECT * FROM users WHERE $searchfield = $searchterm"; ... my $sth = $dbh->prepare ($stmt) or die "Error Preparing:\n" . $stmt . +"\nDBI returned: \n", $dbh->errstr; $sth->execute () or die "Unable to execute query: " . $sth->errstr +;` [download] Please read DBI and learn about placeholders. You should never interpolate user input into SQL (or likely, any other kind of textual data). You should rewrite your SQL statement and then use placeholders: `my $stmt = "SELECT * FROM users WHERE $searchfield = ?"; ... my $sth = $dbh->prepare ($stmt) or die "Error Preparing:\n" . $stm +t . "\nDBI returned: \n", $dbh->errstr; $sth->execute($searchterm) or die "Unable to execute query: " . $s +th->errstr;` [download]	[reply] [d/l] [select]
Re^4: CGI Action call by tultalk (Monk) on Mar 12, 2018 at 20:14 UTC
Hi The "user"in this case is the administrator and logged on as such. That aside your point is well taken. I don't dee that placeholders would be needed below but can you use multiple place holders as in the below code? `if ($kind == 2){ #previous record $stmt = "SELECT * FROM users WHERE user_id < (SELECT MAX(user_id) FR +OM users WHERE user_id < $rowpointer) ORDER BY user_id DESC LIMIT 1"; } if ($kind == 3){ #next record $stmt = "SELECT * FROM users WHERE user_id > (SELECT MIN(user_id) F +ROM users WHERE user_id > $rowpointer) ORDER BY user_id ASC LIMIT 1"; }` [download] I know there is better way through MySQL but not there yet	[reply] [d/l]
Re^5: CGI Action call by Corion (Patriarch) on Mar 12, 2018 at 20:25 UTC
Yes, as is documented in DBI. Also, your SQL seems faulty to me as the "previous record" seems to skip users: `create table users ( user_id integer not null primary key, name varcha +r(16) ); insert into users values (1,'Fred'); insert into users values (2,'Barney'); insert into users values (3,'Wilma'); insert into users values (4,'Betty'); insert into users values (5,'Pebbles'); SELECT * FROM users WHERE user_id < (SELECT MAX(user_id) FROM users WHERE user_id < 4) ORDER BY user_id DESC LIMIT 1` [download] (SQL fiddle) This suggests that the "previous" entry for "Betty" (user id 4) is "Barney" (user id 2), and not user id 3 as I would expect. The same, but inverse holds for the other SQL.	[reply] [d/l]
Re^6: CGI Action call by tultalk (Monk) on Mar 27, 2018 at 17:00 UTC
Re^7: CGI Action call by poj (Abbot) on Mar 27, 2018 at 20:31 UTC
Re^7: CGI Action call by Corion (Patriarch) on Mar 27, 2018 at 17:05 UTC
Some notes below your chosen depth have not been shown here
Re^6: CGI Action call by tultalk (Monk) on Mar 12, 2018 at 22:08 UTC
Re^6: CGI Action call by tultalk (Monk) on Mar 15, 2018 at 12:05 UTC
Re^7: CGI Action call by Corion (Patriarch) on Mar 15, 2018 at 12:22 UTC
Some notes below your chosen depth have not been shown here
Re^4: CGI Action call by tultalk (Monk) on Mar 17, 2018 at 02:23 UTC
Speaking of placeholders. Playing around here. Just conceptualizing. Probably been done a million times before Can something sort of like this this work for automatically selecting the fields needing updating? Run a select * using user_id returning one record and load values into the $oldxxx variables then compare to the values submitted in the update query. $placeholders; $replacement-fields; if ($password <> $oldpassword){ $placeholders = $placeholders . "?,"; $replacement-fields = $replacement-fields . "password = +'$password',"; } if ($forename <> $oldforename)){ $placeholders = $placeholders . "?,"; $replacement-fields = $replacement-fields . "forename = +'$forename',; } if ($lastname.value <> $oldlastname){ $placeholders = $placeholders . "?,"; $replacement-fields = $replacement-fields . "lastname = +'$lastname',"; } if ($business <> $oldbusiness){ $placeholders = $placeholders . "?" $replacement-fields = $replacement-fields . "business = +'$business',"; } if ($city <> $oldcity){ $placeholders = $placeholders . "?," $replacement-fields = $replacement-fields . "city = '$ci +ty',"; } if ($zip <> $oldzip){ $placeholders = $placeholders . "?," <---------------- +---------- $replacement-fields = $replacement-fields . "zip = '$zip +',"; <--- Problem with last , from last field requiring update. Pull + that off before using the two variables } $query = "UPDATE users SET $placeholders where user_id = '$ +user_id'"; $sth = $dbh->prepare($query); $sth->execute($replacement-fields) or die "Unable to execut +e query: " . $sth->errstr; [download]	[reply] [d/l]
Re^5: CGI Action call by Corion (Patriarch) on Mar 17, 2018 at 07:49 UTC
Yes you can do that. But to do that in a sensible way, please learn about the data structures that Perl offers and also read the DBI documentation. `->execute` takes a list, not a comma-separated string. So, instead of building a comma-separated string, which still has the same interpolation problems, use an array to collect the changed fields. Also, it seems you are reinventing your own ORM. You might want to take a look at DBIx::Class or Class::DBI instead.	[reply] [d/l]
Re^5: CGI Action call by poj (Abbot) on Mar 17, 2018 at 08:12 UTC
Use hash keys to avoid lots of variables. Push the field names, values onto arrays and use join to create the statement #!perl use strict; my $user_id = 1; my $old = { password => 'password', forename => 'forename', lastname => 'lastname', business => 'business', city => 'city', zip => 'zip' }; my $new = { password => 'password1', forename => 'forename', lastname => 'lastname1', business => 'business', city => 'city1', zip => 'zip1' }; my @fields; my @values; # compare old v new for (sort keys %$new){ if ($new->{$_} ne $old->{$_}){ push @fields,"$_ = ?"; push @values,$new->{$_}; } } # skip if no change if (@fields == 0){ print "No update required\n"; } else { # build sql my $fields = join ',',@fields; my $stmt = " UPDATE table SET $fields WHERE user_id = ?"; # add id push @values,$user_id; # prepare and execute sql print "$stmt\n(@values)\n"; } [download] poj	[reply] [d/l]
Re^3: CGI Action call by Corion (Patriarch) on Mar 12, 2018 at 18:13 UTC
`my %hash = shift;` [download] This can never work and likely never is what you intend it to be. See shift. Most likely, the correct code you wanted is `my %hash = @_;` [download] ... given the way you call `completeResultHash`.	[reply] [d/l] [select]
Re^4: CGI Action call by tultalk (Monk) on Mar 12, 2018 at 20:43 UTC
My undestanding: `my $a = shift; 1st item my $b = shift; 2nd item (if exits etc since it was bumped up to top by + pervious shift. my ($a, $b) = @_; my $a = $query->param('a'); my $b = $query->param('b');` [download] These are all the same.	[reply] [d/l]
Re^5: CGI Action call by Corion (Patriarch) on Mar 12, 2018 at 21:09 UTC
What you wrote above has nothing to do with the assignment of subroutine parameters. `shift` will only move one element from the parameter list. A hash will occupy multiple slots in the parameter list. You need to learn about how Perl passes parameters in subroutine calls. `param(...)` is only for CGI parameters and has no bearing on subroutine parameters.	[reply] [d/l] [select]
Re^6: CGI Action call by tultalk (Monk) on Mar 12, 2018 at 22:12 UTC
Re^3: CGI Action call by karlgoethebier (Abbot) on Mar 12, 2018 at 20:52 UTC
Or shift it like this: `#!/usr/bin/env perl use strict; use warnings; use feature qw(say); use JSON::Tiny qw(encode_json); my %hash = ( foo => 'bar', nose => 'cuke' ); say completeResultHash( \%hash ); sub completeResultHash { encode_json(shift); } __END__` [download] Regards, Karl �The Crux of the Biscuit is the Apostrophe� `perl -MCrypt::CBC -E 'say Crypt::CBC->new(-key=>'kgb',-cipher=>"Blowfish")->decrypt_hex($ENV{KARL});'`Help	[reply] [d/l] [select]
Re^3: CGI Action call by poj (Abbot) on Mar 12, 2018 at 19:06 UTC
`my $stmt = "SELECT * FROM users WHERE $searchfield = $searchterm";` [download] Can this query return more than one record ? poj	[reply] [d/l]
Re^4: CGI Action call by tultalk (Monk) on Mar 12, 2018 at 19:58 UTC
No	[reply]
Re^5: CGI Action call by poj (Abbot) on Mar 12, 2018 at 20:10 UTC
In which case instead of this `while (my $ref = $sth->fetchrow_hashref ()){ # #warn("generateResponseHash line 587 ref: '$ref'"); hash_display_listing ($ref); ++$count; } $sth->finish (); completeResultHash(%hash_record);` [download] you could just do `my $result = $sth->fetchrow_hashref(); my $json_str = encode_json($result); print $json_str; exit(0);` [download] poj	[reply] [d/l] [select]
Re^6: CGI Action call by tultalk (Monk) on Mar 13, 2018 at 02:20 UTC
Re^7: CGI Action call by poj (Abbot) on Mar 13, 2018 at 07:15 UTC
Re^7: CGI Action call by Anonymous Monk on Mar 13, 2018 at 02:44 UTC
Some notes below your chosen depth have not been shown here
Re^3: CGI Action call by tultalk (Monk) on Mar 12, 2018 at 18:44 UTC
Forgot to mention get 500 server error in the Iframe which is to receive the response	[reply]


go ahead... be a heretic
	PerlMonks