Anomie has uploaded a new change for review.
https://gerrit.wikimedia.org/r/232401
Change subject: API: Add support for selected HTTP precondition headers
......................................................................
API: Add support for selected HTTP precondition headers
Specifically, GET requests can now return ETag and Last-Modified
headers, and If-None-Match and If-Modified-Since headers on such GET
requests will be honored. This doesn't change any API modules to
actually return these values, it just provides the infrastructure.
For reasoning on why only GET requests and why only these two of the
five precondition headers defined by RFC 7232, see the doc comment on
ApiMain::checkConditionalRequestHeaders().
Change-Id: Ia18874c9360fcffdad323b341ca867ba773788fd
---
M RELEASE-NOTES-1.26
M includes/api/ApiBase.php
M includes/api/ApiMain.php
M tests/phpunit/includes/api/ApiMainTest.php
4 files changed, 342 insertions(+), 3 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/01/232401/1
diff --git a/RELEASE-NOTES-1.26 b/RELEASE-NOTES-1.26
index 1b468a1..6203e4c 100644
--- a/RELEASE-NOTES-1.26
+++ b/RELEASE-NOTES-1.26
@@ -92,8 +92,15 @@
sometimes being numerically-indexed objects with formatversion=2.
* When errors about users being blocked are returned, they now include
information about the relevant block.
+* API responses to GET requests may now include ETag and Last-Modified headers,
+ and will honor corresponding If-None-Match and If-Modified-Since on such
+ requests.
=== Action API internal changes in 1.26 ===
+* API action modules may now provide values for the RFC 7232 ETag and
+ Last-Modified headers. The API will check these against If-None-Match and
+ If-Modified-Since request headers on GET requests and avoid executing the
+ module when appropriate.
=== Languages updated in 1.26 ===
diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index 7b71e6c..d53797b 100644
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -345,6 +345,22 @@
return null;
}
+ /**
+ * Returns data for HTTP conditional request mechanisms.
+ *
+ * @since 1.26
+ * @param string $condition Condition being queried:
+ * - last-modified: Return a timestamp representing the maximum of the
+ * last-modified dates for all resources involved in the request. See
+ * RFC 7232 § 2.2 for semantics.
+ * - etag: Return an entity-tag representing the state of all
resources involved
+ * in the request. Quotes must be included. See RFC 7232 § 2.3 for
semantics.
+ * @return string|boolean|null As described above, or null if no value
is available.
+ */
+ public function getConditionalRequestData( $condition ) {
+ return null;
+ }
+
/**@}*/
/************************************************************************//**
diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index 8ce505a..6135cd5 100644
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -430,8 +430,10 @@
$t = microtime( true );
try {
$this->executeAction();
+ $isError = false;
} catch ( Exception $e ) {
$this->handleException( $e );
+ $isError = true;
}
// Log the request whether or not there was an error
@@ -439,7 +441,7 @@
// Send cache headers after any code which might generate an
error, to
// avoid sending public cache headers for errors.
- $this->sendCacheHeaders();
+ $this->sendCacheHeaders( $isError );
ob_end_flush();
}
@@ -532,7 +534,7 @@
// Log the request and reset cache headers
$main->logRequest( 0 );
- $main->sendCacheHeaders();
+ $main->sendCacheHeaders( true );
ob_end_flush();
}
@@ -701,7 +703,12 @@
return "/^https?:\/\/$wildcard$/";
}
- protected function sendCacheHeaders() {
+ /**
+ * Send caching headers
+ * @param boolean $isError Whether an error response is being output
+ * @since 1.26 added $isError parameter
+ */
+ protected function sendCacheHeaders( $isError ) {
$response = $this->getRequest()->response();
$out = $this->getOutput();
@@ -709,6 +716,19 @@
if ( $config->get( 'VaryOnXFP' ) ) {
$out->addVaryHeader( 'X-Forwarded-Proto' );
+ }
+
+ if ( !$isError && $this->mModule &&
+ ( $this->getRequest()->getMethod() === 'GET' ||
$this->getRequest()->getMethod() === 'HEAD' )
+ ) {
+ $etag = $this->mModule->getConditionalRequestData(
'etag' );
+ if ( $etag !== null ) {
+ $response->header( "ETag: $etag" );
+ }
+ $lastMod = $this->mModule->getConditionalRequestData(
'last-modified' );
+ if ( $lastMod !== null ) {
+ $response->header( 'Last-Modified: ' .
wfTimestamp( TS_RFC2822, $lastMod ) );
+ }
}
// The logic should be:
@@ -994,6 +1014,122 @@
}
/**
+ * Check selected RFC 7232 precondition headers
+ *
+ * RFC 7232 envisions a particular model where you send your request to
"a
+ * resource", and for write requests that you can read "the resource" by
+ * changing the method to GET. When the API receives a GET request, it
+ * works out even though "the resource" from RFC 7232's perspective
might
+ * be many resources from MediaWiki's perspective. But it totally fails
for
+ * a POST, since what HTTP sees as "the resource" is probably just
+ * "/api.php" with all the interesting bits in the body.
+ *
+ * Therefore, we only support RFC 7232 precondition headers for GET.
That
+ * means we don't need to bother with If-Match and If-Unmodified-Since
+ * since they only apply to modification requests.
+ *
+ * And since we don't support Range, If-Range is ignored too.
+ *
+ * @since 1.26
+ * @param ApiBase $module Api module being used
+ * @return bool True on success, false should exit immediately
+ */
+ protected function checkConditionalRequestHeaders( $module ) {
+ if ( $this->mInternalMode ) {
+ // No headers to check in internal mode
+ return true;
+ }
+
+ if ( $this->getRequest()->getMethod() !== 'GET' &&
$this->getRequest()->getMethod() !== 'HEAD' ) {
+ // Don't check POSTs
+ return true;
+ }
+
+ $response = $this->getRequest()->response();
+ $return304 = false;
+
+ $ifNoneMatch = array_diff(
+ $this->getRequest()->getHeader( 'If-None-Match',
WebRequest::GETHEADER_LIST ) ?: array(),
+ array( '' )
+ );
+ if ( $ifNoneMatch ) {
+ if ( $ifNoneMatch === array( '*' ) ) {
+ // API responses always "exist"
+ $etag = $test = '*';
+ } else {
+ $etag = $module->getConditionalRequestData(
'etag' );
+ $test = substr( $etag, 0, 2 ) === 'W/' ?
substr( $etag, 2 ) : $etag;
+ }
+ }
+ if ( $ifNoneMatch && $etag !== null ) {
+ $match = array_map( function ( $s ) {
+ return substr( $s, 0, 2 ) === 'W/' ? substr(
$s, 2 ) : $s;
+ }, $ifNoneMatch );
+ $return304 = in_array( $test, $match, true );
+ } else {
+ $value = trim( $this->getRequest()->getHeader(
'If-Modified-Since' ) );
+
+ // Some old browsers sends sizes after the date, like
this:
+ // Wed, 20 Aug 2003 06:51:19 GMT; length=5202
+ // Ignore that.
+ $i = strpos( $value, ';' );
+ if ( $i !== false ) {
+ $value = trim( substr( $value, 0, $i ) );
+ }
+
+ if ( $value !== '' ) {
+ try {
+ $ts = new MWTimestamp( $value );
+ if (
+ // RFC 7231 IMF-fixdate
+ $ts->getTimestamp( TS_RFC2822 )
=== $value ||
+ // RFC 850
+ $ts->format( 'l, d-M-y H:i:s')
. ' GMT' === $value ||
+ // asctime (with and without
space-padded day)
+ $ts->format( 'D M j H:i:s Y')
=== $value ||
+ $ts->format( 'D M j H:i:s Y')
=== $value
+ ) {
+ $lastMod =
$module->getConditionalRequestData( 'last-modified' );
+ if ( $lastMod !== null ) {
+ // Mix in some
MediaWiki modification times
+ $modifiedTimes = array(
+ 'page' =>
$lastMod,
+ 'user' =>
$this->getUser()->getTouched(),
+ 'epoch' =>
$this->getConfig()->get( 'CacheEpoch' ),
+ );
+ if (
$this->getConfig()->get( 'UseSquid' ) ) {
+ // bug 44570:
the core page itself may not change, but resources might
+
$modifiedTimes['sepoch'] = wfTimestamp(
+ TS_MW,
time() - $this->getConfig()->get( 'SquidMaxage' )
+ );
+ }
+ Hooks::run(
'OutputPageCheckLastModified', array( &$modifiedTimes ) );
+ $lastMod = max(
$modifiedTimes );
+ $return304 =
wfTimestamp( TS_MW, $lastMod ) < $ts->getTimestamp( TS_MW );
+ }
+ }
+ } catch ( TimestampException $e ) {
+ // Invalid timestamp, ignore it
+ }
+ }
+ }
+
+ if ( $return304 ) {
+ $response->statusHeader( 304 );
+
+ // Avoid outputting the compressed representation of a
zero-length body
+ MediaWiki\suppressWarnings();
+ ini_set( 'zlib.output_compression', 0 );
+ MediaWiki\restoreWarnings();
+ wfClearOutputBuffers();
+
+ return false;
+ }
+
+ return true;
+ }
+
+ /**
* Check for sufficient permissions to execute
* @param ApiBase $module An Api module
*/
@@ -1083,6 +1219,10 @@
return;
}
+ if ( !$this->checkConditionalRequestHeaders( $module ) ) {
+ return;
+ }
+
if ( !$this->mInternalMode ) {
$this->setupExternalResponse( $module, $params );
}
diff --git a/tests/phpunit/includes/api/ApiMainTest.php
b/tests/phpunit/includes/api/ApiMainTest.php
index ee1a954..95510bf 100644
--- a/tests/phpunit/includes/api/ApiMainTest.php
+++ b/tests/phpunit/includes/api/ApiMainTest.php
@@ -79,4 +79,180 @@
);
}
}
+
+ /**
+ * Test HTTP precondition headers
+ *
+ * @covers ApiMain::checkConditionalRequestHeaders
+ * @dataProvider provideCheckConditionalRequestHeaders
+ * @param array $headers HTTP headers
+ * @param array $conditions Return data for
ApiBase::getConditionalRequestData
+ * @param int $status Expected response status
+ * @param bool $post Request is a POST
+ */
+ public function testCheckConditionalRequestHeaders( $headers,
$conditions, $status, $post = false ) {
+ $request = new FauxRequest( array( 'action' => 'query', 'meta'
=> 'siteinfo' ), $post );
+ $request->setHeaders( $headers );
+ $request->response()->statusHeader( 200 ); // Why doesn't it
default?
+
+ $api = new ApiMain( $request );
+ $priv = TestingAccessWrapper::newFromObject( $api );
+ $priv->mInternalMode = false;
+
+ $module = $this->getMockBuilder( 'ApiBase' )
+ ->setConstructorArgs( array( $api, 'mock' ) )
+ ->setMethods( array( 'getConditionalRequestData' ) )
+ ->getMockForAbstractClass();
+ $module->expects( $this->any() )
+ ->method( 'getConditionalRequestData' )
+ ->will( $this->returnCallback( function ( $condition )
use ( $conditions ) {
+ return isset( $conditions[$condition] ) ?
$conditions[$condition] : null;
+ } ) );
+
+ ob_start();
+ $ret = $priv->checkConditionalRequestHeaders( $module );
+ $out = ob_get_clean();
+
+ $this->assertSame( $status,
$request->response()->getStatusCode() );
+ $this->assertSame( $status === 200, $ret );
+ if ( $status === 412 ) {
+ $this->assertNotSame( '', $out );
+ } else {
+ $this->assertSame( '', $out );
+ }
+ }
+
+ public static function provideCheckConditionalRequestHeaders() {
+ $now = time();
+
+ return array(
+ // Non-existing from module is ignored
+ array( array( 'If-None-Match' => '"foo", "bar"' ),
array(), 200 ),
+ array( array( 'If-Modified-Since' => 'Tue, 18 Aug 2015
00:00:00 GMT' ), array(), 200 ),
+
+ // No headers
+ array(
+ array(),
+ array(
+ 'etag' => '""',
+ 'last-modified' => '20150815000000',
+ ),
+ 200
+ ),
+
+ // Basic If-None-Match
+ array( array( 'If-None-Match' => '"foo", "bar"' ),
array( 'etag' => '"bar"' ), 304 ),
+ array( array( 'If-None-Match' => '"foo", "bar"' ),
array( 'etag' => '"baz"' ), 200 ),
+ array( array( 'If-None-Match' => '"foo"' ), array(
'etag' => 'W/"foo"' ), 304 ),
+ array( array( 'If-None-Match' => 'W/"foo"' ), array(
'etag' => '"foo"' ), 304 ),
+ array( array( 'If-None-Match' => 'W/"foo"' ), array(
'etag' => 'W/"foo"' ), 304 ),
+
+ // Pointless, but supported
+ array( array( 'If-None-Match' => '*' ), array(), 304 ),
+
+ // Basic If-Modified-Since
+ array( array( 'If-Modified-Since' => wfTimestamp(
TS_RFC2822, $now ) ),
+ array( 'last-modified' => wfTimestamp( TS_MW,
$now - 1 ) ), 304 ),
+ array( array( 'If-Modified-Since' => wfTimestamp(
TS_RFC2822, $now ) ),
+ array( 'last-modified' => wfTimestamp( TS_MW,
$now ) ), 200 ),
+ array( array( 'If-Modified-Since' => wfTimestamp(
TS_RFC2822, $now ) ),
+ array( 'last-modified' => wfTimestamp( TS_MW,
$now + 1 ) ), 200 ),
+
+ // If-Modified-Since ignored when If-None-Match is
given too
+ array( array( 'If-None-Match' => '""',
'If-Modified-Since' => wfTimestamp( TS_RFC2822, $now ) ),
+ array( 'etag' => '"x"', 'last-modified' =>
wfTimestamp( TS_MW, $now - 1 ) ), 200 ),
+ array( array( 'If-None-Match' => '""',
'If-Modified-Since' => wfTimestamp( TS_RFC2822, $now ) ),
+ array( 'last-modified' => wfTimestamp( TS_MW,
$now - 1 ) ), 304 ),
+
+ // Ignored for POST
+ array( array( 'If-None-Match' => '"foo", "bar"' ),
array( 'etag' => '"bar"' ), 200, true ),
+ array( array( 'If-Modified-Since' => wfTimestamp(
TS_RFC2822, $now ) ),
+ array( 'last-modified' => wfTimestamp( TS_MW,
$now - 1 ) ), 200, true ),
+
+ // Other date formats allowed by the RFC
+ array( array( 'If-Modified-Since' => gmdate( 'l, d-M-y
H:i:s', $now ) . ' GMT' ),
+ array( 'last-modified' => wfTimestamp( TS_MW,
$now - 1 ) ), 304 ),
+ array( array( 'If-Modified-Since' => gmdate( 'D M j
H:i:s Y', $now ) ),
+ array( 'last-modified' => wfTimestamp( TS_MW,
$now - 1 ) ), 304 ),
+
+ // Old browser extension to HTTP/1.0
+ array( array( 'If-Modified-Since' => wfTimestamp(
TS_RFC2822, $now ) . '; length=123' ),
+ array( 'last-modified' => wfTimestamp( TS_MW,
$now - 1 ) ), 304 ),
+
+ // Invalid date formats should be ignored
+ array( array( 'If-Modified-Since' => gmdate( 'Y-m-d
H:i:s', $now ) . ' GMT' ),
+ array( 'last-modified' => wfTimestamp( TS_MW,
$now - 1 ) ), 200 ),
+ );
+ }
+
+ /**
+ * Test conditional headers output
+ * @dataProvider provideConditionalRequestHeadersOutput
+ * @param array $conditions Return data for
ApiBase::getConditionalRequestData
+ * @param array $headers Expected output headers
+ * @param bool $isError $isError flag
+ * @param bool $post Request is a POST
+ */
+ public function testConditionalRequestHeadersOutput( $conditions,
$headers, $isError = false, $post = false ) {
+ $request = new FauxRequest( array( 'action' => 'query', 'meta'
=> 'siteinfo' ), $post );
+ $response = $request->response();
+
+ $api = new ApiMain( $request );
+ $priv = TestingAccessWrapper::newFromObject( $api );
+ $priv->mInternalMode = false;
+
+ $module = $this->getMockBuilder( 'ApiBase' )
+ ->setConstructorArgs( array( $api, 'mock' ) )
+ ->setMethods( array( 'getConditionalRequestData' ) )
+ ->getMockForAbstractClass();
+ $module->expects( $this->any() )
+ ->method( 'getConditionalRequestData' )
+ ->will( $this->returnCallback( function ( $condition )
use ( $conditions ) {
+ return isset( $conditions[$condition] ) ?
$conditions[$condition] : null;
+ } ) );
+ $priv->mModule = $module;
+
+ $priv->sendCacheHeaders( $isError );
+
+ foreach ( array( 'Last-Modified', 'ETag' ) as $header ) {
+ $this->assertEquals(
+ isset( $headers[$header] ) ? $headers[$header]
: null,
+ $response->getHeader( $header ),
+ $header
+ );
+ }
+ }
+
+ public static function provideConditionalRequestHeadersOutput() {
+ return array(
+ array(
+ array(),
+ array()
+ ),
+ array(
+ array( 'etag' => '"foo"' ),
+ array( 'ETag' => '"foo"' )
+ ),
+ array(
+ array( 'last-modified' => '20150818000102' ),
+ array( 'Last-Modified' => 'Tue, 18 Aug 2015
00:01:02 GMT' )
+ ),
+ array(
+ array( 'etag' => '"foo"', 'last-modified' =>
'20150818000102' ),
+ array( 'ETag' => '"foo"', 'Last-Modified' =>
'Tue, 18 Aug 2015 00:01:02 GMT' )
+ ),
+ array(
+ array( 'etag' => '"foo"', 'last-modified' =>
'20150818000102' ),
+ array(),
+ true,
+ ),
+ array(
+ array( 'etag' => '"foo"', 'last-modified' =>
'20150818000102' ),
+ array(),
+ false,
+ true,
+ ),
+ );
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/232401
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia18874c9360fcffdad323b341ca867ba773788fd
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Anomie <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits