Remove $wgUseKeyHeader and OutputPage::getKeyHeader(), deprecated in 1.32
authorC. Scott Ananian <cscott@cscott.net>
Wed, 19 Jun 2019 18:22:42 +0000 (14:22 -0400)
committerC. Scott Ananian <cscott@cscott.net>
Wed, 19 Jun 2019 19:14:54 +0000 (15:14 -0400)
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

RELEASE-NOTES-1.34
includes/DefaultSettings.php
includes/OutputHandler.php
includes/OutputPage.php
includes/actions/RawAction.php
includes/api/ApiMain.php
includes/session/SessionProvider.php
tests/phpunit/includes/OutputPageTest.php

index 0e4081c..2f656b8 100644 (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 ===
index 2f793b5..c5a2f77 100644 (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
index ba9e2d7..1d1a193 100644 (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;
        }
index 95073c0..fba75ec 100644 (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 ( !array_key_exists( $header, $this->mVaryHeader ) ) {
-                       $this->mVaryHeader[$header] = [];
+               if ( $option !== null && count( $option ) > 0 ) {
+                       wfDeprecated( 'addVaryHeader $option is ignored', '1.34' );
                }
-               if ( !is_array( $option ) ) {
-                       $option = [];
+               if ( !array_key_exists( $header, $this->mVaryHeader ) ) {
+                       $this->mVaryHeader[$header] = null;
                }
-               $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' ) &&
index 3e4e614..f6c4472 100644 (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
index ed17e07..4efd230 100644 (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'] ) ) {
index def3bc3..80a400b 100644 (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
         */
index 32ce08c..5f0067d 100644 (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' ] ],
                ];
        }