improve code quality

[StPanning]

The current code is written in a way that makes it hard to contribute:

• All modules share the same namespace,
• there are global variables, that are reused in several modules
• Almost no functions are existent

To make the code easier to maintain I suggest that:

• the code should use use warnings and use strict everywhere.
• all modules should follow the perl way to create modules with package
• most global variables are made local

As a first step I would:

• put the '*pl' files in modules/ in a separate namespace, like JoomScan/Check
• transform modules to packages
• add 'use warnings' and use 'strict'
• make the code work

I would only change the code that needs to be changed to make the code work, to keep the extensive changes to a minimum.

after this is done, I would proceed with the rest of the code.

some example: modules/robots.pl orig:

dprint("Checking robots.txt existing");
$response=$ua->get("$target/robots.txt");
my $headers  = $response->headers();
my $content_type =$headers->content_type();
if ($response->status_line =~ /200/g and $content_type =~ /text\/plain/g) {
	$source=$response->decoded_content;
	my @lines = split /\n/, $source;
	$probot="";
	foreach my $line( @lines ) { 
		if($line =~ /llow:/g){
			$between=substr($line, index($line, ': ')+2, 99999);
			$probot.="$target$between\n";
		}
	}
	tprint("robots.txt is found\npath : $target/robots.txt \n\nInteresting path found from 
        robots.txt\n$probot");
}else{
	 fprint("robots.txt is not found");
}

would become JoomScan/Check/RobotsTXT.pm:

package JoomScan::Check::RobotsTXT;
use warnings;
use strict;
use JoomScan::Logging qw(tprint dprint fprint)

sub check {
  my ($ua, $target) = @_;
  dprint("Checking robots.txt existing");
  my $response = $ua->get("$target/robots.txt");
  my $headers  = $response->headers();
  my $content_type =$headers->content_type();
  my $probot="";
  if ($response->status_line =~ /200/g and 
      $content_type =~ /text\/plain/g) {
	my $source = $response->decoded_content;
	my @lines = split /\n/, $source;
	foreach my $line( @lines ) { 
	  if($line =~ /llow:/g){
	    my $between=substr($line, index($line, ': ')+2, 99999);
	    $probot.="$target$between\n";
	  }
	}
	tprint("robots.txt is found\npath : $target/robots.txt \n\nInteresting path found from robots.txt\n$probot");
      }else{
	fprint("robots.txt is not found");
      }
}

1;

I think that code could still be improved, but I would leave that for the next round of refactoring

[Ali-Razmjoo]

Hello,

It's actually a great idea, if we want to focus on rewriting things, we may also write a cleaner code, with self-documentation. I believe it's a great idea to use perlpod too.

despite we must have a release for tomorrow #19, let's just fix simple issues for release 0.0.5 and apply this changes for 0.0.6 release.

@rezasp please review and let us know if you agree.

Best Regards.

[rezasp]

Hello,

Thank you for sharing us your great idea, I agree, I am working on a few enhancements for tomorrow, and let you know when I'm finished.

Regards.

[StPanning]

ok. Great. I will refactor the code. When I'm done I will send a PR. Since I have a day job, this may last some days

[Ali-Razmjoo]

Hey,

thanks for updating us. be in touch (Y).

regards.

[StPanning]

I have sent a PR. The change looks huge, but the code changes are little. all I did is:

• put sub {} around the exisiting code
• put that code in modules
• made global variables private
• modified joomla.pl, to make it work

I resisted the urge to fix/cleanup more. This way you can easier follow what I did.

[Ali-Razmjoo]

Hello @StPanning,

Thanks for your contribution, I will review your code after BH Arsenal Singapore. the details you shared is very useful.

Best Regards.