Start caching sanity test failures.

With all the fallout from the introduction of flavours, we have many more sanity test failure, some with very long lists. We never cached STF before, but now we do. Since STF are neither ports nor files, but they relate only to commits, I created a new cache classe in classes/cache-general.php While here, classes/cache-port.php & classes/cache-file.php were slightly altered to use a class constant, much like classes/cache-general.php Code related to packages was removed from www/sanity_test_failures.php - I suspect it was never used and was left over from a copy/paste situation. I attempted to avoid file manipulation by using filter_var() in www/sanity_test_failures.php when accepting incoming message_id values. I see now that I am caching all results, not just successful reads from the DB. Could someone fill up the cache via that? Yes, I'm sure they could.
FreshPorts · Dec 3, 2017 · cda87bf · cda87bf
1 parent 723fb4b
commit cda87bf
Show file tree

Hide file tree

Showing 9 changed files with 49 additions and 103 deletions.
diff --git a/classes/cache-file.php b/classes/cache-file.php
@@ -11,6 +11,8 @@
 
 class CacheFile extends Cache {
 
+	const CacheCategory = 'ports';
+
 	var $PageSize = 100;
 
 	function CacheFile() {
@@ -32,7 +34,7 @@ function Retrieve($FileName, $PageNum = 1) {
 	function Add($FileName, $PageNum = 1) {
 		$this->_Log("CacheFile: Adding for $FileName");
 
-		$CacheDir = $this->CacheDir . '/ports/' . dirname($FileName);
+		$CacheDir = $this->CacheDir . '/' . CacheFile::CacheCategory . '/' . dirname($FileName);
 		$Key = $this->_FileKey($FileName, $PageNum);
 
 		if (!file_exists($CacheDir)) {
@@ -65,7 +67,7 @@ function Remove($FileName) {
 
 	function _FileKey($FileName, $PageNum = 1) {
 		// might want some parameter checking here
-		$Key = "ports/$FileName.PageSize$this->PageSize.PageNum$PageNum.html";
+		$Key = CacheFile::CacheCategory . "/$FileName.PageSize$this->PageSize.PageNum$PageNum.html";
 
 		return $Key;
 	}

diff --git a/classes/cache-general.php b/classes/cache-general.php
@@ -34,7 +34,7 @@ function Retrieve($FileName, $PageNum = 1) {
 	function Add($FileName, $PageNum = 1) {
 		$this->_Log("CacheFile: Adding for $FileName");
 
-		$CacheDir = $this->CacheDir . '/' . CacheCategory . '/' . dirname($FileName);
+		$CacheDir = $this->CacheDir . '/' . CacheGeneral::CacheCategory . '/' . dirname($FileName);
 		$Key = $this->_FileKey($FileName, $PageNum);
 
 		if (!file_exists($CacheDir)) {
@@ -67,7 +67,7 @@ function Remove($FileName) {
 
 	function _FileKey($FileName, $PageNum = 1) {
 		// might want some parameter checking here
-		$Key = CacheCategory . "/$FileName.PageSize$this->PageSize.PageNum$PageNum.html";
+		$Key = CacheGeneral::CacheCategory . "/$FileName.PageSize$this->PageSize.PageNum$PageNum.html";
 
 		return $Key;
 	}

diff --git a/classes/cache-port.php b/classes/cache-port.php
@@ -16,6 +16,8 @@
 
 class CachePort extends Cache {
 
+	const CacheCategory = 'ports';
+
 	var $PageSize = 100;
 
 	function CachePort() {
@@ -37,7 +39,7 @@ function RetrievePort($Category, $Port, $CacheType = CACHE_PORT_COMMITS, $PageNu
 	function AddPort($Category, $Port, $CacheType = CACHE_PORT_COMMITS, $PageNum = 1, $Branch = BRANCH_HEAD) {
 		$this->_Log("CachePort: Adding for $Category/$Port");
 
-		$CacheDir = $this->CacheDir . '/ports/' . $Category . '/' . $Port;
+		$CacheDir = $this->CacheDir . '/' . CachePort::CacheCategory . '/' . $Category . '/' . $Port;
 		$Key = $this->_PortKey($Category, $Port, $CacheType, $PageNum, $Branch);
 
 		if (!file_exists($CacheDir)) {
@@ -70,7 +72,7 @@ function RemovePort($Category, $Port) {
 
 	function _PortKey($Category, $Port, $CacheType, $PageNum = 1, $Branch = BRANCH_HEAD) {
 		// might want some parameter checking here
-		$Key = "ports/$Category/$Port/$CacheType.$Branch.PageSize$this->PageSize.PageNum$PageNum.html";
+		$Key = CachePort::CacheCategory . "/$Category/$Port/$CacheType.$Branch.PageSize$this->PageSize.PageNum$PageNum.html";
 
 		return $Key;
 	}

diff --git a/classes/display_commit.php b/classes/display_commit.php
@@ -30,7 +30,7 @@ class DisplayCommit {
 	var $ShowAllPorts     = FALSE;	# by default we show only the first few ports.
 	var $ShowEntireCommit = 0;		# by default we show only the first few lines of the commit message.
 
-	var $SanityTestFailure = FALSE;
+	var $ShowLinkToSanityTestFailure = FALSE;
 
 	function DisplayCommit($dbh, $result) {
 		$this->dbh    = $dbh;
@@ -60,7 +60,7 @@ function SetShowEntireCommit($ShowEntireCommit) {
 	function CreateHTML() {
 		GLOBAL	$freshports_CommitMsgMaxNumOfLinesToShow;
 
-		$Debug = 0;
+		$Debug = $this->Debug;
 
 		if (!$this->result) {
 			syslog(LOG_ERR, __FILE__ . '::' . __LINE__ . ': no result set supplied');
@@ -110,6 +110,7 @@ function CreateHTML() {
 			// OK, while we have the log change log, let's put the port details here.
 
 			if ($mycommit->commit_log_id != $PreviousCommit->commit_log_id) {
+				if ($Debug) echo "This commit_log_id is different\n";
 				if (($NumberOfPortsInThisCommit > $MaxNumberPortsToShow) && !$this->ShowAllPorts) {
 					$this->HTML .= '<BR>' . freshports_MorePortsToShow($PreviousCommit->message_id, $NumberOfPortsInThisCommit, $MaxNumberPortsToShow);
 				}
@@ -156,10 +157,10 @@ function CreateHTML() {
 
 				if ($mycommit->EncodingLosses()) {
 						$this->HTML .= '&nbsp;' . freshports_Encoding_Errors();
-					}
+				}
 
-				if ($mycommit->stf_message != '') {
-					$this->HTML .= '&nbsp; ' . freshports_SanityTestFailure_Link($mycommit->message_id);
+				if ($mycommit->stf_message != '' && $this->ShowLinkToSanityTestFailure) {
+					$this->HTML .= '&nbsp;' . freshports_SanityTestFailure_Link($mycommit->message_id);
 				}
 
 #        			echo '<pre>' . print_r($mycommit, true) . '</pre>';

diff --git a/classes/sanity_test_failures.php b/classes/sanity_test_failures.php
@@ -84,7 +84,7 @@ function CreateHTML() {
 		$DisplayCommit->SetDaysMarkedAsNew($this->DaysMarkedAsNew);
 		$DisplayCommit->SetUserID($this->UserID);
 		$DisplayCommit->SetWatchListAsk($this->WatchListAsk);
-		$DisplayCommit->SanityTestFailure = true;
+		$DisplayCommit->ShowLinkToSanityTestFailure = false;
 
 		$RetVal = $DisplayCommit->CreateHTML();
 

diff --git a/www/commit.php b/www/commit.php
@@ -233,7 +233,7 @@ function freshports_CommitNextPreviousPage($URL, $NumRowsTotal, $PageNo, $PageSi
 			$DisplayCommit->Debug = $Debug;
 			$DisplayCommit->SetShowAllPorts(true);
 			$DisplayCommit->SetShowEntireCommit(true);
-			$DisplayCommit->SanityTestFailure = true;
+			$DisplayCommit->ShowLinkToSanityTestFailure = true;
 			$RetVal = $DisplayCommit->CreateHTML();
 
 			echo $DisplayCommit->HTML;

diff --git a/www/date.php b/www/date.php
@@ -137,7 +137,7 @@ function ArchiveCreate($Date, $DateMessage, $db, $User, $BranchName) {
 		require_once($_SERVER['DOCUMENT_ROOT'] . '/../classes/display_commit.php');
 
 		$DisplayCommit = new DisplayCommit($db, $commits->LocalResult);
-		$DisplayCommit->SanityTestFailure = true;
+		$DisplayCommit->ShowLinkToSanityTestFailure = true;
 		$RetVal = $DisplayCommit->CreateHTML();
 
 		$HTML = $DisplayCommit->HTML;

diff --git a/www/index.php b/www/index.php
@@ -176,7 +176,7 @@ function freshports_SummaryForDay($MinusN) {
 		$LatestCommits->FetchLimit(date('Y-m-d'), isset($User) ? $User->id : null, 100);
 
 		$DisplayCommit = new DisplayCommit($db, $LatestCommits->LocalResult);
-		$DisplayCommit->SanityTestFailure = true;
+		$DisplayCommit->ShowLinkToSanityTestFailure = true;
 		$RetVal = $DisplayCommit->CreateHTML();
 
 		echo $DisplayCommit->HTML;

diff --git a/www/sanity_test_failures.php b/www/sanity_test_failures.php
@@ -11,54 +11,27 @@
 
 	require_once($_SERVER['DOCUMENT_ROOT'] . '/../include/getvalues.php');
 	require_once($_SERVER['DOCUMENT_ROOT'] . '/../classes/cache-general.php');
+	require_once($_SERVER['DOCUMENT_ROOT'] . '/../classes/sanity_test_failure.php');  # for fetching the message for one commit
 	require_once($_SERVER['DOCUMENT_ROOT'] . '/../classes/sanity_test_failures.php');
 	require_once('Pager/Pager.php');
 
-	define('CACHE_NAME', 'sanity_test_failures');
 
 	$message_id = '';
-#	phpinfo();
-	if (IsSet($_REQUEST['message_id'])) $message_id = pg_escape_string($_REQUEST['message_id']);
+	if (IsSet($_REQUEST['message_id'])) {
+		$message_id = pg_escape_string($_REQUEST['message_id']);
+		# avoid path manipulation e.g. ../../
+		# this will come into play when we fetch / save the cached file
+		$message_id = filter_var($message_id, FILTER_SANITIZE_EMAIL);
+		define('CACHE_NAME', "sanity_test_failures.$message_id" );
+	} else {
+		define('CACHE_NAME', 'sanity_test_failures');
+	}
 
-	#
-	# If they supply a package name, go for it.
-	#
-	if (IsSet($_REQUEST['package'])) {
-		$package = pg_escape_string($_REQUEST['package']);
-		if ($package != '') {
-			require_once($_SERVER['DOCUMENT_ROOT'] . '/../classes/packages.php');
-
-			$Packages = new Packages($db);
-
-			$CategoryPort = $Packages->GetCategoryPortFromPackageName($package);
-			switch ($CategoryPort) {
-				case "0":
-					# no such port found
-					header('Location: /package.php?package=' . $package . '&notfound');
-					exit;
-
-				case "-1":
-					# multiple ports have that package name
-					# search for them all and let the users decide which one they want
-					require_once($_SERVER['DOCUMENT_ROOT'] . '/../classes/searches.php');
-					/* $dbh used here, but I see $db elsewhere in this file */
-					$Searches = new Searches($dbh);
-					$Redirect = $Searches->GetLink($package, FRESHPORTS_SEARCH_METHOD_Exact, 1);
-					header('Location: ' . $Redirect);
-					exit;
-
-				default:
-					# one port found with that name, show that page.
-					header('Location: /' . $CategoryPort . '/');
-					exit;
-			}
-		}
-	} /* package */
 
 	freshports_Start($FreshPortsSlogan,
 					$FreshPortsName . ' - new ports, applications',
 					'FreeBSD, index, applications, ports');
-$Debug = 1;
+$Debug = 0;
 
 if ($Debug) echo "\$User->id='$User->id'";
 
@@ -160,28 +133,12 @@ function freshports_SummaryForDay($MinusN) {
 		}
 	}
 
-	if ($message_id == '') {
-
-	if ($Debug) echo 'we do not have a message_id';
-
 	$UseCache = FALSE;
 
 	$Cache = new CacheGeneral();
 	$Cache->PageSize = $User->page_size;
 	$result = $Cache->Retrieve(CACHE_NAME, $PageNumber);
 
-#	DEFINE('CACHEFILE', PAGES_DIRECTORY . '/sanity_test_failures.html');
-#
-#	if ($User->id == '') {
-#		if (file_exists(CACHEFILE) && is_readable(CACHEFILE)) {
-#			$UseCache = TRUE;
-#		}
-#	}
-#
-#	if ($UseCache) {
-#		readfile(CACHEFILE);
-#	} else {
-
 	if (!$result)  {
 		if ($Debug) echo "found something from the cache<br>\n";
 		$HTML = $Cache->CacheDataGet();
@@ -196,50 +153,34 @@ function freshports_SummaryForDay($MinusN) {
 		$SanityTestFailures->SetDaysMarkedAsNew ($DaysMarkedAsNew);
 		$SanityTestFailures->SetUserID($User->id);
 		$SanityTestFailures->SetWatchListAsk($User->watch_list_add_remove);
+
 		if ($message_id != '') {
 			$SanityTestFailures->SetMessageID($message_id);
 		}
 		$SanityTestFailures->CreateHTML();
+		$HTML .= $SanityTestFailures->HTML;
 
-		echo $SanityTestFailures->HTML;
+		if ($message_id != '') {
+			# when displaying for one commit, we need to pull the sanity test message out and display it.
+			# when displaying all sanity test failures, we do not display the failure messages, but instead, show a list
+			# commits, each with a link to show the message.
 
-		$Cache->CacheDataSet($SanityTestFailures->HTML);
-		 if ($message_id != '') {
-		$Cache->Add(CACHE_NAME, $PageNumber);
-	} /* no result */
-	} /* for no message id */
-	if ($message_id != '') {
-		if ($Debug) echo 'we do have a message_id';
-		echo '<tr><td>';
- 		require_once($_SERVER['DOCUMENT_ROOT'] . '/../classes/sanity_test_failure.php');
-
-		echo "\n<h2>Sanity Test Results</h2>\n";
-		echo "\n<blockquote>\n<pre>";
-
-		$Cache = new CacheGeneral();
-		$Cache->PageSize = $User->page_size;
-		$FileName = "sanity_test_failure.$message_id";
-		$result = $Cache->Retrieve($FileName, $PageNumber);
-		if (!$result)  {
-			if ($Debug) echo "found something from the cache<br>\n";
-			$HTML = $Cache->CacheDataGet();
-			echo $HTML;
-		} else {
-			if ($Debug) echo "DID NOT find anything in the cache<br>\n";
+			# note also, the object we create below is singular, whereas the object we created above is plural.
+			$HTML .= '<tr><td>';
 			$SanityTestFailure = new SanityTestFailure($db);
 			if ($SanityTestFailure->FetchByMessageID($message_id) != -1) {
-				$HTML = $SanityTestFailure->message;
-				echo $HTML;
-
-			} else {
-				if ($Debug) echo "found nothing in the database regarding sanity test failures for that message_id<br>\n";
+				$HTML .= "\n<h2>Sanity Test Results</h2>\n";
+				$HTML .= "\n<blockquote>\n<pre>";
+				$HTML .=  $SanityTestFailure->message;
+				$HTML .= "</pre>\n</blockquote>\n";
 			}
-			$Cache->CacheDataSet($HTML);
-			$Cache->Add($FileName);
+			$HTML .= '</td></tr>';
 		}
-		echo "</pre>\n</blockquote>\n";
-		echo '</td></tr>';
-	}
+
+		echo $HTML;
+		$Cache->CacheDataSet($HTML);
+		$Cache->Add(CACHE_NAME, $PageNumber);
+	} /* no result */
 }
 
 ?>