Require SafeStrings for all HTML in review request fields.

Review Request #14299 — Created Jan. 22, 2025 and updated

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.

Summary ID
Require SafeStrings for all HTML in review request fields.
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 `SafeString`s 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.
10324cb28665867db99b4d46528527eee22a4ba1
Description From Last Updated

Need a version changed for making this keyword-only.

maubinmaubin

Need a version changed for making this keyword-only.

maubinmaubin

Need a version changed for making this keyword-only.

maubinmaubin

Missing a version added.

maubinmaubin
chipx86
Review request changed
Change Summary:

Fixed a call from format_html_join -> format_html.

Commits:
Summary ID
Require SafeStrings for all HTML in review request fields.
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 `SafeString`s 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.
5659c51b5ef2e4a694cd8150e176d8bd239a93d6
Require SafeStrings for all HTML in review request fields.
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 `SafeString`s 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.
10324cb28665867db99b4d46528527eee22a4ba1
Diff:

Revision 2 (+978 -352)

Show changes

reviewboard/reviews/builtin_fields.py
reviewboard/reviews/fields.py

Checks run (1 timed out)

JSHint timed out.
maubin
  1. 
      
  2. reviewboard/reviews/fields.py (Diff revision 2)
     
     
    Show all issues

    Need a version changed for making this keyword-only.

  3. reviewboard/reviews/fields.py (Diff revision 2)
     
     
    Show all issues

    Need a version changed for making this keyword-only.

  4. reviewboard/reviews/fields.py (Diff revision 2)
     
     
    Show all issues

    Need a version changed for making this keyword-only.

  5. reviewboard/reviews/fields.py (Diff revision 2)
     
     
    Show all issues

    Missing a version added.

  6. 
      
Loading...