From 8aed5cfb9c0013615d60a16092b774e624001ab5 Mon Sep 17 00:00:00 2001 From: Joshua May Date: Fri, 26 May 2023 05:30:33 -0700 Subject: [PATCH] Ignore Brotli decode result on 0-length return value (#4632) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Brotli response decoding was introduced via https://github.com/facebook/flipper/pull/4288, and released in 0.177.0. We noticed from that release that many of our iOS response bodies were not being rendered. It simply showed `(empty)` in the `Response Body` section. As noted in the gzip decoder ([here](https://github.com/facebook/flipper/blob/2a52656d0b67f10fd2650cd85154b7abfb0e1ba5/desktop/plugins/public/network/utils.tsx#L117-L119)) within Flipper, iOS already provides an inflated `data` value, so it doesn't need inflating again. This PR adds a best-effort guess to detect when the same problem arises in the Brotli decoder. I'm definitely not a Brotli expert, but according to [this SO post](https://stackoverflow.com/questions/39008957/is-there-a-way-to-check-if-a-buffer-is-in-brotli-compressed-format), there's no sure-fire way to detect Brotli data, and some blobs of random data will still present as Brotli. We may still occasionally see false positives that continue to show `(empty)`, however in my testing, all of our server responses have rendered JSON responses perfectly. The library used for decoding doesn't throw an error on failure with any responses we've seen, it just simply returns a 0-length buffer. So the naïve approach taken in this PR simply looks for a 0-length output buffer on a non-zero-length input buffer, and concludes "probably not Brotli, shrug emoji". ## Changelog Ignore Brotli decode result on 0-length return value Pull Request resolved: https://github.com/facebook/flipper/pull/4632 Test Plan: We can use Facebook's servers to test this. Fire up a RN app, and add the following somewhere you can run it: ```typescript fetch('https://graph.facebook.com/facebook/picture?redirect=false', { headers: { 'accept-encoding': 'br', }, }) ``` Before this patch, we can see that `Response Body` is `(empty)` in Flipper: Screenshot 2023-03-30 at 1 26 48 am But after this patch, we can see some valid JSON in the `Response Body`: Screenshot 2023-03-30 at 1 26 07 am Most importantly, both responses have `Content-Encoding: br` headers. Reviewed By: passy Differential Revision: D46219337 Pulled By: mweststrate fbshipit-source-id: 2ae775d381fa325c6d9e543bdbc617d1fd986671 --- desktop/plugins/public/network/utils.tsx | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/desktop/plugins/public/network/utils.tsx b/desktop/plugins/public/network/utils.tsx index d034226d4..a6a62c681 100644 --- a/desktop/plugins/public/network/utils.tsx +++ b/desktop/plugins/public/network/utils.tsx @@ -126,9 +126,16 @@ export function decodeBody( // Brotli encoding (https://github.com/facebook/flipper/issues/2578) case 'br': { - return new TextDecoder().decode( - decompress(Buffer.from(Base64.toUint8Array(data))), - ); + const inflated = decompress(Buffer.from(Base64.toUint8Array(data))); + + // on iOS, the stream send to flipper is already inflated, so the content-encoding will not + // match the actual data anymore, so a 0-length result without an error is expected. + // In that case, we intentionally fall-through + if (inflated.length === 0 && data.length > 0) { + break; + } + + return new TextDecoder().decode(inflated); } } // If this is not a gzipped or brotli-encoded request, assume we are interested in a proper utf-8 string.