jenkins-bot has submitted this change and it was merged. Change subject: conf: Clean up highlight.php and fix http-redirect bug ......................................................................
conf: Clean up highlight.php and fix http-redirect bug Follows-up 26929a2f2e4. https://noc.wikimedia.org/conf/highlight.php redirects to HTTP due to a missing 'else' statement in the $protocol logic. So the second block always executes due to the second block just being an unnamed block that unconditionally executes. Since we hardcode HTTP 1.1 and because this is primarily a developer endpoint, just use relative urls for the redirect. This way it also works locally for developers that have mediawiki-config itself mounted on a local webserver as it now is relative to the path instead of hardcoding /conf. It previously allowed for the hostname to be overridden (why?) via SERVER_NAME, but did not the path (via e.g. SCRIPT_FILENAME). Change-Id: Ie15eec3a90fa0a29b965387ee89e74131117fb71 --- M docroot/noc/conf/highlight.php M tests/noc-conf/highlightTest.php 2 files changed, 10 insertions(+), 14 deletions(-) Approvals: Reedy: Looks good to me, approved jenkins-bot: Verified diff --git a/docroot/noc/conf/highlight.php b/docroot/noc/conf/highlight.php index d6cf8ed..6298cbb 100644 --- a/docroot/noc/conf/highlight.php +++ b/docroot/noc/conf/highlight.php @@ -33,26 +33,22 @@ break; } } -if ( PHP_SAPI !== 'cli' ) { // Don't run if executing unit tests +// Don't run if executing unit tests +if ( PHP_SAPI !== 'cli' ) { ob_start( 'ob_gzhandler' ); header( 'Content-Type: text/html; charset=utf-8' ); } if ( !$selectedFilePath ) { if( $selectedFileName === null ){ - // No parameter file given, e.g. if you go to this file directly, redirect to overview - if( isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off' ){ - $protocol = "https"; - } { - $protocol = "http"; - } - header( "HTTP/1.1 302 Found" ); - header( "Location: ". $protocol . "://" . $_SERVER[ 'SERVER_NAME'] ."/conf/index.php" ); - echo $protocol . "://" . $_SERVER[ 'SERVER_NAME'] ."/conf/index.php"; + // If no 'file' is given (e.g. accessing this file directly), redirect to overview + header( 'HTTP/1.1 302 Found' ); + header( 'Location: ./index.php' ); + echo '<a href="./index.php">Redirect</a>'; exit; } else { - // Parameter file IS given, but for whatever reason no filename given or filename not existing in this directory - $hlHtml = "<pre>No valid, whitelisted filename in parameter \"file\" given.</pre>"; + // Parameter file is given, but not existing in this directory + $hlHtml = '<p>Invalid filename given.</p>'; } } else { // Follow symlink diff --git a/tests/noc-conf/highlightTest.php b/tests/noc-conf/highlightTest.php index fa75db3..8e9bd9c 100644 --- a/tests/noc-conf/highlightTest.php +++ b/tests/noc-conf/highlightTest.php @@ -88,7 +88,7 @@ /** * @dataProvider provideInvalidCases */ - public function testInvalidCases( $q, $expect = 'No valid, whitelisted filename' ) { + public function testInvalidCases( $q, $expect = 'Invalid filename given' ) { $this->assertContains( $expect, $this->runHighlight( $q ), @@ -102,7 +102,7 @@ 'file' => $q ); ob_start(); - require( $this->nocConfDir . '/highlight.php' ); + require $this->nocConfDir . '/highlight.php'; $out = ob_get_clean(); return $out; } -- To view, visit https://gerrit.wikimedia.org/r/186327 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie15eec3a90fa0a29b965387ee89e74131117fb71 Gerrit-PatchSet: 3 Gerrit-Project: operations/mediawiki-config Gerrit-Branch: master Gerrit-Owner: Krinkle <[email protected]> Gerrit-Reviewer: Hashar <[email protected]> Gerrit-Reviewer: Ori.livneh <[email protected]> Gerrit-Reviewer: Reedy <[email protected]> Gerrit-Reviewer: Tim Starling <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
