Use SCRIPT_NAME instead of REQUEST_URI to check path (#585) (#589)

The script is currently checking if the `REQUEST_URI` is containing
`wp-comments-post.php`, the default script to handle the submission
of a comment. Some security plugins have options to rename this file
to disguise that WordPress is used.

With this fix, the `SCRIPT_NAME` is used instead. Since many security
plugins do use rewrite rules, while the `REQUEST_URI` value is changed,
the `SCRIPT_NAME` value stays the same. Therefor the condition would
still recognize if a comment was submitted.

Original fix by @2ndkauboy in #589, adapted to v3.

Author: stklcode

src/Handlers/Comment.php

@@ -56,7 +56,7 @@ public static function process( $comment ) {
 
 		$comment['comment_author_IP'] = IpHelper::get_client_ip();
 
-		$request_uri  = isset( $_SERVER['REQUEST_URI'] ) ? esc_url_raw( wp_unslash( $_SERVER['REQUEST_URI'] ) ) : null;
+		$request_uri  = isset( $_SERVER['SCRIPT_NAME'] ) ? esc_url_raw( wp_unslash( $_SERVER['SCRIPT_NAME'] ) ) : null;
 		$request_path = DataHelper::parse_url( $request_uri, 'path' );
 
 		if ( empty( $request_path ) ) {

src/Rules/Honeypot.php

@@ -81,7 +81,7 @@ public static function precheck() {
 			return;
 		}
 
-		$request_uri  = Settings::get_key( $_SERVER, 'REQUEST_URI' );
+		$request_uri  = Settings::get_key( $_SERVER, 'SCRIPT_NAME' );
 		$request_path = DataHelper::parse_url( $request_uri, 'path' );
 
 		if ( strpos( $request_path, 'wp-comments-post.php' ) === false ) {

tests/Unit/Handlers/CommentTest.php

@@ -0,0 +1,70 @@
+<?php
+
+namespace AntispamBee\Tests\Unit\Handlers;
+
+use AntispamBee\Handlers\Comment;
+use AntispamBee\Handlers\Reaction;
+use Yoast\WPTestUtils\BrainMonkey\TestCase;
+
+use function Brain\Monkey\Functions\stubs;
+/**
+ * Unit tests for {@see Comment}.
+ */
+class CommentTest extends TestCase {
+
+	public function test_process() {
+		global $_POST;
+		global $_SERVER;
+
+		$_POST   = null;
+		$_SERVER = [
+			'HTTP_CLIENT_IP' => '192.0.2.100',
+			'SCRIPT_NAME'    => '/index.php'
+		];
+
+		stubs(
+			[
+				'esc_url_raw'  => function (string $url) {
+					return $url;
+				},
+				'wp_parse_url' => 'parse_url',
+				'wp_unslash'   => function ($value) {
+					return $value;
+				},
+			]
+		);
+
+		$processed = [];
+		mock('overload:' . Reaction::class )
+			->expects( 'process' )
+			->withArgs( function( $input ) use ( &$processed ) {
+				$processed[] = $input;
+				return true;
+			} );
+
+		$comment = [ 'comment_type' => 'comment' ];
+
+		$result = Comment::process( $comment );
+		self::assertSame( '192.0.2.100', $result['comment_author_IP'], 'Unexpected author IP on index.php' );
+		self::assertEmpty( $processed, 'Comment should no have been processed on index.php' );
+
+		$_SERVER['SCRIPT_NAME'] = '';
+		$result = Comment::process( $comment );
+		self::assertSame( '192.0.2.100', $result['comment_author_IP'], 'Unexpected author IP on invalid request' );
+		self::assertSame( 1, $result['ab_spam__invalid_request'], 'Invalid request not detected' );
+		self::assertEmpty( $processed, 'Comment should no have been processed on invalid request' );
+
+		$_SERVER['SCRIPT_NAME'] = '/wp-comments-post.php';
+		$result = Comment::process( $comment );
+		self::assertSame( '192.0.2.100', $result['comment_author_IP'], 'Unexpected author IP on invalid request' );
+		self::assertArrayNotHasKey( 'processed', $result, 'Comment should no have been processed without POST data' );
+
+		$_POST = 'test me';
+		$result = Comment::process( $comment );
+		self::assertSame( [ $result ], $processed, 'Comment was not processed' );
+
+		$comment = [ 'comment_type' => 'linkback' ];
+		$result = Comment::process( $comment );
+		self::assertSame( $comment, $result, 'Linkback should not be modified by comment handler' );
+	}
+}

tests/Unit/Rules/HoneypotTest.php

@@ -4,6 +4,8 @@
 
 use AntispamBee\Rules\Honeypot;
 
+use function Brain\Monkey\Functions\stubs;
+
 /**
  * Unit tests for {@see Honeypot}.
  *
@@ -35,4 +37,59 @@ public function test_init() {
 			'comment_form_field_comment filter was not added'
 		);
 	}
+
+	public function test_precheck() {
+		global $_POST;
+		global $_SERVER;
+
+		stubs(
+			[
+				'esc_url_raw'  => function (string $url) {
+					return $url;
+				},
+				'is_feed'      => false,
+				'is_trackback' => false,
+				'wp_parse_url' => 'parse_url',
+				'wp_unslash'   => function ($value) {
+					return $value;
+				},
+			]
+		);
+		mock( 'overload:' . \AntispamBee\Helpers\Honeypot::class )
+			->expects( 'get_secret_name_for_post' )
+			->andReturns( 'my-secret' );
+
+		$_POST   = [];
+		$_SERVER = [ 'SCRIPT_NAME' => '/index.php' ];
+
+		Honeypot::precheck();
+		self::assertEmpty( $_POST, 'Empty POST data modified unexpectedly' );
+
+		$_POST = [ 'foo' => 'bar' ];
+		Honeypot::precheck();
+		self::assertSame( ['foo' => 'bar' ], $_POST, 'POST data modified on index.php' );
+
+		$_SERVER = [ 'SCRIPT_NAME' => '/wp-comments-post.php' ];
+		Honeypot::precheck();
+		self::assertSame( 1, $_POST[ 'ab_spam__invalid_request' ], 'request without missing fields not detected' );
+
+		$_POST = [
+			'my-secret' => 'S3cr3t',
+			'my-hidden' => 'H1dd3n',
+		];
+		Honeypot::precheck();
+		self::assertSame( 1, $_POST[ 'ab_spam__hidden_field' ], 'non-empty hidden fiend not detected' );
+
+		$_POST = [
+			'my-secret' => 'S3cr3t',
+			'my-hidden' => '',
+		];
+		Honeypot::precheck();
+		self::assertSame(
+			[ 'my-hidden' => 'S3cr3t' ],
+			$_POST,
+			'secret was not moved to hidden field'
+		);
+
+	}
 }