Variables/ServerVariables: bug fix - false negatives for $GLOBALS['_SERVER']

The `$GLOBALS['_SERVER']` superglobals access is equivalent to using `$_SERVER`, so should be examined too.

Includes tests.

Author: jrfnl

WordPressVIPMinimum/Sniffs/Variables/ServerVariablesSniff.php

@@ -55,18 +55,39 @@ public function register() {
 	 */
 	public function process_token( $stackPtr ) {
 
-		if ( $this->tokens[ $stackPtr ]['content'] !== '$_SERVER' ) {
-			// Not the variable we are looking for.
+		if ( $this->tokens[ $stackPtr ]['content'] !== '$_SERVER'
+			&& $this->tokens[ $stackPtr ]['content'] !== '$GLOBALS'
+		) {
+			// Not a variable we are looking for.
 			return;
 		}
 
+		$searchStart = $stackPtr;
+		if ( $this->tokens[ $stackPtr ]['content'] === '$GLOBALS' ) {
+			$globalsIndexPtr = $this->get_array_access_key( $stackPtr );
+			if ( $globalsIndexPtr === false ) {
+				// Couldn't find an array index token usable for the purposes of this sniff. Bow out.
+				return;
+			}
+
+			$globalsIndexName = TextStrings::stripQuotes( $this->tokens[ $globalsIndexPtr ]['content'] );
+			if ( $globalsIndexName !== '_SERVER' ) {
+				// Not access to `$GLOBALS['_SERVER']`.
+				return;
+			}
+
+			// Set the start point for the next array access key search to the close bracket of this array index.
+			// No need for defensive coding as we already know there will be a valid close bracket next.
+			$searchStart = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $globalsIndexPtr + 1 ), null, true );
+		}
+
 		$prevNonEmpty = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true );
 		if ( $this->tokens[ $prevNonEmpty ]['code'] === T_DOUBLE_COLON ) {
 			// Access to OO property mirroring the name of the superglobal. Not our concern.
 			return;
 		}
 
-		$indexPtr = $this->get_array_access_key( $stackPtr );
+		$indexPtr = $this->get_array_access_key( $searchStart );
 		if ( $indexPtr === false ) {
 			// Couldn't find an array index token usable for the purposes of this sniff. Bow out as undetermined.
 			return;

WordPressVIPMinimum/Tests/Variables/ServerVariablesUnitTest.inc

@@ -61,3 +61,15 @@ echo $_SERVER[$other_key] . $NOT_SERVER['PHP_AUTH_PW'];
 
 // Safeguard the sniff doesn't get confused over partially dynamic keys.
 echo $_SERVER[$other_key . 'PHP_AUTH_PW'];
+
+// Access of the array indices via the $GLOBALS superglobal should also be recognized.
+$GLOBALS['NOT_SERVER']['PHP_AUTH_USER']; // OK.
+$GLOBALS[$a]['PHP_AUTH_USER']; // OK.
+$GLOBALS['_SERVER']['SOME_OTHER_KEY']; // OK.
+$GLOBALS['_SERVER'][$a]; // OK.
+
+$GLOBALS['_SERVER']['PHP_AUTH_USER'];
+$GLOBALS['_SERVER']['PHP_AUTH_PW'];
+$GLOBALS["_SERVER"]['HTTP_X_IP_TRAIL'];
+$GLOBALS['_SERVER']['HTTP_X_FORWARDED_FOR'];
+$GLOBALS['_SERVER']["REMOTE_ADDR"];

WordPressVIPMinimum/Tests/Variables/ServerVariablesUnitTest.php

@@ -28,6 +28,11 @@ public function getErrorList() {
 			33 => 1,
 			36 => 1,
 			37 => 1,
+			71 => 1,
+			72 => 1,
+			73 => 1,
+			74 => 1,
+			75 => 1,
 		];
 	}