Re: [interchange] Refactor robot testing into an internal routine

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