• 
      

    Require SafeStrings for all HTML in review request fields.

    Review Request #14299 — Created Jan. 22, 2025 and submitted — Latest diff uploaded

    Information

    Review Board
    release-7.1.x

    Reviewers

    We've had a long-standing task to enforce HTML safety in review request
    fields. We've trusted that all HTML strings were safe, but with TODO
    comments saying we need to move to SafeString enforcement.

    This change makes that move. We now expect SafeString results from all
    rendering functions for views. Since this is a breaking change, it's a
    soft requirement. We convert native strings to SafeStrings when found,
    but with a deprecation warning.

    This gave me the opportunity to clean up some of our HTML rendering code
    to be a bit more manageable.

    All built-in fields have been updated to ensure SafeString results.

    Note that affected functions have not received any signature updates.
    Those will be handled separately as part of a larger change.

    All unit tests pass.

    Tested a review request with all the fields. Verified that they all
    rendered their contents correctly on the review request and the change
    descriptions.

    Tested this with and without the updates to the built-in fields.
    Without those updates, I saw the warnings in the console but the fields
    rendered as expected.

    Commits

    Files