Fix diffing of rendered text file attachments on Python 3.

Review Request #10709 — Created Sept. 10, 2019 and submitted

Information

Review Board
release-4.0.x
9947e50...

Reviewers

Rendered text file attachments (such as Markdown) are rendered as
Unicode, for output to the client. These were getting passed directly to
the differ, which itself takes byte strings. On Python 2, this all
worked out fine. On Python 3, this blew up pretty early, but in a way
that was hard to diagnose due any errors coming from subclasses just
being logged (with a massively-huge log message) and then the exception
ignored.

The Markdown review UI specifically had another issue where we were
passing in a file handle containing byte strings, which was then getting
written back as byte strings but to a StringIO expecting Unicode
strings. While the core issue lives in Djblets (and there's another
patch up that aims to fix this), it's better if we just read the file
content and convert to Unicode first, since the Markdown library doesn't
stream the file pointer anyway (it just grabs all the data at once).

This change most of these issues by doing the following:

1) Ensures we're always passing the right string types around (both to
Markdown and the diff chunk generators).

2) Switches the Markdown rendering call to render_markdown(), passing
it a Unicode string instead of a file handle.

3) Adds new unit tests covering all this.

It does not address the overzealous exception sandboxing. That will be
taken care of separately.

Unit tests passed.

Verified I could view Markdown files in both rendered and source mode,
with diffs.

Description From Last Updated

E127 continuation line over-indented for visual indent

reviewbotreviewbot

E127 continuation line over-indented for visual indent

reviewbotreviewbot

Needs a Returns section.

daviddavid
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

chipx86
chipx86
amalik2
  1. LGTM

  2. I tested out some of these changes in my branch for the XML viewer, it got the rendering working perfectly with both Python 2 and 3. LGTM

  3. 
      
david
  1. 
      
  2. reviewboard/reviews/ui/text.py (Diff revision 3)
     
     

    Needs a Returns section.

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

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (ebbb292)
Loading...