Fix content encoding issues
Summary: Changelog: [Network] Non-binary request are not properly utf-8 decoded on both iOS and Android, both when gzipped and when not gzipped This diff fixes a long standing / ping-pong issue regarding network decoding differences between * iOS vs Android * binary vs utf-8 * gzipped vs uncompressed The changes aren't too big, but the underlying investigating is :) The primary contribution to this diff is: First, adding test cases for know problematic cases. This is done by grabbing the messages that are send from the flipper client to flipper using the flipper messages plugin. This is the base64 data that is stored in the `.txt` files. Beyond that, for all tests public endpoints are used, so that we can both get a hold of the raw original files, and how we expect them to be displayed in flipper. For testing a simple RN app was build, with a button that fires a bunch requests. The first 3 are captured in unit tests, the last one is not idempotent, but a case reported in #1466, so just left it there as manual verification. ``` const fetchData = async () => { await fetch( 'https://raw.githubusercontent.com/SangKa/MobX-Docs-CN/master/docs/donating.md', { headers: { 'Accept-Encoding': 'identity', // signals that we don't want gzip }, }, ); await fetch('https://reactnative.dev/img/tiny_logo.png?x=' + Math.random()); await fetch( 'https://raw.githubusercontent.com/SangKa/MobX-Docs-CN/master/docs/donating.md', ); await fetch( 'https://ex.ke.com/sdk/recommend/html/100001314?hdicCityId=110000¶mMap[source]=&id=100001314&mediumId=100000037&elementId=&resblockId=1111027381003&templateConfig=%5Bobject%20Object%5D&fbExpoId=346620976471638017&fbQueryId=&required400=true&unique=1111027381003&parentSceneId=', ); }; ``` The second contribution of this diff is that it doesn't use weird URLencoder hacks to convert base64 to utf8, but rather a proper library. The problem with our original solution, using `atob` is that it converts to ASCII, not to utf-8, which is the source of the original bugs. See for more background on this: https://www.npmjs.com/package/js-base64#decode-vs-atob-and-encode-vs-btoa- Solves: https://github.com/facebook/flipper/issues/1466 https://github.com/facebook/flipper/pull/1541 https://github.com/facebook/flipper/issues/1458 Supersedes D23837750 Future work: we don't inspect the `content-type=xxx;charset` header yet, which we should do for less common encodings, to make sure that they get displayed correctly as well Future work: in feature like copy data and curl, we always call decode body, without check if we are actually dealing with non-binary data. Probably it is better to keep binary data in base64, rather than decoding it, as that will assume the data is an utf-8 string, which might fail. An assumption in these changes is that binary data is never gzipped, which is generally correct; gzip is not applied by webserver to things like images, as it would increase, not decrease their size, and waste a lot of computation power. Reviewed By: cekkaewnumchai Differential Revision: D23403095 fbshipit-source-id: 5099cc4a7503f0f63bd10585dc6590ba893f3dde
This commit is contained in:
committed by
Facebook GitHub Bot
parent
5c82b9d860
commit
6b7b1fab5c
@@ -397,6 +397,7 @@ const Empty = () => (
|
|||||||
);
|
);
|
||||||
|
|
||||||
function renderRawBody(container: Request | Response) {
|
function renderRawBody(container: Request | Response) {
|
||||||
|
// TODO: we want decoding only for non-binary data! See D23403095
|
||||||
const decoded = decodeBody(container);
|
const decoded = decodeBody(container);
|
||||||
return (
|
return (
|
||||||
<BodyContainer>
|
<BodyContainer>
|
||||||
|
|||||||
101
desktop/plugins/network/__tests__/encoding.node.tsx
Normal file
101
desktop/plugins/network/__tests__/encoding.node.tsx
Normal file
@@ -0,0 +1,101 @@
|
|||||||
|
/**
|
||||||
|
* Copyright (c) Facebook, Inc. and its affiliates.
|
||||||
|
*
|
||||||
|
* This source code is licensed under the MIT license found in the
|
||||||
|
* LICENSE file in the root directory of this source tree.
|
||||||
|
*
|
||||||
|
* @format
|
||||||
|
*/
|
||||||
|
|
||||||
|
import {readFile} from 'fs';
|
||||||
|
import path from 'path';
|
||||||
|
import {decodeBody} from '../utils';
|
||||||
|
import {Response} from '../types';
|
||||||
|
import {promisify} from 'util';
|
||||||
|
import {readFileSync} from 'fs';
|
||||||
|
|
||||||
|
async function createMockResponse(input: string): Promise<Response> {
|
||||||
|
const inputData = await promisify(readFile)(
|
||||||
|
path.join(__dirname, 'fixtures', input),
|
||||||
|
'ascii',
|
||||||
|
);
|
||||||
|
const gzip = input.includes('gzip'); // if gzip in filename, assume it is a gzipped body
|
||||||
|
const testResponse: Response = {
|
||||||
|
id: '0',
|
||||||
|
timestamp: 0,
|
||||||
|
status: 200,
|
||||||
|
reason: 'dunno',
|
||||||
|
headers: gzip
|
||||||
|
? [
|
||||||
|
{
|
||||||
|
key: 'Content-Encoding',
|
||||||
|
value: 'gzip',
|
||||||
|
},
|
||||||
|
]
|
||||||
|
: [],
|
||||||
|
data: inputData.replace(/\s+?/g, '').trim(), // remove whitespace caused by copy past of the base64 data,
|
||||||
|
isMock: false,
|
||||||
|
insights: undefined,
|
||||||
|
totalChunks: 1,
|
||||||
|
index: 0,
|
||||||
|
};
|
||||||
|
return testResponse;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('network data encoding', () => {
|
||||||
|
const donatingExpected = readFileSync(
|
||||||
|
path.join(__dirname, 'fixtures', 'donating.md'),
|
||||||
|
'utf-8',
|
||||||
|
).trim();
|
||||||
|
const tinyLogoExpected = readFileSync(
|
||||||
|
path.join(__dirname, 'fixtures', 'tiny_logo.png'),
|
||||||
|
);
|
||||||
|
const tinyLogoBase64Expected = readFileSync(
|
||||||
|
path.join(__dirname, 'fixtures', 'tiny_logo.base64.txt'),
|
||||||
|
'utf-8',
|
||||||
|
);
|
||||||
|
|
||||||
|
test('donating.md.utf8.ios.txt', async () => {
|
||||||
|
const response = await createMockResponse('donating.md.utf8.ios.txt');
|
||||||
|
expect(decodeBody(response).trim()).toEqual(donatingExpected);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('donating.md.utf8.gzip.ios.txt', async () => {
|
||||||
|
const response = await createMockResponse('donating.md.utf8.gzip.ios.txt');
|
||||||
|
expect(decodeBody(response).trim()).toEqual(donatingExpected);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('donating.md.utf8.android.txt', async () => {
|
||||||
|
const response = await createMockResponse('donating.md.utf8.android.txt');
|
||||||
|
expect(decodeBody(response).trim()).toEqual(donatingExpected);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('donating.md.utf8.gzip.android.txt', async () => {
|
||||||
|
const response = await createMockResponse(
|
||||||
|
'donating.md.utf8.gzip.android.txt',
|
||||||
|
);
|
||||||
|
expect(decodeBody(response).trim()).toEqual(donatingExpected);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('tiny_logo.android.txt', async () => {
|
||||||
|
const response = await createMockResponse('tiny_logo.android.txt');
|
||||||
|
expect(response.data).toEqual(tinyLogoExpected.toString('base64'));
|
||||||
|
});
|
||||||
|
|
||||||
|
test('tiny_logo.android.txt - encoded', async () => {
|
||||||
|
const response = await createMockResponse('tiny_logo.android.txt');
|
||||||
|
// this compares to the correct base64 encoded src tag of the img in Flipper UI
|
||||||
|
expect(response.data).toEqual(tinyLogoBase64Expected.trim());
|
||||||
|
});
|
||||||
|
|
||||||
|
test('tiny_logo.ios.txt', async () => {
|
||||||
|
const response = await createMockResponse('tiny_logo.ios.txt');
|
||||||
|
expect(response.data).toEqual(tinyLogoExpected.toString('base64'));
|
||||||
|
});
|
||||||
|
|
||||||
|
test('tiny_logo.ios.txt - encoded', async () => {
|
||||||
|
const response = await createMockResponse('tiny_logo.ios.txt');
|
||||||
|
// this compares to the correct base64 encoded src tag of the img in Flipper UI
|
||||||
|
expect(response.data).toEqual(tinyLogoBase64Expected.trim());
|
||||||
|
});
|
||||||
|
});
|
||||||
3
desktop/plugins/network/__tests__/fixtures/donating.md
Normal file
3
desktop/plugins/network/__tests__/fixtures/donating.md
Normal file
@@ -0,0 +1,3 @@
|
|||||||
|
# 捐赠
|
||||||
|
|
||||||
|
MobX 是使您的项目成功的关键吗? 使用[捐赠按钮](https://mobxjs.github.io/mobx/donate.html)分享胜利!如果你留下一个名字,它将被添加到赞助商列表。
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
IyDmjZDotaAKCk1vYlgg5piv5L2/5oKo55qE6aG555uu5oiQ5Yqf55qE5YWz6ZSu5ZCX77yfIOS9 v+eUqFvmjZDotaDmjInpkq5dKGh0dHBzOi8vbW9ieGpzLmdpdGh1Yi5pby9tb2J4L2RvbmF0ZS5o dG1sKeWIhuS6q+iDnOWIqe+8geWmguaenOS9oOeVmeS4i+S4gOS4quWQjeWtl++8jOWug+Wwhuii q+a3u+WKoOWIsOi1nuWKqeWVhuWIl+ihqOOAggo=
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
H4sIAAAAAAAAAyWNXQvBUByH7/cpVm642e59B/dKLiwywpSjXDKsY6gl8hrjQkNGSeYtH8b5n51d +QoWl7+n5+kX4GnXYGeT4yKKFOXp6ECeL6pa7qThLa/u1KbYAH3hT2ievL4NxvDzWPC+5Pat2L+l nZbXs+NBGaFiKSyKeUWqZEtCOoPksiRklB8Qk0ohgVKCjPK5EGCN3HasPgO8+TxqsFbpfEaepjsY E6dNnCpxtmB0Ye+fdcCuw1Fjqx293EE3AR/ZeQ76BgYa4CFbWu+qyn0Bz88iqcgAAAA=
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
IyDmjZDotaAKCk1vYlgg5piv5L2/5oKo55qE6aG555uu5oiQ5Yqf55qE5YWz6ZSu5ZCX77yfIOS9v+eUqFvmjZDotaDmjInpkq5dKGh0dHBzOi8vbW9ieGpzLmdpdGh1Yi5pby9tb2J4L2RvbmF0ZS5odG1sKeWIhuS6q+iDnOWIqe+8geWmguaenOS9oOeVmeS4i+S4gOS4quWQjeWtl++8jOWug+Wwhuiiq+a3u+WKoOWIsOi1nuWKqeWVhuWIl+ihqOOAggo=
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
IyDmjZDotaAKCk1vYlgg5piv5L2/5oKo55qE6aG555uu5oiQ5Yqf55qE5YWz6ZSu5ZCX77yfIOS9v+eUqFvmjZDotaDmjInpkq5dKGh0dHBzOi8vbW9ieGpzLmdpdGh1Yi5pby9tb2J4L2RvbmF0ZS5odG1sKeWIhuS6q+iDnOWIqe+8geWmguaenOS9oOeVmeS4i+S4gOS4quWQjeWtl++8jOWug+Wwhuiiq+a3u+WKoOWIsOi1nuWKqeWVhuWIl+ihqOOAggo=
|
||||||
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
BIN
desktop/plugins/network/__tests__/fixtures/tiny_logo.png
Normal file
BIN
desktop/plugins/network/__tests__/fixtures/tiny_logo.png
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 24 KiB |
@@ -366,7 +366,7 @@ export function plugin(client: PluginClient<Events, Methods>) {
|
|||||||
headers[e.key] = e;
|
headers[e.key] = e;
|
||||||
});
|
});
|
||||||
|
|
||||||
// convert data
|
// convert data TODO: we only want this for non-binary data! See D23403095
|
||||||
const responseData =
|
const responseData =
|
||||||
response && response.data ? decodeBody(response) : null;
|
response && response.data ? decodeBody(response) : null;
|
||||||
|
|
||||||
@@ -607,6 +607,7 @@ function buildRow(
|
|||||||
)
|
)
|
||||||
.join('\n')}`;
|
.join('\n')}`;
|
||||||
|
|
||||||
|
// TODO: we want decoding only for non-binary data! See D23403095
|
||||||
const requestData = request.data ? decodeBody(request) : null;
|
const requestData = request.data ? decodeBody(request) : null;
|
||||||
const responseData =
|
const responseData =
|
||||||
response && response.data ? decodeBody(response) : null;
|
response && response.data ? decodeBody(response) : null;
|
||||||
|
|||||||
@@ -26,6 +26,7 @@
|
|||||||
"flipper-plugin": "0.59.0"
|
"flipper-plugin": "0.59.0"
|
||||||
},
|
},
|
||||||
"devDependencies": {
|
"devDependencies": {
|
||||||
"@types/pako": "^1.0.1"
|
"@types/pako": "^1.0.1",
|
||||||
|
"js-base64": "^3.5.2"
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -9,6 +9,7 @@
|
|||||||
|
|
||||||
import pako from 'pako';
|
import pako from 'pako';
|
||||||
import {Request, Response, Header} from './types';
|
import {Request, Response, Header} from './types';
|
||||||
|
import {Base64} from 'js-base64';
|
||||||
|
|
||||||
export function getHeaderValue(headers: Array<Header>, key: string): string {
|
export function getHeaderValue(headers: Array<Header>, key: string): string {
|
||||||
for (const header of headers) {
|
for (const header of headers) {
|
||||||
@@ -24,44 +25,35 @@ export function decodeBody(container: Request | Response): string {
|
|||||||
return '';
|
return '';
|
||||||
}
|
}
|
||||||
|
|
||||||
const b64Decoded = atob(container.data);
|
|
||||||
try {
|
try {
|
||||||
if (getHeaderValue(container.headers, 'Content-Encoding') === 'gzip') {
|
const isGzip =
|
||||||
// for gzip, use pako to decompress directly to unicode string
|
getHeaderValue(container.headers, 'Content-Encoding') === 'gzip';
|
||||||
return decompress(b64Decoded);
|
if (isGzip) {
|
||||||
|
try {
|
||||||
|
// The request is gzipped, so convert the base64 back to the raw bytes first,
|
||||||
|
// then inflate. pako will detect the BOM headers and return a proper utf-8 string right away
|
||||||
|
return pako.inflate(Base64.atob(container.data), {to: 'string'});
|
||||||
|
} catch (e) {
|
||||||
|
// on iOS, the stream send to flipper is already inflated, so the content-encoding will not
|
||||||
|
// match the actual data anymore, and we should skip inflating.
|
||||||
|
// In that case, we intentionally fall-through
|
||||||
|
if (!('' + e).includes('incorrect header check')) {
|
||||||
|
throw e;
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
// If this is not a gzipped request, assume we are interested in a proper utf-8 string.
|
||||||
return b64Decoded;
|
// - If the raw binary data in is needed, in base64 form, use container.data directly
|
||||||
|
// - either directly use container.data (for example)
|
||||||
|
return Base64.decode(container.data);
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
console.warn(
|
console.warn(
|
||||||
`Flipper failed to decode request/response body (size: ${b64Decoded.length}): ${e}`,
|
`Flipper failed to decode request/response body (size: ${container.data.length}): ${e}`,
|
||||||
);
|
);
|
||||||
return '';
|
return '';
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
function decompress(body: string): string {
|
|
||||||
const charArray = body.split('').map((x) => x.charCodeAt(0));
|
|
||||||
|
|
||||||
const byteArray = new Uint8Array(charArray);
|
|
||||||
|
|
||||||
try {
|
|
||||||
if (body) {
|
|
||||||
return pako.inflate(byteArray, {to: 'string'});
|
|
||||||
} else {
|
|
||||||
return body;
|
|
||||||
}
|
|
||||||
} catch (e) {
|
|
||||||
// Sometimes Content-Encoding is 'gzip' but the body is already decompressed.
|
|
||||||
// Assume this is the case when decompression fails.
|
|
||||||
if (!('' + e).includes('incorrect header check')) {
|
|
||||||
console.warn('decompression failed: ' + e);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return body;
|
|
||||||
}
|
|
||||||
|
|
||||||
export function convertRequestToCurlCommand(request: Request): string {
|
export function convertRequestToCurlCommand(request: Request): string {
|
||||||
let command: string = `curl -v -X ${request.method}`;
|
let command: string = `curl -v -X ${request.method}`;
|
||||||
command += ` ${escapedString(request.url)}`;
|
command += ` ${escapedString(request.url)}`;
|
||||||
@@ -70,7 +62,7 @@ export function convertRequestToCurlCommand(request: Request): string {
|
|||||||
const headerStr = `${header.key}: ${header.value}`;
|
const headerStr = `${header.key}: ${header.value}`;
|
||||||
command += ` -H ${escapedString(headerStr)}`;
|
command += ` -H ${escapedString(headerStr)}`;
|
||||||
});
|
});
|
||||||
// Add body
|
// Add body. TODO: we only want this for non-binary data! See D23403095
|
||||||
const body = decodeBody(request);
|
const body = decodeBody(request);
|
||||||
if (body) {
|
if (body) {
|
||||||
command += ` -d ${escapedString(body)}`;
|
command += ` -d ${escapedString(body)}`;
|
||||||
|
|||||||
@@ -8327,6 +8327,11 @@ jest@^26:
|
|||||||
import-local "^3.0.2"
|
import-local "^3.0.2"
|
||||||
jest-cli "^26.5.2"
|
jest-cli "^26.5.2"
|
||||||
|
|
||||||
|
js-base64@^3.5.2:
|
||||||
|
version "3.5.2"
|
||||||
|
resolved "https://registry.yarnpkg.com/js-base64/-/js-base64-3.5.2.tgz#3cc800e4f10812b55fb5ec53e7cabaef35dc6d3c"
|
||||||
|
integrity sha512-VG2qfvV5rEQIVxq9UmAVyWIaOdZGt9M16BLu8vFkyWyhv709Hyg4nKUb5T+Ru+HmAr9RHdF+kQDKAhbJlcdKeQ==
|
||||||
|
|
||||||
js-message@1.0.5:
|
js-message@1.0.5:
|
||||||
version "1.0.5"
|
version "1.0.5"
|
||||||
resolved "https://registry.yarnpkg.com/js-message/-/js-message-1.0.5.tgz#2300d24b1af08e89dd095bc1a4c9c9cfcb892d15"
|
resolved "https://registry.yarnpkg.com/js-message/-/js-message-1.0.5.tgz#2300d24b1af08e89dd095bc1a4c9c9cfcb892d15"
|
||||||
|
|||||||
Reference in New Issue
Block a user