Skip to content

Commit

Permalink
Don't escape and decode network response payloads
Browse files Browse the repository at this point in the history
Summary:
This was originally done in #377 to support chinese characters, but it's not clear how it works, or what problem specifically it is trying to solve.

It's causing some problems where valid responses are failing to be "decoded", so I'm removing it.

Reviewed By: mweststrate

Differential Revision: D22866879

fbshipit-source-id: a19a57b0ddba2b9f434eb3c37e6b92da1dfbef01
  • Loading branch information
jknoxville authored and facebook-github-bot committed Aug 3, 2020
1 parent 6458541 commit 45d461a
Showing 1 changed file with 1 addition and 3 deletions.
4 changes: 1 addition & 3 deletions desktop/plugins/network/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ export function decodeBody(container: Request | Response): string {
return decompress(b64Decoded);
}

// Data is transferred as base64 encoded bytes to support unicode characters,
// we need to decode the bytes here to display the correct unicode characters.
return decodeURIComponent(escape(b64Decoded));
return b64Decoded;
} catch (e) {
console.warn('Discarding malformed body, size: ' + b64Decoded.length);
return '';
Expand Down

4 comments on commit 45d461a

@skychx
Copy link

@skychx skychx commented on 45d461a Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixed is wrong, this PR caused UTF8 characters to not parse properly.

For example, the BASE64 encoding for the character 東京 is 5p2x5Lqs; if you use atob('5p2x5Lqs') to decode it, you will get æ±äº¬, which is obviously wrong.

The essential reason is that atob/btoa does not support conversion between UTF8 characters and BASE64 characters

@skychx
Copy link

@skychx skychx commented on 45d461a Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@passy
Copy link
Member

@passy passy commented on 45d461a Aug 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing this to our attention, @skychx! Would you mind opening an issue, linking to this commit so our oncall can triage this?

@skychx
Copy link

@skychx skychx commented on 45d461a Aug 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for bringing this to our attention, @skychx! Would you mind opening an issue, linking to this commit so our oncall can triage this?

Yes, I've created a new issue detailing the Network parsing problem here.

Please sign in to comment.