Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Remove $wgUseKeyHeader and OutputPage::getKeyHeader(), deprecated in 1.32

Browse source 
These implemented a since-abandoned draft IETF spec, and the code was
broken due to (1) case-(in)sensitivity issues with the Accept-Language
header and (2) the BCP47 language code compatibility workaround we use.

Change-Id: Ia53d07cd8ce8ab1497294ea244c13c7499f632c7
This commit is contained in:
C. Scott Ananian 2019-06-19 14:22:42 -04:00
parent 8a80f58cef
commit ce1bd86a4b
8 changed files with 53 additions and 152 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
RELEASE-NOTES-1.34
View file

@ -214,6 +214,8 @@ because of Phabricator reports.
* OutputPage::addWikiText(), ::addWikiTextWithTitle(), ::addWikiTextTitleTidy(),
::addWikiTextTidy(), ::addWikiTextTitle(), deprecated in 1.32, have been
removed.
* The $wgUseKeyHeader configuration option and the OutputPage::getKeyHeader()
method, deprecated in 1.32, have been removed.
* …
=== Deprecations in 1.34 ===

8
includes/DefaultSettings.php
View file

@ -2737,14 +2737,6 @@ $wgUseCdn = false;
*/
$wgUseESI = false;
/**
* Send the Key HTTP header for better caching.
* See https://datatracker.ietf.org/doc/draft-ietf-httpbis-key/ for details.
* @since 1.27
* @deprecated in 1.32, the IETF spec expired without becoming a standard.
*/
$wgUseKeyHeader = false;
/**
* Add X-Forwarded-Proto to the Vary and Key headers for API requests and
* RSS/Atom feeds. Use this if you have an SSL termination setup

4
includes/OutputHandler.php
View file

@ -123,10 +123,6 @@ class OutputHandler {
}
if ( !$foundVary ) {
header( 'Vary: Accept-Encoding' );
global $wgUseKeyHeader;
if ( $wgUseKeyHeader ) {
header( 'Key: Accept-Encoding;match=gzip' );
}
}
return $s;
}

91
includes/OutputPage.php
View file

@ -265,11 +265,12 @@ class OutputPage extends ContextSource {
private $mFollowPolicy = 'follow';
/**
* @var array Headers that cause the cache to vary. Key is header name, value is an array of
* options for the Key header.
* @var array Headers that cause the cache to vary. Key is header name,
* value should always be null. (Value was an array of options for
* the `Key` header, which was deprecated in 1.32 and removed in 1.34.)
*/
private $mVaryHeader = [
'Accept-Encoding' => [ 'match=gzip' ],
'Accept-Encoding' => null,
];
/**
@ -2222,19 +2223,18 @@ class OutputPage extends ContextSource {
* Add an HTTP header that will influence on the cache
*
* @param string $header Header name
* @param string[]|null $option Options for the Key header. See
* https://datatracker.ietf.org/doc/draft-fielding-http-key/
* for the list of valid options.
* @param string[]|null $option Deprecated; formerly options for the
* Key header, deprecated in 1.32 and removed in 1.34. See
* https://datatracker.ietf.org/doc/draft-fielding-http-key/
* for the list of formerly-valid options.
*/
public function addVaryHeader( $header, array $option = null ) {
if ( $option !== null && count( $option ) > 0 ) {
wfDeprecated( 'addVaryHeader $option is ignored', '1.34' );
}
if ( !array_key_exists( $header, $this->mVaryHeader ) ) {
$this->mVaryHeader[$header] = [];
$this->mVaryHeader[$header] = null;
}
if ( !is_array( $option ) ) {
$option = [];
}
$this->mVaryHeader[$header] =
array_unique( array_merge( $this->mVaryHeader[$header], $option ) );
}
/**
@ -2277,41 +2277,6 @@ class OutputPage extends ContextSource {
return 'Link: ' . implode( ',', $this->mLinkHeader );
}
/**
* Get a complete Key header
*
* @return string
* @deprecated in 1.32; the IETF spec for this header expired w/o becoming
* a standard.
*/
public function getKeyHeader() {
wfDeprecated( '$wgUseKeyHeader', '1.32' );
$cvCookies = $this->getCacheVaryCookies();
$cookiesOption = [];
foreach ( $cvCookies as $cookieName ) {
$cookiesOption[] = 'param=' . $cookieName;
}
$this->addVaryHeader( 'Cookie', $cookiesOption );
foreach ( SessionManager::singleton()->getVaryHeaders() as $header => $options ) {
$this->addVaryHeader( $header, $options );
}
$headers = [];
foreach ( $this->mVaryHeader as $header => $option ) {
$newheader = $header;
if ( is_array( $option ) && count( $option ) > 0 ) {
$newheader .= ';' . implode( ';', $option );
}
$headers[] = $newheader;
}
$key = 'Key: ' . implode( ',', $headers );
return $key;
}
/**
* T23672: Add Accept-Language to Vary and Key headers if there's no 'variant' parameter in GET.
*
@ -2327,33 +2292,7 @@ class OutputPage extends ContextSource {
$lang = $title->getPageLanguage();
if ( !$this->getRequest()->getCheck( 'variant' ) && $lang->hasVariants() ) {
$variants = $lang->getVariants();
$aloption = [];
foreach ( $variants as $variant ) {
if ( $variant === $lang->getCode() ) {
continue;
}
// XXX Note that this code is not strictly correct: we
// do a case-insensitive match in
// LanguageConverter::getHeaderVariant() while the
// (abandoned, draft) spec for the `Key` header only
// allows case-sensitive matches. To match the logic
// in LanguageConverter::getHeaderVariant() we should
// also be looking at fallback variants and deprecated
// mediawiki-internal codes, as well as BCP 47
// normalized forms.
$aloption[] = "substr=$variant";
// IE and some other browsers use BCP 47 standards in their Accept-Language header,
// like "zh-CN" or "zh-Hant". We should handle these too.
$variantBCP47 = LanguageCode::bcp47( $variant );
if ( $variantBCP47 !== $variant ) {
$aloption[] = "substr=$variantBCP47";
}
}
$this->addVaryHeader( 'Accept-Language', $aloption );
$this->addVaryHeader( 'Accept-Language' );
}
}
@ -2464,10 +2403,6 @@ class OutputPage extends ContextSource {
# maintain different caches for logged-in users and non-logged in ones
$response->header( $this->getVaryHeader() );
if ( $config->get( 'UseKeyHeader' ) ) {
$response->header( $this->getKeyHeader() );
}
if ( $this->mEnableClientCache ) {
if (
$config->get( 'UseCdn' ) &&

3
includes/actions/RawAction.php
View file

@ -88,9 +88,6 @@ class RawAction extends FormlessAction {
// Set standard Vary headers so cache varies on cookies and such (T125283)
$response->header( $this->getOutput()->getVaryHeader() );
if ( $config->get( 'UseKeyHeader' ) ) {
$response->header( $this->getOutput()->getKeyHeader() );
}
// Output may contain user-specific data;
// vary generated content for open sessions on private wikis

16
includes/api/ApiMain.php
View file

@ -916,32 +916,20 @@ class ApiMain extends ApiBase {
return;
}
$useKeyHeader = $config->get( 'UseKeyHeader' );
if ( $this->mCacheMode == 'anon-public-user-private' ) {
$out->addVaryHeader( 'Cookie' );
$response->header( $out->getVaryHeader() );
if ( $useKeyHeader ) {
$response->header( $out->getKeyHeader() );
if ( $out->haveCacheVaryCookies() ) {
// Logged in, mark this request private
$response->header( "Cache-Control: $privateCache" );
return;
}
// Logged out, send normal public headers below
} elseif ( MediaWiki\Session\SessionManager::getGlobalSession()->isPersistent() ) {
if ( MediaWiki\Session\SessionManager::getGlobalSession()->isPersistent() ) {
// Logged in or otherwise has session (e.g. anonymous users who have edited)
// Mark request private
$response->header( "Cache-Control: $privateCache" );
return;
} // else no Key and anonymous, send public headers below
} // else anonymous, send public headers below
}
// Send public headers
$response->header( $out->getVaryHeader() );
if ( $useKeyHeader ) {
$response->header( $out->getKeyHeader() );
}
// If nobody called setCacheMaxAge(), use the (s)maxage parameters
if ( !isset( $this->mCacheControl['s-maxage'] ) ) {

3
includes/session/SessionProvider.php
View file

@ -402,6 +402,9 @@ abstract class SessionProvider implements SessionProviderInterface, LoggerAwareI
* }
* @endcode
*
* Note that the $options parameter to addVaryHeader has been deprecated
* since 1.34, and should be `null` or an empty array.
*
* @protected For use by \MediaWiki\Session\SessionManager only
* @return array
*/

78
tests/phpunit/includes/OutputPageTest.php
View file

@ -2246,14 +2246,13 @@ class OutputPageTest extends MediaWikiTestCase {
*
* @covers OutputPage::addVaryHeader
* @covers OutputPage::getVaryHeader
* @covers OutputPage::getKeyHeader
*
* @param array[] $calls For each array, call addVaryHeader() with those arguments
* @param string[] $cookies Array of cookie names to vary on
* @param string $vary Text of expected Vary header (including the 'Vary: ')
* @param string $key Text of expected Key header (including the 'Key: ')
*/
public function testVaryHeaders( array $calls, array $cookies, $vary, $key ) {
public function testVaryHeaders( array $calls, array $cookies, $vary ) {
// Get rid of default Vary fields
$op = $this->getMockBuilder( OutputPage::class )
->setConstructorArgs( [ new RequestContext() ] )
@ -2264,22 +2263,19 @@ class OutputPageTest extends MediaWikiTestCase {
->will( $this->returnValue( $cookies ) );
TestingAccessWrapper::newFromObject( $op )->mVaryHeader = [];
$this->hideDeprecated( '$wgUseKeyHeader' );
$this->hideDeprecated( 'addVaryHeader $option is ignored' );
foreach ( $calls as $call ) {
$op->addVaryHeader( ...$call );
}
$this->assertEquals( $vary, $op->getVaryHeader(), 'Vary:' );
$this->assertEquals( $key, $op->getKeyHeader(), 'Key:' );
}
public function provideVaryHeaders() {
// note: getKeyHeader() automatically adds Vary: Cookie
return [
'No header' => [
[],
[],
'Vary: ',
'Key: Cookie',
],
'Single header' => [
[
@ -2287,7 +2283,6 @@ class OutputPageTest extends MediaWikiTestCase {
],
[],
'Vary: Cookie',
'Key: Cookie',
],
'Non-unique headers' => [
[
@ -2297,26 +2292,26 @@ class OutputPageTest extends MediaWikiTestCase {
],
[],
'Vary: Cookie, Accept-Language',
'Key: Cookie,Accept-Language',
],
'Two headers with single options' => [
// Options are deprecated since 1.34
[
[ 'Cookie', [ 'param=phpsessid' ] ],
[ 'Accept-Language', [ 'substr=en' ] ],
],
[],
'Vary: Cookie, Accept-Language',
'Key: Cookie;param=phpsessid,Accept-Language;substr=en',
],
'One header with multiple options' => [
// Options are deprecated since 1.34
[
[ 'Cookie', [ 'param=phpsessid', 'param=userId' ] ],
],
[],
'Vary: Cookie',
'Key: Cookie;param=phpsessid;param=userId',
],
'Duplicate option' => [
// Options are deprecated since 1.34
[
[ 'Cookie', [ 'param=phpsessid' ] ],
[ 'Cookie', [ 'param=phpsessid' ] ],
@ -2324,30 +2319,28 @@ class OutputPageTest extends MediaWikiTestCase {
],
[],
'Vary: Cookie, Accept-Language',
'Key: Cookie;param=phpsessid,Accept-Language;substr=en',
],
'Same header, different options' => [
// Options are deprecated since 1.34
[
[ 'Cookie', [ 'param=phpsessid' ] ],
[ 'Cookie', [ 'param=userId' ] ],
],
[],
'Vary: Cookie',
'Key: Cookie;param=phpsessid;param=userId',
],
'No header, vary cookies' => [
[],
[ 'cookie1', 'cookie2' ],
'Vary: Cookie',
'Key: Cookie;param=cookie1;param=cookie2',
],
'Cookie header with option plus vary cookies' => [
// Options are deprecated since 1.34
[
[ 'Cookie', [ 'param=cookie1' ] ],
],
[ 'cookie2', 'cookie3' ],
'Vary: Cookie',
'Key: Cookie;param=cookie1;param=cookie2;param=cookie3',
],
'Non-cookie header plus vary cookies' => [
[
@ -2355,16 +2348,15 @@ class OutputPageTest extends MediaWikiTestCase {
],
[ 'cookie' ],
'Vary: Accept-Language, Cookie',
'Key: Accept-Language,Cookie;param=cookie',
],
'Cookie and non-cookie headers plus vary cookies' => [
// Options are deprecated since 1.34
[
[ 'Cookie', [ 'param=cookie1' ] ],
[ 'Accept-Language' ],
],
[ 'cookie2' ],
'Vary: Cookie, Accept-Language',
'Key: Cookie;param=cookie1;param=cookie2,Accept-Language',
],
];
}
@ -2417,10 +2409,9 @@ class OutputPageTest extends MediaWikiTestCase {
/**
* @dataProvider provideAddAcceptLanguage
* @covers OutputPage::addAcceptLanguage
* @covers OutputPage::getKeyHeader
*/
public function testAddAcceptLanguage(
$code, array $variants, array $expected, array $options = []
$code, array $variants, $expected, array $options = []
) {
$req = new FauxRequest( in_array( 'varianturl', $options ) ? [ 'variant' => 'x' ] : [] );
$op = $this->newInstance( [], $req, in_array( 'notitle', $options ) ? 'notitle' : null );
@ -2444,41 +2435,38 @@ class OutputPageTest extends MediaWikiTestCase {
// This will run addAcceptLanguage()
$op->sendCacheControl();
$this->hideDeprecated( '$wgUseKeyHeader' );
$keyHeader = $op->getKeyHeader();
if ( !$expected ) {
$this->assertFalse( strpos( 'Accept-Language', $keyHeader ) );
return;
}
$keyHeader = explode( ' ', $keyHeader, 2 )[1];
$keyHeader = explode( ',', $keyHeader );
$acceptLanguage = null;
foreach ( $keyHeader as $item ) {
if ( strpos( $item, 'Accept-Language;' ) === 0 ) {
$acceptLanguage = $item;
break;
}
}
$expectedString = 'Accept-Language;substr=' . implode( ';substr=', $expected );
$this->assertSame( $expectedString, $acceptLanguage );
$this->assertSame( "Vary: $expected", $op->getVaryHeader() );
}
public function provideAddAcceptLanguage() {
return [
'No variants' => [ 'en', [ 'en' ], [] ],
'One simple variant' => [ 'en', [ 'en', 'en-x-piglatin' ], [ 'en-x-piglatin' ] ],
'No variants' => [
'en',
[ 'en' ],
'Accept-Encoding, Cookie',
],
'One simple variant' => [
'en',
[ 'en', 'en-x-piglatin' ],
'Accept-Encoding, Cookie, Accept-Language',
],
'Multiple variants with BCP47 alternatives' => [
'zh',
[ 'zh', 'zh-hans', 'zh-cn', 'zh-tw' ],
[ 'zh-hans', 'zh-Hans', 'zh-cn', 'zh-Hans-CN', 'zh-tw', 'zh-Hant-TW' ],
'Accept-Encoding, Cookie, Accept-Language',
],
'No title' => [
'en',
[ 'en', 'en-x-piglatin' ],
'Accept-Encoding, Cookie',
[ 'notitle' ]
],
'Variant in URL' => [
'en',
[ 'en', 'en-x-piglatin' ],
'Accept-Encoding, Cookie',
[ 'varianturl' ]
],
'No title' => [ 'en', [ 'en', 'en-x-piglatin' ], [], [ 'notitle' ] ],
'Variant in URL' => [ 'en', [ 'en', 'en-x-piglatin' ], [], [ 'varianturl' ] ],
];
}