0

I need some insight on my Perl CGI script.

First of all all this is running under webmin so i'm doing a custom module.

I'm calling a CGI Perl script passing 2 parameter from another Perl CGI. The link I'm calling is in the following format:

http://IP:8080/foobar/alat.cgi?sysinfo=xxxxxxx&SR=yyyyyyyy

The alat.cgi script look like this

#!/usr/bin/perl
use CGI qw(:standard); 
ReadParse();
$q = new CGI;
my $dir = $in->param('SR');
my $s = $in->param('sysinfo');
ui_print_header(undef, $text{'edit_title'}.$dir, "");
print $dir."<br>";
print $s"<br>";

The only output I get printed is the value of $dir and $s seems to be empty.

What am I doing wrong?

Improve this question
5
  • 3
    use strict; use diagnostics; might be helpful. Commented Jun 5, 2015 at 7:24
  • 1
    That last line is missing an operator between $s and "<br>" ... is that a typos? Commented Jun 5, 2015 at 7:26
  • 4
    $q is not the same as $in. Commented Jun 5, 2015 at 7:27
  • Thanks, I actually had a number of silly syntax error which finally i was able to detect using the diagnostics. for the same of discussion adding here the corrected script: #!/usr/bin/perl # Run alat on selected sysinfo and allow display of output #use strict; use diagnostics; require 'recoverpoint-lib.pl'; use CGI qw(:standard); ReadParse(); my $q = new CGI; my $dir = $q->param('SR'); my $s = $q->param('sysinfo'); ui_print_header(undef, $text{'edit_title'}.$dir, ""); print $dir."<br>"; print $s."<br>"; Commented Jun 5, 2015 at 7:41
  • Don't keep use diagnostics in your code. Only put it in for very specific analysis. It is very slow and adds a load of execution time. Commented Jun 5, 2015 at 9:07

3 Answers 3

Reset to default
2

As @Сухой27 said, add use strict;, but also use warnings; to the top of your script, right below the shebang (#!/usr/bin/perl) line. Those will tell you about syntax errors and other stuff where Perl is doing something other than you might intend.

With CGI (which is btw not part of the Perl core in the latest 5.22 release any more) and the object oriented approach you are tyring to take, you don't need to use ReadParse(). That is an abomination left in from Perl 4's cgilib.pl times.

I don't know what your ui_print_header function does. I'm guessing it outputs a bunch of HTML. Are you sure you defined it?

With fixing all your syntax errors and using modern syntax, your program would look like this. I'll break down what is happening for you.

#!/usr/bin/perl
use strict;
use warnings;
use CGI;

my $q = CGI->new;
my $dir = $q->param('SR');
my $s = $q->param('sysinfo');

# you need to declare this to use it below
my %text = ( edit_title => 'foo' );

# we declare this sub further down
ui_print_header(undef, $text{'edit_title'} . $dir, q{});

print $dir . '<br />';
print $s . '<br  />';

sub ui_print_header {
  my ( $foo, $title, $dir, $bar ) = @_;

  # do stuff here...
}

Let's look at some of the things I did here.

  • Saying new CGI as the CGI docs suggest is fine, but since we are using the OOP way you can use the more common CGI->new. It's the same thing really, but it's consistent with the rest of the OOP Perl world and it's more clear that you are calling the new method on the CGI package.
  • If you have $q, keep using it. There is no $in.
  • Declare all your variables with my.
  • Declare %text so you can use $text{'edit_title'} later. Probably you imported that, or ommitted it from the code you showed us.
  • Declare ui_print_header(). See above.
  • q{} is the same as '', but it's clearer that it's an empty string.
Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

Check out SawyerX's talk CGI.pm must die!, which is extremely entertaining and shows why CGI is outdated: youtube.com/watch?v=jKOqtRMT85s - of course you can still use it, but take into account that there are way easier things around today. :)
0

thank you everyone for the very quick answer, and as I was suspecting I just had some silly mistake. Adding here the corrected code that now works

#!/usr/bin/perl
# Run alat on selected sysinfo and allow display of output
#use strict; 
use diagnostics;
require 'recoverpoint-lib.pl';
use CGI qw(:standard); 
ReadParse();
my $q = new CGI;
my $dir = $q->param('SR');
my $s = $q->param('sysinfo');

ui_print_header(undef, $text{'edit_title'}.$dir, "");
print  $dir."<br>";
print  $s."<br>";

Just to clarify for some of previous answer, this is a custom module of webmin so variable $text is imported and function ui_print_header is a webmin defined one, it basically print the page header in HTML

Improve this answer

1 Comment

use strict; should be uncommented.
-1

As you enable strict and warnings you can easily know the errors.Also you should check Apache error logs, I think the script should be like this: 

#!/usr/bin/perl
use CGI qw(:standard); 
use strict;
use warnings;
ReadParse();
my $q = new CGI;
my $dir = $q->param('SR');
my $s = $q->param('sysinfo');
ui_print_header(undef, $text{'edit_title'}.$dir, "");
print $dir."<br>";
print $s."<br>";
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.