From 56e6117afd49a1b97b801b9d27b03d703ede793d Mon Sep 17 00:00:00 2001
From: Daimona Eaytoy <daimona.wiki@gmail.com>
Date: Sun, 14 Jul 2019 11:46:12 +0200
Subject: [PATCH] Initialize user-defined variables during shortcircuit

Bug: T214674
Depends-On: I5a14d4b2bc3ffd9caaaa095f16f36b9b6009db05
Change-Id: Id85c673337fa90a3782fd22eb9690cd996967111
---
 includes/parser/AbuseFilterCachingParser.php | 34 ++++++++++++++++++++
 includes/parser/AbuseFilterParser.php        | 17 ++++++++++
 tests/phpunit/AbuseFilterParserTest.php      | 22 +++++++++++++
 3 files changed, 73 insertions(+)

diff --git a/includes/parser/AbuseFilterCachingParser.php b/includes/parser/AbuseFilterCachingParser.php
index 7c98cf690..c9951a7c2 100644
--- a/includes/parser/AbuseFilterCachingParser.php
+++ b/includes/parser/AbuseFilterCachingParser.php
@@ -155,6 +155,11 @@ class AbuseFilterCachingParser extends AbuseFilterParser {
 				list( $array, $offset ) = $node->children;
 
 				$array = $this->evalNode( $array );
+
+				if ( $array->getType() === AFPData::DNONE ) {
+					return new AFPData( AFPData::DNONE );
+				}
+
 				if ( $array->getType() !== AFPData::DARRAY ) {
 					throw new AFPUserVisibleException( 'notarray', $node->position, [] );
 				}
@@ -235,6 +240,9 @@ class AbuseFilterCachingParser extends AbuseFilterParser {
 				$value = $leftOperand->toBool();
 				// Short-circuit.
 				if ( ( !$value && $op === '&' ) || ( $value && $op === '|' ) ) {
+					if ( $rightOperand instanceof AFPTreeNode ) {
+						$this->discardNode( $rightOperand );
+					}
 					return $leftOperand;
 				}
 				$rightOperand = $this->evalNode( $rightOperand );
@@ -305,4 +313,30 @@ class AbuseFilterCachingParser extends AbuseFilterParser {
 				// @codeCoverageIgnoreEnd
 		}
 	}
+
+	/**
+	 * Intended to be used for short-circuit. Given a node, check it and its children; if there are
+	 * assignments, execute them. T214674
+	 *
+	 * @param AFPTreeNode $node
+	 */
+	private function discardNode( AFPTreeNode $node ) {
+		if ( $node->type === AFPTreeNode::ASSIGNMENT ) {
+			$this->setUserVariable( $node->children[0], new AFPData( AFPData::DNONE ) );
+		} elseif ( $node->type === AFPTreeNode::INDEX_ASSIGNMENT ) {
+			$varName = $node->children[0];
+			if ( !$this->mVariables->varIsSet( $varName ) ) {
+				throw new AFPUserVisibleException( 'unrecognisedvar', $node->position, [ $varName ] );
+			}
+			$this->setUserVariable( $varName, new AFPData( AFPData::DNONE ) );
+		} elseif ( $node->type === AFPTreeNode::ATOM ) {
+			return;
+		}
+		// @phan-suppress-next-line PhanTypeSuspiciousNonTraversableForeach ATOM case excluded above
+		foreach ( $node->children as $child ) {
+			if ( $child instanceof AFPTreeNode ) {
+				$this->discardNode( $child );
+			}
+		}
+	}
 }
diff --git a/includes/parser/AbuseFilterParser.php b/includes/parser/AbuseFilterParser.php
index 172773686..f67f61cb7 100644
--- a/includes/parser/AbuseFilterParser.php
+++ b/includes/parser/AbuseFilterParser.php
@@ -230,6 +230,21 @@ class AbuseFilterParser {
 				} elseif ( $this->mCur->value === ')' ) {
 					$braces--;
 				}
+			} elseif ( $this->mCur->type === AFPToken::TID ) {
+				// T214674, define variables
+				$varname = $this->mCur->value;
+				$this->move();
+				if ( $this->mCur->type === AFPToken::TOP && $this->mCur->value === ':=' ) {
+					$this->setUserVariable( $varname, new AFPData( AFPData::DNONE ) );
+				} elseif ( $this->mCur->type === AFPToken::TSQUAREBRACKET && $this->mCur->value === '[' ) {
+					if ( !$this->mVariables->varIsSet( $varname ) ) {
+						throw new AFPUserVisibleException( 'unrecognisedvar',
+							$this->mCur->pos,
+							[ $varname ]
+						);
+					}
+					$this->setUserVariable( $varname, new AFPData( AFPData::DNONE ) );
+				}
 			}
 		}
 		if ( !( $this->mCur->type === AFPToken::TBRACE && $this->mCur->value === ')' ) ) {
@@ -710,6 +725,8 @@ class AbuseFilterParser {
 						[ $idx, count( $result->getData() ) ] );
 				}
 				$result = $result->getData()[$idx];
+			} elseif ( $result->getType() === AFPData::DNONE ) {
+				$result = new AFPData( AFPData::DNONE );
 			} else {
 				throw new AFPUserVisibleException( 'notarray', $this->mCur->pos, [] );
 			}
diff --git a/tests/phpunit/AbuseFilterParserTest.php b/tests/phpunit/AbuseFilterParserTest.php
index e725e36ea..45cc2b739 100644
--- a/tests/phpunit/AbuseFilterParserTest.php
+++ b/tests/phpunit/AbuseFilterParserTest.php
@@ -704,4 +704,26 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 			yield $case => [ $case, false ];
 		}
 	}
+
+	/**
+	 * Test that code declaring a variable in a skipped brace (because of shortcircuit)
+	 * will be parsed without throwing an exception when later trying to use that var. T214674
+	 */
+	public function testVarDeclarationInSkippedBlock() {
+		$code =
+			"arr := [5]; false & (1 == 1; var := 'b'; arr[1] := 'x'; 3 < 4); var != 'b' & arr[1] != 'x'";
+
+		foreach ( self::getParsers() as $parser ) {
+			$pname = get_class( $parser );
+			try {
+				// Whether this evaluating to true makes sense is another matter.
+				$this->assertTrue(
+					$parser->parse( $code ),
+					"Parser: $pname"
+				);
+			} catch ( Exception $e ) {
+				$this->fail( "Parser: $pname\n$e" );
+			}
+		}
+	}
 }