Fix up some string types.

Review Request #12106 — Created March 7, 2022 and submitted

david
Review Board
release-5.0.x
reviewboard

This change fixes up a handful of issues related to string types. Some
of these are more important than others.

For a couple places, we had code deep in the diff rendering which was
creating a SafeString. Because we have caching at many levels in the
diff viewer, these values would end up trying to go through pickle to
store the rendered data. When pickle encounters a value that it doesn't
understand, it tries to call str() on it. With the older Django
version, this would correctly convert, but with the modern Django,
calling str() on a SafeString would return the SafeString again.
This would just end up infinitely recursing inside of the pickle
machinery. There are two places where this affects. For the first, the
fix is to remove some old mark_safe calls. For the second (where we
were rendering a template), the solution is to explicitly convert to
str by using a slice trick.

Next, the HTTP digest authentication backend was calling MD5, but
passing in an str instead of bytes. We also had a handful of other
places where there was some confusion about bytes/str which was found
via static analysis.

Finally, a bunch of unit tests were passing in int types for the
revision when creating test filediffs. These do end up getting cast to
str, but since we say the type of the argument is str, it's more correct
to pass that in directly.

  • Created a review request with a diff and was able to successfully load
    the diff viewer page.
  • Ran unit tests.
Summary
Fix up some string types.
Description From Last Updated

I saw the bit in the description about this, but I really don't like it. This is a valid part …

chipx86chipx86
chipx86
  1. 
      
  2. I saw the bit in the description about this, but I really don't like it. This is a valid part of the contract. We should be allowed to do this, and the tool is just wrong. I don't want to cater to a misinformed tool. What's reporting the error?

    1. The thing which I've been playing with is pyright, but I figured given that create_filediff says source_revision should be str that this was a correct fix. I'm happy to undo the source_revision changes for Revision objects (though I think it's still correct for the changes that change int -> str).

    2. Yeah int -> str is absolutely correct.

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

Status: Closed (submitted)

Change Summary:

Pushed to django-3.2 (dcd61ac)
Loading...