Mailing List Archive: [interchange] Refactor robot testing into an internal routine

commit 77110c740d70c24e56b88f9f07c3973f2edaf9bb
Author: David Christensen <david@endpoint.com>
Date: Tue Jan 17 16:09:06 2017 -0600

Refactor robot testing into an internal routine

lib/Vend/Server.pm | 18 +++++++++++++-----
1 files changed, 13 insertions(+), 5 deletions(-)
---
diff --git a/lib/Vend/Server.pm b/lib/Vend/Server.pm
index 0d9c852..a03248d 100644
--- a/lib/Vend/Server.pm
+++ b/lib/Vend/Server.pm
@@ -279,10 +279,19 @@ EOF
#::logDebug("request_method=$CGI::request_method");
#::logDebug("content_type=$CGI::content_type");

+ $Vend::Robot = check_is_robot();
+
+ $CGI::values{mv_tmp_session} ||= 1 if $Vend::Robot;
+}
+
+
+sub check_is_robot {
+ my $ret = 1;
+
#::logDebug("Check robot UA=$Global::RobotUA IP=$Global::RobotIP");
if ($Global::RobotIP and $CGI::remote_addr =~ $Global::RobotIP) {
#::logDebug("It is a robot by IP!");
- $Vend::Robot = 1;
+ $ret = 1;
}
elsif ($Global::HostnameLookups && $Global::RobotHost) {
if (!$CGI::remote_host && $CGI::remote_addr) {
@@ -291,7 +300,7 @@ EOF
}
if ($CGI::remote_host && $CGI::remote_host =~ $Global::RobotHost) {
#::logDebug("It is a robot by host!");
- $Vend::Robot = 1;
+ $ret = 1;
}
}
unless ($Vend::Robot) {
@@ -300,11 +309,10 @@ EOF
}
elsif ($Global::RobotUA and $CGI::useragent =~ $Global::RobotUA) {
#::logDebug("It is a robot by UA!");
- $Vend::Robot = 1;
+ $ret = 1;
}
}
-
- $CGI::values{mv_tmp_session} ||= 1 if $Vend::Robot;
+ return $ret;
}

# This is called by parse_multipart

_______________________________________________
interchange-cvs mailing list
interchange-cvs@icdevgroup.org
http://www.icdevgroup.org/mailman/listinfo/interchange-cvs

On 18/01/17 11:18, David Christensen wrote:
> + $Vend::Robot = check_is_robot();
> +
> + $CGI::values{mv_tmp_session} ||= 1 if $Vend::Robot;
> +}
> +
> +
> +sub check_is_robot {
> + my $ret = 1;
> +
> #::logDebug("Check robot UA=$Global::RobotUA IP=$Global::RobotIP");
> if ($Global::RobotIP and $CGI::remote_addr =~ $Global::RobotIP) {
> #::logDebug("It is a robot by IP!");
> - $Vend::Robot = 1;
> + $ret = 1;
> }
> elsif ($Global::HostnameLookups && $Global::RobotHost) {
> if (!$CGI::remote_host && $CGI::remote_addr) {
> @@ -291,7 +300,7 @@ EOF
> }
> if ($CGI::remote_host && $CGI::remote_host =~ $Global::RobotHost) {
> #::logDebug("It is a robot by host!");
> - $Vend::Robot = 1;
> + $ret = 1;
> }
> }
> unless ($Vend::Robot) {
> @@ -300,11 +309,10 @@ EOF
> }
> elsif ($Global::RobotUA and $CGI::useragent =~ $Global::RobotUA) {
> #::logDebug("It is a robot by UA!");
> - $Vend::Robot = 1;
> + $ret = 1;
> }
> }
> -
> - $CGI::values{mv_tmp_session} ||= 1 if $Vend::Robot;
> + return $ret;
> }

I see some issues with this:

1. check_is_robot() will always return 1, I think you meant to
initialize $ret to 0, not 1 at the top.

2. The previous code would not explicitly set $Vend::Robot to 0, only
to 1, so there is an incompatible case where $Vend::Robot is already set
to 1 here but check_is_robot() would return 0. You can fix this by
changing:

$Vend::Robot = check_is_robot();

...to...

$Vend::Robot ||= check_is_robot();

Peter

_______________________________________________
interchange-cvs mailing list
interchange-cvs@icdevgroup.org
http://www.icdevgroup.org/mailman/listinfo/interchange-cvs