[Extensions] Sanitize extension short name in LoadShortName [chromium/src : main]
0 views
Skip to first unread message
Andrea Orru (Gerrit)
May 29, 2026, 8:21:36 PM (3 days ago) May 29
to Solomon Kinard, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Solomon Kinard
Andrea Orru voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Solomon Kinard
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Solomon Kinard (Gerrit)
1:23 PM (4 hours ago) 1:23 PM
to Andrea Orru, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Andrea Orru
Solomon Kinard added 1 comment![]( "Open in Gerrit")
File extensions/common/extension.cc
Line 777, Patchset 2 (Latest):
localized_short_name = base::CollapseWhitespace(localized_short_name, true);
base::i18n::SanitizeUserSuppliedString(&localized_short_name);
base::i18n::AdjustStringForLocaleDirection(&localized_short_name);
short_name_ = base::UTF16ToUTF8(localized_short_name);
} else {
short_name_ = display_name_;
}Solomon Kinard . unresolved
Thoughts on a catch all so that `display_name_` can get the same sanitization, for consistency?
```suggestion
base::i18n::AdjustStringForLocaleDirection(&localized_short_name);
short_name_ = base::UTF16ToUTF8(localized_short_name);
} else {
short_name_ = display_name_;
}
// Now that short_name_ is populated from either branch, sanitize it.
std::u16string sanitized_short_name = base::UTF8ToUTF16(short_name_);
sanitized_short_name = base::CollapseWhitespace(sanitized_short_name, true);
base::i18n::SanitizeUserSuppliedString(&sanitized_short_name);
base::i18n::AdjustStringForLocaleDirection(&sanitized_short_name);
short_name_ = base::UTF16ToUTF8(sanitized_short_name);
```
Related details
Attention is currently required from:
- Andrea Orru
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrea Orru (Gerrit)
1:35 PM (3 hours ago) 1:35 PM
to Solomon Kinard, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Solomon Kinard
Andrea Orru added 1 comment![]( "Open in Gerrit")
File extensions/common/extension.cc
Line 777, Patchset 2 (Latest):
localized_short_name = base::CollapseWhitespace(localized_short_name, true);
base::i18n::SanitizeUserSuppliedString(&localized_short_name);
base::i18n::AdjustStringForLocaleDirection(&localized_short_name);
short_name_ = base::UTF16ToUTF8(localized_short_name);
} else {
short_name_ = display_name_;
}Solomon Kinard . unresolved
Thoughts on a catch all so that `display_name_` can get the same sanitization, for consistency?
```suggestion
base::i18n::AdjustStringForLocaleDirection(&localized_short_name);
short_name_ = base::UTF16ToUTF8(localized_short_name);
} else {
short_name_ = display_name_;
}// Now that short_name_ is populated from either branch, sanitize it.
std::u16string sanitized_short_name = base::UTF8ToUTF16(short_name_);
sanitized_short_name = base::CollapseWhitespace(sanitized_short_name, true);
base::i18n::SanitizeUserSuppliedString(&sanitized_short_name);
base::i18n::AdjustStringForLocaleDirection(&sanitized_short_name);
short_name_ = base::UTF16ToUTF8(sanitized_short_name);```
Andrea Orru
I don't think that's necessary. `display_name_` is already sanitized by `LoadName`, so the else branch is safe as-is.
Related details
Attention is currently required from:
- Solomon Kinard
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Solomon Kinard (Gerrit)
1:53 PM (3 hours ago) 1:53 PM
to Andrea Orru, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Andrea Orru
Solomon Kinard voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Andrea Orru
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrea Orru (Gerrit)
3:53 PM (1 hour ago) 3:53 PM
to Solomon Kinard, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Andrea Orru voted and added 1 comment![]( "Open in Gerrit")
Votes added by Andrea Orru
| Commit-Queue | +2 |
1 comment
File extensions/common/extension.cc
Line 777, Patchset 2 (Latest):
localized_short_name = base::CollapseWhitespace(localized_short_name, true);
base::i18n::SanitizeUserSuppliedString(&localized_short_name);
base::i18n::AdjustStringForLocaleDirection(&localized_short_name);
short_name_ = base::UTF16ToUTF8(localized_short_name);
} else {
short_name_ = display_name_;
}Solomon Kinard . resolved
Andrea OrruThoughts on a catch all so that `display_name_` can get the same sanitization, for consistency?
```suggestion
base::i18n::AdjustStringForLocaleDirection(&localized_short_name);
short_name_ = base::UTF16ToUTF8(localized_short_name);
} else {
short_name_ = display_name_;
}// Now that short_name_ is populated from either branch, sanitize it.
std::u16string sanitized_short_name = base::UTF8ToUTF16(short_name_);
sanitized_short_name = base::CollapseWhitespace(sanitized_short_name, true);
base::i18n::SanitizeUserSuppliedString(&sanitized_short_name);
base::i18n::AdjustStringForLocaleDirection(&sanitized_short_name);
short_name_ = base::UTF16ToUTF8(sanitized_short_name);```
I don't think that's necessary. `display_name_` is already sanitized by `LoadName`, so the else branch is safe as-is.
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages