SafeMarkup::set() were converted to
SafeMarkup::format(). This makes sense. However, the added use of
SafeMarkup::format() is not best practice because it passes a variable as the first argument:
$this->line = SafeMarkup::format($format_string, ['@original_line' => $this->line, '@group' => $this->group]);
This is not technically incorrect because
$format_string is defined as one or the other of two string literals in an if/else immediately above and is composed of
@ tokens (so there is no chance of unsanitized user input in the variable), and since it's not a translated string, we also don't need to worry about scanning for the translations. However, the code would set a better example (and be easier to read and security audit) if
SafeMarkup::format were just used with the appropriate pattern in the two different codepaths.
SafeMarkup::format with the appropriate pattern in the two different codepaths. Patch attached.
Beta phase evaluation
|Issue category||Task because there is not actually anything functionally wrong with the current code.|
|Issue priority||Normal; it's not that big of a deal and also in part of component code that's barely not vendor code.|
|Prioritized changes||This issue is a followup for a recent major which is also part of an ongoing critical.|
User interface changes