Document expandBlob behavior when no flags are given.
authordaniel <daniel.kinzler@wikimedia.de>
Fri, 12 Jan 2018 13:51:56 +0000 (14:51 +0100)
committerDaniel Kinzler <daniel.kinzler@wikimedia.de>
Sun, 14 Jan 2018 08:59:42 +0000 (08:59 +0000)
Bug: T184749
Change-Id: I5f1f029d928a7bc25877b0eae9f3822ec321b24a

includes/Storage/RevisionStore.php
includes/Storage/SqlBlobStore.php

index b0c193b..f8481fe 100644 (file)
@@ -804,7 +804,8 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup
         * @param SlotRecord $slot The SlotRecord to load content for
         * @param string|null $blobData The content blob, in the form indicated by $blobFlags
         * @param string|null $blobFlags Flags indicating how $blobData needs to be processed.
-        *  null if no processing should happen.
+        *        Use null if no processing should happen. That is in constrast to the empty string,
+        *        which causes the blob to be decoded according to the configured legacy encoding.
         * @param string|null $blobFormat MIME type indicating how $dataBlob is encoded
         * @param int $queryFlags
         *
@@ -825,6 +826,7 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup
                        $cacheKey = $slot->hasAddress() ? $slot->getAddress() : null;
 
                        if ( $blobFlags === null ) {
+                               // No blob flags, so use the blob verbatim.
                                $data = $blobData;
                        } else {
                                $data = $this->blobStore->expandBlob( $blobData, $blobFlags, $cacheKey );
index ec66431..031cb58 100644 (file)
@@ -369,6 +369,8 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
         *        May be the blob itself, or the blob compressed, or just the address
         *        of the actual blob, depending on $flags.
         * @param string|string[] $flags Blob flags, such as 'external' or 'gzip'.
+        *   Note that not including 'utf-8' in $flags will cause the data to be decoded
+        *   according to the legacy encoding specified via setLegacyEncoding.
         * @param string|null $cacheKey May be used for caching if given
         *
         * @return false|string The expanded blob or false on failure
@@ -431,7 +433,8 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
                $blobFlags = [];
 
                // Revisions not marked as UTF-8 will have legacy decoding applied by decompressData().
-               // XXX: if $this->legacyEncoding is not set, we could skip this. May be risky, though.
+               // XXX: if $this->legacyEncoding is not set, we could skip this. That would however be
+               // risky, since $this->legacyEncoding being set in the future would lead to data corruption.
                $blobFlags[] = 'utf-8';
 
                if ( $this->compressBlobs ) {
@@ -460,11 +463,13 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
         * @todo make this private, there should be no need to use this method outside this class.
         *
         * @param mixed $blob Reference to a text
-        * @param array $blobFlags Compression flags
+        * @param array $blobFlags Compression flags, such as 'gzip'.
+        *   Note that not including 'utf-8' in $blobFlags will cause the data to be decoded
+        *   according to the legacy encoding specified via setLegacyEncoding.
         *
         * @return string|bool Decompressed text, or false on failure
         */
-       public function decompressData( $blob, $blobFlags ) {
+       public function decompressData( $blob, array $blobFlags ) {
                if ( $blob === false ) {
                        // Text failed to be fetched; nothing to do
                        return false;