• 
      

    Rework encoding of Revisions during diff processing to avoid regressions.

    Review Request #12825 — Created Feb. 7, 2023 and submitted — Latest diff uploaded

    Information

    Review Board
    release-4.0.x

    Reviewers

    Due to a bug during a refactor late in 4.0.x, we were attempting to
    store a raw Revision object in extra_data. The intent of the code
    was to convert a revision to something we could store by using
    convert_to_unicode() and factoring in the repository's configured
    encoding list. However, this didn't know how to convert Revision
    objects.

    We received a patch that addresses this by performing a conversion
    before passing it in, which guaranteed we'd get a string out of it. This
    used force_str()/force_text(). However, that performs its decoding
    from byte strings to Unicode strings, and makes its own assumptions
    about the encoding type that should be used. That could result in an
    incorrectly-decoded string.

    This isn't a major issue for revisions on most SCMs, which are generally
    using ASCII (though, there's no guarantee here). However, the filename
    was also force-decoded prematurely, which could impact some setups.

    This change removes the force-decoding for filenames entirely, which was
    probably just added as a "just in case" measure, and then does a very
    explicit Revision to bytes conversion before passing to
    convert_to_unicode(). This is the safest way to handle this, avoiding
    the heuristics in Django's string conversion machinery, and gets us the
    result we want.

    Unit tests were added to cover this case.

    Unit tests pass. Verified the new test case failed with the reported
    error without the fix.

    Commits

    Files