Fix Unicode parsing issues with diff fragment and entry update payloads.

Review Request #9450 — Created Dec. 18, 2017 and submitted

Information

Review Board
release-3.0.x
995ff33...

Reviewers

Review Board 3.0 introduced a more efficient way of transferring diff
fragments for comments, and a mechanism for providing updates to entries
on the review request page. These used a streamable format that encoded
byte lengths followed by content.

The problem is, JavaScript doesn't deal with strings as binary data,
they deal with them as Unicode data. This meant that our byte counts
would be wildly off whenever any multi-byte characters were introduced
in any of the content sections, causing various breakages.

A long-term (and in-progress) solution is to switch to interpreting
these payloads as binary content by using JavaScript's ArrayBuffer and
DataView objects, which would allow any form of content to be made
available in the payload. For now, though, since these are all UTF-8
string content, we can resolve this issue by storing character counts
and not byte counts.

Python and JavaScript unit tests pass on Chrome, Firefox, and IE.

Manually tested Unicode content in updates and diff fragments. They
loaded correctly.

Description From Last Updated

Seems a little silly to define variables for these. Why not just payload.write(metadata.encode('utf-8'))?

daviddavid

Same here re: defining a single-use variable.

daviddavid
chipx86
david
  1. 
      
  2. reviewboard/reviews/views.py (Diff revision 1)
     
     
     
    Show all issues

    Seems a little silly to define variables for these. Why not just payload.write(metadata.encode('utf-8'))?

    1. Had it during some debugging.

  3. reviewboard/reviews/views.py (Diff revision 1)
     
     
    Show all issues

    Same here re: defining a single-use variable.

  4. 
      
chipx86
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (4f73586)
Loading...