Add HTML sanitizer for translated message resources#7
Conversation
Closes #37428 Signed-off-by: Alexander Schwartz <alexander.schwartz@gmx.net>
WalkthroughThis update introduces enhanced HTML safety verification for localized property files, adds supporting dependencies, and expands test coverage for message property validation. Numerous localization files are refined for pluralization, HTML correctness, and formatting consistency, with several TOTP and WebAuthn instructions generalized and HTML tags standardized across multiple languages. Changes
Sequence Diagram(s)sequenceDiagram
participant Verifier as VerifyMessageProperties
participant PropertiesFile as .properties file
participant EnglishSource as English .properties file
participant Sanitizer as HTML Sanitizer
Verifier->>PropertiesFile: Load translation properties
Verifier->>EnglishSource: Load English source properties
loop For each key in properties
Verifier->>Verifier: normalizeValue(key, value)
Verifier->>Verifier: getEnglishValue(key, EnglishSource)
Verifier->>Verifier: santizeAnchors(key, value, englishValue)
Verifier->>Verifier: containsHtml(englishValue)
alt English value has HTML
Verifier->>Sanitizer: Allow limited HTML tags
else No HTML allowed
Verifier->>Sanitizer: Disallow all HTML tags
end
Sanitizer-->>Verifier: Sanitized value
Verifier->>Verifier: Compare sanitized vs original
alt Difference found
Verifier->>Verifier: Record error for illegal HTML
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (12)
themes/src/main/resources-community/theme/base/email/messages/messages_hu.properties (1)
6-6: HTML fix looks correct – consider descriptive anchor text for accessibilityThe removal of the stray prefix and the resulting markup is syntactically sound (all tags close properly, placeholders remain intact).
For better accessibility and phishing-prevention UX, you could change the visible text of the<a>element from the raw URL ({0}) to a descriptive phrase (e.g. „Hivatkozás jóváhagyáshoz”), while keeping the samehref.
This keeps e-mails cleaner and reduces the chance that the rendered link is wrapped or filtered by mail clients.themes/src/main/resources-community/theme/base/email/messages/messages_da.properties (1)
9-9: Extraneous character and mixed-language phrase in last paragraphThe final
<p>starts withnHvis(leading stray ‘n’) and contains the English word “ignore”.
This will surface directly to Danish end-users and looks unpolished.-<p>nHvis du ikke vil forbinde disse konti, kan du bare ignore denne besked. Hvis du vælger at forbinde de to konti, kan du logge ind som {1} via {0}.</p> +<p>Hvis du ikke vil forbinde disse konti, kan du blot ignorere denne besked. Hvis du vælger at forbinde de to konti, kan du logge ind som {1} via {0}.</p>Removes the rogue character, restores full Danish wording (“blot ignorere”), and keeps placeholders intact.
Please mirror the same fix in other locale files if they were generated from the same template.themes/src/main/resources-community/theme/base/email/messages/messages_uk.properties (1)
6-6: Optional: addrel="noopener noreferrer"for safer linksTo harden against reverse-tabnabbing and similar issues, consider adding
rel="noopener noreferrer"(and, if desired,target="_blank") to the anchor.-<p><a href="{0}">{0}</a></p> +<p><a href="{0}" rel="noopener noreferrer">{0}</a></p>themes/src/main/resources-community/theme/base/email/messages/messages_tr.properties (1)
9-9: Remove stray spaces inside the<b>tags.
<b> {1} </b>and<b> {0} </b>render an extra space before/after the bolded placeholder, which is inconsistent with the other locales and may be flagged by the new HTML-sanitizer logic.-identityProviderLinkBodyHtml=<p>Birisi <b> {1} </b> hesabınızı {2} kullanıcısı <b> {0} </b> hesabına bağlamak istiyor. Bu sizseniz, bağlantı vermek için aşağıdaki bağlantıyı tıklayın</p><p><a href="{3}">Hesap bağlantısını onaylamak için bağlantı</a></p><p>Bu bağlantının süresi {5} içerisinde sona erecek.</p><p>Hesabı bağlamak istemiyorsanız, bu mesajı göz ardı edin. Hesapları bağlarsanız, {1} ile {0} arasında oturum açabilirsiniz.</p> +identityProviderLinkBodyHtml=<p>Birisi <b>{1}</b> hesabınızı {2} kullanıcısı <b>{0}</b> hesabına bağlamak istiyor. Bu sizseniz, bağlantı vermek için aşağıdaki bağlantıyı tıklayın</p><p><a href="{3}">Hesap bağlantısını onaylamak için bağlantı</a></p><p>Bu bağlantının süresi {5} içerisinde sona erecek.</p><p>Hesabı bağlamak istemiyorsanız, bu mesajı göz ardı edin. Hesapları bağlarsanız, {1} ile {0} arasında oturum açabilirsiniz.</p>misc/theme-verifier/src/test/resources/changedAnchor_en.properties (1)
17-17: Consider using HTTPS even in test fixturesAlthough this is only a test resource, replacing
http://example.comwithhttps://example.comavoids accidental reliance on insecure URLs and keeps the sample data aligned with best-practice defaults.-key=Some <a href="http://example.com">link</a> +key=Some <a href="https://example.com">link</a>misc/theme-verifier/src/test/resources/illegalHtmlTag_en.properties (1)
17-17: Ensure file ends with a trailing newline
illegalHtmlTag_en.propertiescurrently terminates on Line 17 without a final newline character. While most tools tolerate this, POSIX‐style text files are expected to end with\n, and some diff or concatenation utilities give warnings otherwise.
Adding a trailing newline avoids such edge-case tooling issues.-key=Some <div>tag</div +key=Some <div>tag</div\nmisc/theme-verifier/pom.xml (1)
75-85: Keep dependency scope declarations consistentYou explicitly set
<scope>compile</scope>forcommons-text, but left the OWASP Sanitizer to rely on Maven’s default (alsocompile). Mixing explicit vs implicit scope declarations in the same block creates pointless noise in diffs and hinders quick visual scanning.Pick one convention and apply it to both dependencies. A minimal patch:
- <dependency> - <groupId>com.googlecode.owasp-java-html-sanitizer</groupId> - <artifactId>owasp-java-html-sanitizer</artifactId> - <version>20240325.1</version> - </dependency> + <dependency> + <groupId>com.googlecode.owasp-java-html-sanitizer</groupId> + <artifactId>owasp-java-html-sanitizer</artifactId> + <version>20240325.1</version> + <scope>compile</scope> + </dependency>(or drop the explicit
scopeelement fromcommons-text).
Pure style, but worth tightening for long-term maintainability.themes/src/main/resources-community/theme/base/login/messages/messages_ca.properties (1)
423-427: Inconsistent spacing after<br />Lines 426-427 omit the space before
{0}(<br />{0}vs<br /> {0}elsewhere). For visual consistency across messages and locales, add the space:-webauthn-error-auth-verification=El resultat de l'autenticació amb clau de seguretat no és vàlid.<br />{0} -webauthn-error-register-verification=El resultat del registre amb clau de seguretat no és vàlid.<br />{0} +webauthn-error-auth-verification=El resultat de l'autenticació amb clau de seguretat no és vàlid.<br /> {0} +webauthn-error-register-verification=El resultat del registre amb clau de seguretat no és vàlid.<br /> {0}themes/src/main/resources-community/theme/base/login/messages/messages_th.properties (1)
462-466: Remove superfluous space after<br />for HTML-consistencyA few other languages (e.g.
messages_ka.properties) omit the space, so the rendered result is identical but the markup becomes consistent across locales.-webauthn-error-registration=การลงทะเบียนคีย์รักษาความปลอดภัยของคุณไม่สำเร็จ<br /> {0} +webauthn-error-registration=การลงทะเบียนคีย์รักษาความปลอดภัยของคุณไม่สำเร็จ<br />{0} -webauthn-error-api-get=การพิสูจน์ตัวจริงโดยใช้คีย์รักษาความปลอดภัยไม่สำเร็จ<br /> {0} +webauthn-error-api-get=การพิสูจน์ตัวจริงโดยใช้คีย์รักษาความปลอดภัยไม่สำเร็จ<br />{0} -webauthn-error-auth-verification=ผลการพิสูจน์ตัวจริงของคีย์รักษาความปลอดภัยไม่ถูกต้อง<br /> {0} +webauthn-error-auth-verification=ผลการพิสูจน์ตัวจริงของคีย์รักษาความปลอดภัยไม่ถูกต้อง<br />{0} -webauthn-error-register-verification=ผลการลงทะเบียนของคีย์รักษาความปลอดภัยไม่ถูกต้อง<br /> {0} +webauthn-error-register-verification=ผลการลงทะเบียนของคีย์รักษาความปลอดภัยไม่ถูกต้อง<br />{0}themes/src/main/resources-community/theme/base/login/messages/messages_tr.properties (1)
469-473: Consistent<br />self-closing tag – good, but trailing comma looks like a typo👍 for switching to the XHTML-style
<br />that the new HTML-sanitiser expects.While touching the line, consider removing the trailing comma after the
{0}placeholder – it renders literally in the UI:-webauthn-error-registration= Geçiş Anahatarınızı kaydederken hata oluştu.<br /> {0}, +webauthn-error-registration= Geçiş Anahtarınızı kaydederken hata oluştu.<br /> {0}Same for 470, 472, 473.
[nitpick]js/apps/account-ui/maven-resources/theme/keycloak.v3/account/messages/messages_en.properties (1)
188-188: Pluralisation pattern works, but double–0 rule is redundant
{2,choice,0#values|1#value|1<values}is fine; for clarity you could drop the0#rule – the1<branch already covers 0 as well in ChoiceFormat. Optional, not blocking.-error-invalid-multivalued-size=Attribute {0} must have at least {1} and at most {2} {2,choice,0#values|1#value|1<values}. +error-invalid-multivalued-size=Attribute {0} must have at least {1} and at most {2} {2,choice,1#value|1<values}.misc/theme-verifier/src/main/java/org/keycloak/themeverifier/VerifyMessageProperties.java (1)
117-137: Consider making special case handling more maintainableThe hardcoded key names and patterns make this method brittle. Consider externalizing these rules or using a more flexible configuration approach.
// Consider defining these patterns at class level private static final Map<String, Pattern> KEY_SPECIFIC_PATTERNS = Map.of( "templateHelp", Pattern.compile("CLAIM\\.<[A-Z]*>"), "optimizeLookupHelp", Pattern.compile("<Extensions>") ); private static final Pattern TIME_PERIOD_KEY_PATTERN = Pattern.compile("^linkExpirationFormatter\\.timePeriodUnit.*|error-invalid-multivalued-size$");This would make it easier to add new special cases and understand the rules at a glance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
js/apps/account-ui/maven-resources/theme/keycloak.v3/account/messages/messages_en.properties(1 hunks)js/apps/admin-ui/maven-resources-community/theme/keycloak.v2/admin/messages/messages_zh_CN.properties(1 hunks)js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties(1 hunks)misc/theme-verifier/pom.xml(1 hunks)misc/theme-verifier/src/main/java/org/keycloak/themeverifier/VerifyMessageProperties.java(2 hunks)misc/theme-verifier/src/test/java/org/keycloak/themeverifier/VerifyMessagePropertiesTest.java(1 hunks)misc/theme-verifier/src/test/resources/changedAnchor_de.properties(1 hunks)misc/theme-verifier/src/test/resources/changedAnchor_en.properties(1 hunks)misc/theme-verifier/src/test/resources/illegalHtmlTag_en.properties(1 hunks)misc/theme-verifier/src/test/resources/noHtml_de.properties(1 hunks)misc/theme-verifier/src/test/resources/noHtml_en.properties(1 hunks)themes/src/main/resources-community/theme/base/account/messages/messages_fi.properties(1 hunks)themes/src/main/resources-community/theme/base/account/messages/messages_lt.properties(1 hunks)themes/src/main/resources-community/theme/base/account/messages/messages_sk.properties(1 hunks)themes/src/main/resources-community/theme/base/account/messages/messages_sv.properties(1 hunks)themes/src/main/resources-community/theme/base/account/messages/messages_zh_CN.properties(1 hunks)themes/src/main/resources-community/theme/base/email/messages/messages_da.properties(1 hunks)themes/src/main/resources-community/theme/base/email/messages/messages_fa.properties(1 hunks)themes/src/main/resources-community/theme/base/email/messages/messages_hu.properties(1 hunks)themes/src/main/resources-community/theme/base/email/messages/messages_lt.properties(1 hunks)themes/src/main/resources-community/theme/base/email/messages/messages_nl.properties(1 hunks)themes/src/main/resources-community/theme/base/email/messages/messages_no.properties(1 hunks)themes/src/main/resources-community/theme/base/email/messages/messages_tr.properties(1 hunks)themes/src/main/resources-community/theme/base/email/messages/messages_uk.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_ar.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_ca.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_cs.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_de.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_el.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_es.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_fa.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_fi.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_fr.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_hr.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_ka.properties(2 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_ko.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_lt.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_no.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_sk.properties(4 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_sv.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_th.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_tr.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_uk.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_zh_CN.properties(1 hunks)themes/src/main/resources-community/theme/base/login/messages/messages_zh_TW.properties(1 hunks)themes/src/main/resources/theme/base/admin/messages/messages_en.properties(1 hunks)themes/src/main/resources/theme/base/login/messages/messages_en.properties(2 hunks)
🔇 Additional comments (38)
themes/src/main/resources-community/theme/base/email/messages/messages_uk.properties (1)
6-6: LGTM – removed stray space fixes malformed closing tagThe corrected
</p>eliminates invalid HTML, ensuring proper rendering across email clients.themes/src/main/resources/theme/base/admin/messages/messages_en.properties (1)
69-69: Pluralisation logic duplicates the max-value parameter.The current text renders e.g.
“at most 4 4 values”, repeating{2}before the choice.
Drop the standalone{2}and embed the numeric token inside thechoiceblock so the number and word appear only once.-error-invalid-multivalued-size=Attribute {0} must have at least {1} and at most {2} {2,choice,0#values|1#value|1<values}. +error-invalid-multivalued-size=Attribute {0} must have at least {1} and at most {2,choice,0#0 values|1#1 value|1< # values}.This yields “at most 4 values” and “at most 1 value” as expected.
Likely an incorrect or invalid review comment.
js/apps/admin-ui/maven-resources/theme/keycloak.v2/admin/messages/messages_en.properties (1)
3136-3136: Plural-aware rewrite looks correctThe switch to MessageFormat placeholders (
{0},{1},{2}) and the{2,choice,…}expression will render the proper singular/plural form:
…at most 1 value…at most 2 values…at most 0 valuesNo escaping or quote-escaping issues are introduced, and re-using index 2 twice is allowed by
MessageFormat.themes/src/main/resources-community/theme/base/login/messages/messages_el.properties (1)
418-422: HTML line-break standardisation looks good.Switching from
<br/>to<br />aligns with the sanitizer’s whitelist and the rest of the code-base. No further issues.themes/src/main/resources-community/theme/base/email/messages/messages_nl.properties (1)
3-3: Fixed missing>– LGTM.The malformed
<p>tag is now well-formed; this avoids parser errors and passes the new verifier.misc/theme-verifier/src/test/resources/noHtml_de.properties (1)
17-17: Test resource correctly exercises “no-HTML” rule.Having
<b>markup in a noHtml locale file is a useful negative test for the new verifier. File header & licence blocks also comply with project standards.themes/src/main/resources-community/theme/base/login/messages/messages_ko.properties (1)
480-484: Consistent<br />formatting – looks good.The four WebAuthn error strings now match the HTML style enforced across locales; no issues spotted.
themes/src/main/resources-community/theme/base/login/messages/messages_ar.properties (1)
466-470: Consistent self-closing<br />tag – looks goodThe switch from
<br/>to<br />aligns with W3C-style self-closing tags used elsewhere in the project and with the new HTML-sanitizer rules. No further action required.misc/theme-verifier/src/test/resources/changedAnchor_de.properties (1)
17-17: Intentionally divergent anchor – OK for negative testThe
http://malicious.comanchor differs from the English source and is evidently used to assert the “changed anchor” detection path. No issues.misc/theme-verifier/src/test/resources/noHtml_en.properties (1)
17-17: No HTML scenario fixture – looks fineThe plain-text value correctly represents a no-HTML case for the verifier tests.
themes/src/main/resources-community/theme/base/email/messages/messages_fa.properties (1)
6-6: Removed stray space in<p>tag – good catchThe fix from
< p>to<p>prevents malformed HTML in outgoing emails and will satisfy the new sanitizer. ✅themes/src/main/resources-community/theme/base/login/messages/messages_fa.properties (1)
457-462: Consistent<br />spacing – looks goodThe switch from
<br/>to<br />aligns these messages with HTML-5 style and the new sanitizer. No further issues spotted.themes/src/main/resources-community/theme/base/email/messages/messages_no.properties (1)
5-5: Removed HTML from plain-text template – correct fixStripping the
<b>tags from the non-HTML body prevents markup bleed in plain emails while the HTML variant (line 6) keeps formatting. ✔️themes/src/main/resources-community/theme/base/login/messages/messages_de.properties (1)
420-424: Unified<br />self-closing styleThe spaced form is now consistent with the sanitizer policy applied project-wide. No action required.
themes/src/main/resources-community/theme/base/login/messages/messages_es.properties (1)
442-446: HTML break tag normalisedSame harmless formatting tweak as in other locales; translation content unchanged. ✅
themes/src/main/resources-community/theme/base/login/messages/messages_sv.properties (1)
68-68: Step-1 wording simplified – verify downstream list renderingThe line now reads “Installera en av följande applikationer …”.
Assuming the UI still renders the app list via separate keys (totpAppFreeOTPNameetc.), this is fine. If that list is not shown, users lose explicit examples—double-check the template.themes/src/main/resources-community/theme/base/login/messages/messages_zh_CN.properties (1)
466-470: Consistent<br />spacing looks goodThe update standardises self-closing break tags to the XHTML-style
<br />, matching changes across other locale files and avoiding false positives in the new HTML-sanitiser. No further action required.js/apps/admin-ui/maven-resources-community/theme/keycloak.v2/admin/messages/messages_zh_CN.properties (1)
1255-1255: Placeholder formatting fixed – LGTMThe extra space inside
${CLAIM.<NAME>}was removed, preventing runtime placeholder-resolution issues.themes/src/main/resources/theme/base/login/messages/messages_en.properties (2)
487-492: Consistent XHTML-style<br />looks goodSwitching to the spaced self-closing tag aligns with the HTML-sanitizer rules introduced in this PR and avoids noisy lint warnings. Nice housekeeping.
530-531: Double-check theChoiceFormatedge case
{2,choice,0#values|1#value|1<values}will fall through to the last alternative for every value > 1. That is fine, but make sure this key really receives the “max” argument ({2}) at runtime; some callers pass the current count instead. A mismatch will produce grammatically wrong output (“at most 5 value”).themes/src/main/resources-community/theme/base/login/messages/messages_fr.properties (1)
377-381: HTML tag tweak acknowledged
<br/> → <br />is purely cosmetic—no functional impact. Change is correct.themes/src/main/resources-community/theme/base/login/messages/messages_cs.properties (1)
466-470: Good consistency updateSame XHTML self-closing adjustment as other locales – all good.
themes/src/main/resources-community/theme/base/account/messages/messages_sk.properties (1)
101-103: Generic TOTP instructions OKHard-coding app names was removed; the new wording is clearer and will age better.
themes/src/main/resources-community/theme/base/login/messages/messages_hr.properties (1)
479-483: HTML line-break tag now standards-compliant – looks good.The change from
<br/>to<br />matches the sanitizer’s whitelist and the rest of the codebase. No further action needed.themes/src/main/resources-community/theme/base/login/messages/messages_no.properties (1)
71-71: Generic Norwegian TOTP step 1 text is correct.The revised sentence is clear, in correct Bokmål, and free of app-specific links. No issues spotted.
themes/src/main/resources-community/theme/base/account/messages/messages_fi.properties (1)
141-141: Finnish wording updated correctly.The new neutral instruction reads naturally and aligns with other locales. No problems identified.
themes/src/main/resources-community/theme/base/login/messages/messages_uk.properties (1)
459-463: HTML line-break change looks goodThe
<br/>→<br />replacement is correct and safe; no other issues spotted.themes/src/main/resources-community/theme/base/account/messages/messages_sv.properties (1)
96-96: Generic wording LGTMSwitching to a vendor-neutral instruction removes outdated app references and improves longevity. ✔
themes/src/main/resources-community/theme/base/login/messages/messages_ka.properties (1)
172-174: Consistent<br />usage appliedChange aligns with sanitizer rules. No further action required.
Also applies to: 451-452
themes/src/main/resources-community/theme/base/login/messages/messages_fi.properties (1)
400-404: LGTM – only tag spacing changedThe
<br/>→<br />update aligns with the new sanitiser rules; nothing else to flag.themes/src/main/resources-community/theme/base/login/messages/messages_zh_TW.properties (1)
473-477: LGTM – XHTML-style<br />adoptedChange is purely cosmetic and required by the verifier. No further issues.
themes/src/main/resources-community/theme/base/login/messages/messages_sk.properties (3)
30-30: Fixed stray closing tag – good catchThe
<strong>{0}</strong>wrapper now closes correctly.
123-123: Generic app list phrasing – no functional impactReplacing concrete app names with a generic sentence is fine; just confirm with localisation team that context is still clear for users.
466-470:<br />tag fix – looks goodThe change satisfies the new sanitiser; no further action.
misc/theme-verifier/src/test/java/org/keycloak/themeverifier/VerifyMessagePropertiesTest.java (4)
31-34: LGTM!The test correctly verifies duplicate key detection functionality using appropriate Hamcrest matchers.
36-40: LGTM!The test appropriately verifies the detection of illegal HTML tags in message properties.
42-46: LGTM!Good test coverage for verifying HTML restrictions in localized properties files.
48-52: LGTM!Important test for verifying that anchor tags in translations match the English source, preventing potential security issues.
| PolicyFactory POLICY_SOME_HTML = new org.owasp.html.HtmlPolicyBuilder() | ||
| .allowElements( | ||
| "br", "p", "strong", "b" | ||
| ).toFactory(); | ||
|
|
||
| PolicyFactory POLICY_NO_HTML = new org.owasp.html.HtmlPolicyBuilder().toFactory(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make PolicyFactory instances static final constants
These policy factories are immutable and should be declared as static final constants for better performance and clarity.
- PolicyFactory POLICY_SOME_HTML = new org.owasp.html.HtmlPolicyBuilder()
+ private static final PolicyFactory POLICY_SOME_HTML = new org.owasp.html.HtmlPolicyBuilder()
.allowElements(
- "br", "p", "strong", "b"
+ "br", "p", "strong", "b", "em", "i"
).toFactory();
- PolicyFactory POLICY_NO_HTML = new org.owasp.html.HtmlPolicyBuilder().toFactory();
+ private static final PolicyFactory POLICY_NO_HTML = new org.owasp.html.HtmlPolicyBuilder().toFactory();Also consider adding "em" and "i" tags for italic text formatting, which are commonly used alongside bold formatting.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| PolicyFactory POLICY_SOME_HTML = new org.owasp.html.HtmlPolicyBuilder() | |
| .allowElements( | |
| "br", "p", "strong", "b" | |
| ).toFactory(); | |
| PolicyFactory POLICY_NO_HTML = new org.owasp.html.HtmlPolicyBuilder().toFactory(); | |
| private static final PolicyFactory POLICY_SOME_HTML = new org.owasp.html.HtmlPolicyBuilder() | |
| .allowElements( | |
| "br", "p", "strong", "b", "em", "i" | |
| ).toFactory(); | |
| private static final PolicyFactory POLICY_NO_HTML = new org.owasp.html.HtmlPolicyBuilder().toFactory(); |
🤖 Prompt for AI Agents
In
misc/theme-verifier/src/main/java/org/keycloak/themeverifier/VerifyMessageProperties.java
around lines 58 to 64, the PolicyFactory instances are currently not declared as
static final constants. Change their declarations to static final to improve
performance and clarity since they are immutable. Additionally, update the
allowed elements in POLICY_SOME_HTML to include "em" and "i" tags to support
italic text formatting alongside the existing bold tags.
| String englishFile = file.getAbsolutePath().replaceAll("resources-community", "resources") | ||
| .replaceAll("_[a-zA-Z-_]*\\.properties", "_en.properties"); | ||
| try (FileInputStream fis = new FileInputStream(englishFile)) { | ||
| bundleEnglish = new PropertyResourceBundle(fis); | ||
| } catch (IOException e) { | ||
| throw new RuntimeException("unable to read file " + englishFile, e); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve English file path resolution and error handling
The current approach using string replacement is fragile and may fail with unexpected directory structures. Additionally, the error handling should be consistent with the rest of the class.
PropertyResourceBundle bundleEnglish;
- String englishFile = file.getAbsolutePath().replaceAll("resources-community", "resources")
- .replaceAll("_[a-zA-Z-_]*\\.properties", "_en.properties");
+ // More robust approach to find English file
+ File parentDir = file.getParentFile();
+ String fileName = file.getName();
+ String englishFileName = fileName.replaceAll("_[a-zA-Z-]+\\.properties$", "_en.properties");
+ File englishFile = new File(parentDir, englishFileName);
+
+ // If in resources-community, also check resources directory
+ if (!englishFile.exists() && parentDir.getName().equals("resources-community")) {
+ File resourcesDir = new File(parentDir.getParentFile(), "resources");
+ englishFile = new File(resourcesDir, englishFileName);
+ }
+
try (FileInputStream fis = new FileInputStream(englishFile)) {
bundleEnglish = new PropertyResourceBundle(fis);
} catch (IOException e) {
- throw new RuntimeException("unable to read file " + englishFile, e);
+ messages.add("Unable to read English source file for " + file.getName() + ": " + e.getMessage());
+ return; // Skip HTML validation if English file is not available
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String englishFile = file.getAbsolutePath().replaceAll("resources-community", "resources") | |
| .replaceAll("_[a-zA-Z-_]*\\.properties", "_en.properties"); | |
| try (FileInputStream fis = new FileInputStream(englishFile)) { | |
| bundleEnglish = new PropertyResourceBundle(fis); | |
| } catch (IOException e) { | |
| throw new RuntimeException("unable to read file " + englishFile, e); | |
| } | |
| PropertyResourceBundle bundleEnglish; | |
| // More robust approach to find English file | |
| File parentDir = file.getParentFile(); | |
| String fileName = file.getName(); | |
| String englishFileName = fileName.replaceAll("_[a-zA-Z-]+\\.properties$", "_en.properties"); | |
| File englishFile = new File(parentDir, englishFileName); | |
| // If in resources-community, also check resources directory | |
| if (!englishFile.exists() && "resources-community".equals(parentDir.getName())) { | |
| File resourcesDir = new File(parentDir.getParentFile(), "resources"); | |
| englishFile = new File(resourcesDir, englishFileName); | |
| } | |
| try (FileInputStream fis = new FileInputStream(englishFile)) { | |
| bundleEnglish = new PropertyResourceBundle(fis); | |
| } catch (IOException e) { | |
| messages.add("Unable to read English source file for " + file.getName() + ": " + e.getMessage()); | |
| return; // Skip HTML validation if English file is not available | |
| } |
🤖 Prompt for AI Agents
In
misc/theme-verifier/src/main/java/org/keycloak/themeverifier/VerifyMessageProperties.java
around lines 74 to 80, the English file path is constructed using fragile string
replacements that may break with different directory structures. Refactor this
to build the path more robustly, for example by manipulating File or Path
objects to navigate directories and replace locale suffixes safely. Also, update
the error handling to match the class's standard approach, ensuring consistent
exception types and messages.
| String englishValue = getEnglishValue(key, bundleEnglish); | ||
| englishValue = normalizeValue(key, englishValue); | ||
|
|
||
| value = santizeAnchors(key, value, englishValue); |
There was a problem hiding this comment.
Fix typo in method name
The method name "santizeAnchors" has a typo - it should be "sanitizeAnchors".
- value = santizeAnchors(key, value, englishValue);
+ value = sanitizeAnchors(key, value, englishValue);- private String santizeAnchors(String key, String value, String englishValue) {
+ private String sanitizeAnchors(String key, String value, String englishValue) {Also applies to: 150-150
🤖 Prompt for AI Agents
In
misc/theme-verifier/src/main/java/org/keycloak/themeverifier/VerifyMessageProperties.java
at lines 88 and 150, the method name "santizeAnchors" is misspelled. Rename all
occurrences of "santizeAnchors" to "sanitizeAnchors" to correct the typo.
| Pattern HTML_TAGS = Pattern.compile("<[a-z]+[^>]*>"); | ||
|
|
||
| private boolean containsHtml(String englishValue) { | ||
| return HTML_TAGS.matcher(englishValue).find(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make regex pattern static final
The HTML_TAGS pattern should be a static final constant for better performance.
- Pattern HTML_TAGS = Pattern.compile("<[a-z]+[^>]*>");
+ private static final Pattern HTML_TAGS = Pattern.compile("<[a-z]+[^>]*>");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Pattern HTML_TAGS = Pattern.compile("<[a-z]+[^>]*>"); | |
| private boolean containsHtml(String englishValue) { | |
| return HTML_TAGS.matcher(englishValue).find(); | |
| } | |
| private static final Pattern HTML_TAGS = Pattern.compile("<[a-z]+[^>]*>"); | |
| private boolean containsHtml(String englishValue) { | |
| return HTML_TAGS.matcher(englishValue).find(); | |
| } |
🤖 Prompt for AI Agents
In
misc/theme-verifier/src/main/java/org/keycloak/themeverifier/VerifyMessageProperties.java
around lines 139 to 143, the HTML_TAGS regex pattern is currently an instance
variable. Change its declaration to be static final to make it a constant,
improving performance by compiling the pattern only once for all instances.
| configureAuthenticators=Sukonfigūruotas autentifikatorius | ||
| mobile=Mobilus | ||
| totpStep1=Įdiekite <a href="https://freeotp.github.io/" target="_blank">FreeOTP</a> arba Google Authenticator savo įrenginyje. Programėlės prieinamos <a href="https://play.google.com">Google Play</a> ir Apple App Store. | ||
| totpStep1=Installa una delle seguenti applicazioni sul tuo cellulare: |
There was a problem hiding this comment.
Italian text accidentally inserted into Lithuanian resource file.
totpStep1 is now Italian (“Installa una …”) instead of Lithuanian, breaking localisation quality.
-totpStep1=Installa una delle seguenti applicazioni sul tuo cellulare:
+totpStep1=Įdiekite vieną iš šių programėlių į savo telefoną:🤖 Prompt for AI Agents
In
themes/src/main/resources-community/theme/base/account/messages/messages_lt.properties
at line 101, the value for the key totpStep1 is mistakenly in Italian instead of
Lithuanian. Replace the Italian text with the correct Lithuanian translation to
maintain proper localization quality.
| emailVerificationSubject=El. pašto patvirtinimas | ||
| emailVerificationBody=Paskyra {2} sukurta naudojant šį el. pašto adresą. Jei tai buvote Jūs, tuomet paspauskite žemiau esančią nuorodą\n\n{0}\n\nŠi nuoroda galioja {1} min.\n\nJei paskyros nekūrėte, tuomet ignuoruokite šį laišką. | ||
| emailVerificationBodyHtml=<p>Paskyra {2} sukurta naudojant šį el. pašto adresą. Jei tao buvote Jūs, tuomet paspauskite žemiau esančią nuorodą</p><p><a href=LT"{0}">{0}</a></p><p>Ši nuoroda galioja {1} min.</p><p>nJei paskyros nekūrėte, tuomet ignuoruokite šį laišką.</p> | ||
| emailVerificationBodyHtml=<p>Paskyra {2} sukurta naudojant šį el. pašto adresą. Jei tao buvote Jūs, tuomet paspauskite žemiau esančią nuorodą</p><p><a href="{0}">{0}</a></p><p>Ši nuoroda galioja {1} min.</p><p>nJei paskyros nekūrėte, tuomet ignuoruokite šį laišką.</p> |
There was a problem hiding this comment.
Fix several typos in emailVerificationBodyHtml
tao → tai, stray n at sentence start, and ignuoruokite → ignoruokite.
-emailVerificationBodyHtml=<p>Paskyra {2} sukurta naudojant šį el. pašto adresą. Jei tao buvote Jūs, tuomet paspauskite žemiau esančią nuorodą</p><p><a href="{0}">{0}</a></p><p>Ši nuoroda galioja {1} min.</p><p>nJei paskyros nekūrėte, tuomet ignuoruokite šį laišką.</p>
+emailVerificationBodyHtml=<p>Paskyra {2} sukurta naudojant šį el. pašto adresą. Jei tai buvote Jūs, tuomet paspauskite žemiau esančią nuorodą</p><p><a href="{0}">{0}</a></p><p>Ši nuoroda galioja {1} min.</p><p>Jei paskyros nekūrėte, tuomet ignoruokite šį laišką.</p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| emailVerificationBodyHtml=<p>Paskyra {2} sukurta naudojant šį el. pašto adresą. Jei tao buvote Jūs, tuomet paspauskite žemiau esančią nuorodą</p><p><a href="{0}">{0}</a></p><p>Ši nuoroda galioja {1} min.</p><p>nJei paskyros nekūrėte, tuomet ignuoruokite šį laišką.</p> | |
| emailVerificationBodyHtml=<p>Paskyra {2} sukurta naudojant šį el. pašto adresą. Jei tai buvote Jūs, tuomet paspauskite žemiau esančią nuorodą</p><p><a href="{0}">{0}</a></p><p>Ši nuoroda galioja {1} min.</p><p>Jei paskyros nekūrėte, tuomet ignoruokite šį laišką.</p> |
🤖 Prompt for AI Agents
In
themes/src/main/resources-community/theme/base/email/messages/messages_lt.properties
at line 3, fix the typos in the emailVerificationBodyHtml string by replacing
"tao" with "tai", removing the stray "n" at the start of the last paragraph, and
correcting "ignuoruokite" to "ignoruokite".
| identityProviderLinkSubject=Sąsaja {0} | ||
| identityProviderLinkBody=Kažas pageidauja susieti Jūsų "{1}" paskyrą su "{0}" {2} naudotojo paskyrą. Jei tai buvote Jūs, tuomet paspauskite žemiau esančią nuorodą norėdami susieti paskyras\n\n{3}\n\nŠi nuoroda galioja {4} min.\n\nJei paskyrų susieti nenorite, tuomet ignoruokite šį laišką. Jei paskyras susiesite, tuomet prie {1} galėsiste prisijungti per {0}. | ||
| identityProviderLinkBodyHtml=<p>žas pageidauja susieti Jūsų <b>{1}</b> paskyrą su <b>{0}</b> {2} naudotojo paskyrą. Jei tai buvote Jūs, tuomet paspauskite žemiau esančią nuorodą norėdami susieti paskyras</p><p><a href=LT"{3}">{3}</a></p><p>Ši nuoroda galioja {4} min.</p><p>Jei paskyrų susieti nenorite, tuomet ignoruokite šį laišką. Jei paskyras susiesite, tuomet prie {1} galėsiste prisijungti per {0}.</p> | ||
| identityProviderLinkBodyHtml=<p>žas pageidauja susieti Jūsų <b>{1}</b> paskyrą su <b>{0}</b> {2} naudotojo paskyrą. Jei tai buvote Jūs, tuomet paspauskite žemiau esančią nuorodą norėdami susieti paskyras</p><p><a href="{3}">{3}</a></p><p>Ši nuoroda galioja {4} min.</p><p>Jei paskyrų susieti nenorite, tuomet ignoruokite šį laišką. Jei paskyras susiesite, tuomet prie {1} galėsiste prisijungti per {0}.</p> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Restore lost characters and typo in identityProviderLinkBodyHtml
The string starts with žas instead of Kažkas and later contains galėsiste (should be galėsite).
-identityProviderLinkBodyHtml=<p>žas pageidauja susieti Jūsų <b>{1}</b> paskyrą su <b>{0}</b> {2} naudotojo paskyrą. ...
+identityProviderLinkBodyHtml=<p>Kažkas pageidauja susieti Jūsų <b>{1}</b> paskyrą su <b>{0}</b> {2} naudotojo paskyra. ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| identityProviderLinkBodyHtml=<p>žas pageidauja susieti Jūsų <b>{1}</b> paskyrą su <b>{0}</b> {2} naudotojo paskyrą. Jei tai buvote Jūs, tuomet paspauskite žemiau esančią nuorodą norėdami susieti paskyras</p><p><a href="{3}">{3}</a></p><p>Ši nuoroda galioja {4} min.</p><p>Jei paskyrų susieti nenorite, tuomet ignoruokite šį laišką. Jei paskyras susiesite, tuomet prie {1} galėsiste prisijungti per {0}.</p> | |
| identityProviderLinkBodyHtml=<p>Kažkas pageidauja susieti Jūsų <b>{1}</b> paskyrą su <b>{0}</b> {2} naudotojo paskyrą. Jei tai buvote Jūs, tuomet paspauskite žemiau esančią nuorodą norėdami susieti paskyras</p><p><a href="{3}">{3}</a></p><p>Ši nuoroda galioja {4} min.</p><p>Jei paskyrų susieti nenorite, tuomet ignoruokite šį laišką. Jei paskyras susiesite, tuomet prie {1} galėsiste prisijungti per {0}.</p> |
🤖 Prompt for AI Agents
In
themes/src/main/resources-community/theme/base/email/messages/messages_lt.properties
at line 6, the string for identityProviderLinkBodyHtml has a typo and missing
characters: it starts with "žas" instead of "Kažkas" and contains "galėsiste"
instead of "galėsite". Correct these typos by replacing "žas" with "Kažkas" at
the start and "galėsiste" with "galėsite" later in the string.
| executeActionsSubject=Atnaujinkite savo paskyrą | ||
| executeActionsBody=Sistemos administratorius pageidauja, kad Jūs atnaujintumėte savo {2} paskyrą. Paspauskite žemiau esančią nuorodą paskyros duomenų atnaujinimui.\n\n{0}\n\nŠi nuoroda galioja {1} min.\n\nJei Jūs neasate tikri, kad tai administratoriaus pageidavimas, tuomet ignoruokite šį laišką ir niekas nebus pakeista. | ||
| executeActionsBodyHtml=<p>Sistemos administratorius pageidauja, kad Jūs atnaujintumėte savo {2} paskyrą. Paspauskite žemiau esančią nuorodą paskyros duomenų atnaujinimui.</p><p><a href=LT"{0}">{0}</a></p><p>Ši nuoroda galioja {1} min.</p><p>Jei Jūs neasate tikri, kad tai administratoriaus pageidavimas, tuomet ignoruokite šį laišką ir niekas nebus pakeista.</p> | ||
| executeActionsBodyHtml=<p>Sistemos administratorius pageidauja, kad Jūs atnaujintumėte savo {2} paskyrą. Paspauskite žemiau esančią nuorodą paskyros duomenų atnaujinimui.</p><p><a href="{0}">{0}</a></p><p>Ši nuoroda galioja {1} min.</p><p>Jei Jūs neasate tikri, kad tai administratoriaus pageidavimas, tuomet ignoruokite šį laišką ir niekas nebus pakeista.</p> |
There was a problem hiding this comment.
Correct misspelling in executeActionsBodyHtml
neasate → nesate.
-... Jei Jūs neasate tikri, kad tai administratoriaus pageidavimas, tuomet ignoruokite šį laišką ir niekas nebus pakeista.</p>
+... Jei Jūs nesate tikri, kad tai administratoriaus pageidavimas, tuomet ignoruokite šį laišką ir niekas nebus pakeista.</p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| executeActionsBodyHtml=<p>Sistemos administratorius pageidauja, kad Jūs atnaujintumėte savo {2} paskyrą. Paspauskite žemiau esančią nuorodą paskyros duomenų atnaujinimui.</p><p><a href="{0}">{0}</a></p><p>Ši nuoroda galioja {1} min.</p><p>Jei Jūs neasate tikri, kad tai administratoriaus pageidavimas, tuomet ignoruokite šį laišką ir niekas nebus pakeista.</p> | |
| executeActionsBodyHtml=<p>Sistemos administratorius pageidauja, kad Jūs atnaujintumėte savo {2} paskyrą. Paspauskite žemiau esančią nuorodą paskyros duomenų atnaujinimui.</p><p><a href="{0}">{0}</a></p><p>Ši nuoroda galioja {1} min.</p><p>Jei Jūs nesate tikri, kad tai administratoriaus pageidavimas, tuomet ignoruokite šį laišką ir niekas nebus pakeista.</p> |
🤖 Prompt for AI Agents
In
themes/src/main/resources-community/theme/base/email/messages/messages_lt.properties
at line 12, correct the misspelling in the executeActionsBodyHtml value by
replacing "neasate" with "nesate" to fix the typo.
| loginTotpStep1=Installa una delle seguenti applicazioni sul tuo cellulare: | ||
| loginTotpStep2=Atidarykite programėlę ir nuskenuokite barkodą arba įveskite kodą. | ||
| loginTotpStep3=Įveskite programėlėje sugeneruotą vieną kartą galiojantį kodą ir paspauskite Saugoti norėdami prisijungti. |
There was a problem hiding this comment.
Italian text accidentally injected into Lithuanian bundle
loginTotpStep1 is now Italian (“Installa una delle seguenti applicazioni …”), breaking the Lithuanian locale and likely confusing users.
-loginTotpStep1=Installa una delle seguenti applicazioni sul tuo cellulare:
+loginTotpStep1=Įdiekite vieną iš šių programėlių savo mobiliajame telefone:Please restore a correct Lithuanian translation and re-verify other keys for similar regressions.
🤖 Prompt for AI Agents
In
themes/src/main/resources-community/theme/base/login/messages/messages_lt.properties
around lines 71 to 73, the value for loginTotpStep1 is mistakenly in Italian
instead of Lithuanian, which breaks the Lithuanian locale. Replace the Italian
text with the correct Lithuanian translation for loginTotpStep1 and review the
other keys in this section to ensure all are properly in Lithuanian without
similar language mix-ups.
|
|
||
| readOnlyUsernameMessage=Nemôžete aktualizovať svoje používateľské meno, pretože je iba na čítanie. | ||
| error-invalid-multivalued-size=Atribút {0} musí mať najmenej {1} {1,choice,0#hodnôt|1#hodnotu|1<hodnoty|4<hodnôt} a najviac {2} {1,choice,0#hodnôt|1#hodnotu|1<hodnoty|4<hodnôt}. No newline at end of file | ||
| error-invalid-multivalued-size=Atribút {0} musí mať najmenej {1} {1,choice,0#hodnôt|1#hodnotu|1<hodnoty|4<hodnôt} a najviac {2} {2,choice,0#hodnôt|1#hodnotu|1<hodnoty|4<hodnôt}. No newline at end of file |
There was a problem hiding this comment.
ChoiceFormat ordering prevents 5-plus case from ever matching
With ...|1<hodnoty|4<hodnôt the 1< rule swallows every number ≥ 2, so the 4< branch is dead code and plurals for 5+ will still use “hodnoty”.
Recommend:
-error-invalid-multivalued-size=Atribút {0} musí mať najmenej {1} {1,choice,0#hodnôt|1#hodnotu|1<hodnoty|4<hodnôt} a najviac {2} {2,choice,0#hodnôt|1#hodnotu|1<hodnoty|4<hodnôt}.
+error-invalid-multivalued-size=Atribút {0} musí mať najmenej {1} {1,choice,0#hodnôt|1#hodnotu|1<hodnoty|4<hodnôt} a najviac {2} {2,choice,0#hodnôt|1#hodnotu|1<hodnoty|4<hodnôt}.Move the 5-plus rule (4<hodnôt) before the 2-4 rule or use explicit ranges (2#hodnoty|5<hodnôt).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
themes/src/main/resources-community/theme/base/login/messages/messages_sk.properties
at line 501, the ChoiceFormat pattern for plural forms is incorrectly ordered,
causing the 5-plus case to never match because the '1<' rule captures all
numbers 2 and above. To fix this, reorder the plural rules by placing the '4<'
(5-plus) condition before the '1<' (2-4) condition or use explicit numeric
ranges like '2#hodnoty|5<hodnôt' to correctly handle plurals for numbers 5 and
above.
2 participants
Test 7
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores