Load supported languages dynamically from LanguageOptions.properties #9543
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Replace hardcoded language mappings with dynamic loading from properties file - Add caching mechanism using request attributes to improve performance - Parse LanguageOptions.properties to extract language codes and country mappings - Maintain backward compatibility with existing language support
Contributor
WalkthroughReplaces hard-coded language mappings in a JSP with dynamic loading from LanguageOptions.properties, adds request-scoped caching of parsed supported languages and supported countries, and introduces stream/reader imports for file reading. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
@identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp:
- Line 129: Replace the FileReader usage that relies on the platform default
charset: in the try-with-resources where BufferedReader is created from new
FileReader(filePath), open the file with an InputStreamReader wrapping a
FileInputStream and specify StandardCharsets.UTF_8 (keeping the same variable
names: bufferedReader and filePath) so the file is read with explicit UTF-8
encoding; ensure StandardCharsets is available/imported in the context.
- Around line 153-155: The catch block currently re-throws exceptions which
causes page failure; instead catch the Exception, log or record the error,
populate a safe default language mapping object, and continue so the page can
render; move the existing setAttribute(...) calls for the language mappings
outside the try/catch so they always execute (referencing the try/catch around
the properties loading and the setAttribute calls in localize.jsp) and ensure
the exception handler assigns the defaults before leaving the catch.
🧹 Nitpick comments (1)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp (1)
116-118: Misleading comment and ineffective caching strategy.The comment says "application scope" but the code uses
request.getAttribute(), which is request scope. Request-level caching provides minimal benefit since this JSP is typically included once per request. Consider usingapplication.getAttribute()(ServletContext) for true cross-request caching, which would avoid re-parsing the file on every request.♻️ Suggested fix for application-scope caching
- // Dynamically load supported languages from languageOptions.properties - // Use application scope to cache the parsed language options - Map<String, String> cachedSupportedLanguages = (Map<String, String>) request.getAttribute(SUPPORTED_LANGUAGES_ATTR); - List<String> cachedLanguageSupportedCountries = (List<String>) request.getAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR); + // Dynamically load supported languages from LanguageOptions.properties + // Use application scope to cache the parsed language options across requests + Map<String, String> cachedSupportedLanguages = (Map<String, String>) application.getAttribute(SUPPORTED_LANGUAGES_ATTR); + List<String> cachedLanguageSupportedCountries = (List<String>) application.getAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR);Also update the setAttribute calls at lines 151-152:
- request.setAttribute(SUPPORTED_LANGUAGES_ATTR, supportedLanguages); - request.setAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR, languageSupportedCountries); + application.setAttribute(SUPPORTED_LANGUAGES_ATTR, supportedLanguages); + application.setAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR, languageSupportedCountries);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
🔇 Additional comments (2)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp (2)
23-24: LGTM!The imports for
FileReaderandBufferedReaderare appropriate for reading the properties file.
104-105: LGTM!The attribute name constants are well-defined for the caching mechanism.
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
Show resolved
Hide resolved
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
Outdated
Show resolved
Hide resolved
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
Show resolved
Hide resolved
JayaShakthi97
previously approved these changes
Jan 9, 2026
Contributor
|
Let's add changeset as well. |
…includes/localize.jsp Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
@identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp:
- Around line 116-124: The cache is incorrectly stored in request scope causing
repeated file I/O; replace request.getAttribute/request.setAttribute usage with
application.getAttribute/application.setAttribute (use the
SUPPORTED_LANGUAGES_ATTR and LANGUAGE_SUPPORTED_COUNTRIES_ATTR keys) so
supportedLanguages and languageSupportedCountries are kept in application scope,
and wrap the populate-and-set logic in a synchronization block (e.g.,
synchronized on the ServletContext or a dedicated lock attribute) to avoid race
conditions when multiple requests attempt to initialize the cache.
- Around line 129-164: The catch block currently re-throws all exceptions (catch
in localize.jsp) which leaks internals and breaks the request; replace that
behavior by logging the exception (use the existing logger or add one), populate
supportedLanguages and languageSupportedCountries with a safe default (at
minimum put "en_US" into supportedLanguages and languageSupportedCountries),
call request.setAttribute(SUPPORTED_LANGUAGES_ATTR, supportedLanguages) and
request.setAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR,
languageSupportedCountries) so the request proceeds, and do not re-throw the
exception; ensure the change targets the catch block that currently contains
"throw e" and touches the supportedLanguages/languageSupportedCountries
variables and the request.setAttribute calls.
- Line 126: The code uses application.getRealPath("/") to build filePath which
can be null for WAR deployments; replace this with
application.getResourceAsStream("/WEB-INF/classes/LanguageOptions.properties")
and read the properties via an InputStream (e.g., wrap in
InputStreamReader/BufferedReader) instead of concatenating paths. Update the
code around the variable filePath and any parsing logic that reads
LanguageOptions.properties to use the InputStream returned by
application.getResourceAsStream, and ensure the stream is closed in a
try-with-resources block.
🧹 Nitpick comments (1)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp (1)
133-157: Consider adding validation for parsed language/country codes.The parsing logic extracts language codes and country codes from the properties file but doesn't validate that they are valid ISO codes. While this may not cause immediate issues, invalid codes could lead to unexpected behavior in locale resolution.
💡 Optional enhancement: Validate parsed codes
String country = values[0]; + + // Optional: Validate language and country codes + if (languageCode.length() < 2 || languageCode.length() > 3 || + country.length() != 2) { + continue; // Skip invalid codes + } + // displayName is available in values[1] if needed in the future // Add the values to the list. supportedLanguages.put(languageCode, country);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
🔇 Additional comments (2)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp (2)
23-24: LGTM! Required imports for file I/O.The imports are necessary for reading the LanguageOptions.properties file.
104-105: LGTM! Clear constant definitions.The cache attribute name constants are well-defined and follow good naming conventions.
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
Show resolved
Hide resolved
pavinduLakshan
commented
Jan 9, 2026
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp:
- Around line 164-171: Replace the System.err.println in the catch block with
the project's logger (e.g., an SLF4J/Log4j logger) and log the failure to load
LanguageOptions.properties including the exception and stack trace; update the
catch(Exception e) block (the block that manipulates supportedLanguages and
languageSupportedCountries) to call logger.error("Failed to load
LanguageOptions.properties", e) or equivalent, creating or reusing a logger
instance for the JSP if one exists.
- Around line 118-125: The code intends to cache parsed language options in
application scope but currently uses
request.getAttribute(SUPPORTED_LANGUAGES_ATTR) and
request.getAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR), which causes
per-request re-parsing; change retrieval to use servletContext.getAttribute(...)
(or request.getServletContext().getAttribute(...)) for cachedSupportedLanguages
and cachedLanguageSupportedCountries and likewise update the storage logic that
sets these values (currently using request.setAttribute(...)) to
servletContext.setAttribute(...) so the Map<String,String>
cachedSupportedLanguages and List<String> cachedLanguageSupportedCountries are
stored application-wide; if needed, add minimal synchronization or initialize
them in a ServletContextListener to ensure thread-safe one-time loading.
🧹 Nitpick comments (1)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp (1)
137-152: Consider trimming parsed values.The parsing logic doesn't trim whitespace from the parsed language code and country values. Properties files often have inconsistent spacing, which could lead to incorrect lookups or duplicate entries with leading/trailing whitespace.
♻️ Proposed enhancement: Trim parsed values
// Split the key further using '.' as the delimiter String[] parts = keyValue[0].split("\\."); if (parts.length == 0) { continue; } - String languageCode = parts[parts.length - 1]; + String languageCode = parts[parts.length - 1].trim(); // Split the value further using ',' as the delimiter - String[] values = keyValue[1].split(","); + String[] values = keyValue[1].trim().split(","); if (values.length < 2) { continue; // Skip lines without proper country,displayName format } - String country = values[0]; + String country = values[0].trim();
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
🔇 Additional comments (2)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp (2)
23-25: LGTM!The new imports are appropriate for reading and parsing the properties file with UTF-8 encoding support.
105-106: LGTM!The attribute name constants improve code maintainability and reduce the risk of typos.
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp:
- Around line 127-169: The catch block that currently swallows exceptions when
loading LanguageOptions.properties should log the failure; update the
catch(Exception e) in localize.jsp to call application.log with a clear message
and the exception (e) before falling back to the default entries
(supportedLanguages.put("en","US") and languageSupportedCountries.add("US")), so
any missing file or parse error is recorded for diagnostics while preserving the
existing fallback behavior and subsequent request.setAttribute calls.
- Around line 117-170: The code is not actually caching in application scope,
uses fragile file I/O and manual parsing, and swallows errors; change loading to
read /WEB-INF/classes/LanguageOptions.properties via
application.getResourceAsStream(...) and parse with
java.util.Properties.load(Reader) into trimmed keys/values, populate the
Map<String,String> supportedLanguages and List<String>
languageSupportedCountries deterministically (clear them first) and then store
them with application.setAttribute(SUPPORTED_LANGUAGES_ATTR, supportedLanguages)
and application.setAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR,
languageSupportedCountries) so it is cached application-wide (not per-request),
and on any load error or missing stream call application.log(...) with the
exception/message and reset supportedLanguages to a deterministic default (e.g.,
put("en","US") and languageSupportedCountries.clear(); add "US") before storing.
🧹 Nitpick comments (2)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp (2)
23-25: Keep imports minimal; preferInputStreamoverFileInputStreamif you switch to resource-stream loading.
These imports are fine as-is, but if you adoptapplication.getResourceAsStream(...)/ classloader loading (recommended below), you can dropFileInputStream.
105-106: Namespace + make thesefinalto avoid attribute collisions.
supportedLanguages/languageSupportedCountriesare generic keys and may collide with other JSPs/filters. Suggest a namespaced key andfinallocals.Proposed tweak
- String SUPPORTED_LANGUAGES_ATTR = "supportedLanguages"; - String LANGUAGE_SUPPORTED_COUNTRIES_ATTR = "languageSupportedCountries"; + final String SUPPORTED_LANGUAGES_ATTR = "org.wso2.authn.endpoint.supportedLanguages"; + final String LANGUAGE_SUPPORTED_COUNTRIES_ATTR = "org.wso2.authn.endpoint.languageSupportedCountries";
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
🔇 Additional comments (1)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp (1)
117-170: Verify the LanguageOptions.properties format matches the parsing assumptions.
This logic assumeskeySuffix == languageandvalue == country,displayName. If keys containen_US(noten) your later lookupsupportedLanguages.get(browserLocale.getLanguage())won’t match. Please confirm the file format + add a couple of representative examples in the PR description (or a small unit test for the parser if moved out of JSP).
Comment on lines
117
to
170
| // Dynamically load supported languages from languageOptions.properties | ||
| // Use application scope to cache the parsed language options | ||
| Map<String, String> cachedSupportedLanguages = (Map<String, String>) request.getAttribute(SUPPORTED_LANGUAGES_ATTR); | ||
| List<String> cachedLanguageSupportedCountries = (List<String>) request.getAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR); | ||
| if (cachedSupportedLanguages != null && cachedLanguageSupportedCountries != null) { | ||
| supportedLanguages = cachedSupportedLanguages; | ||
| languageSupportedCountries = cachedLanguageSupportedCountries; | ||
| } else { | ||
| // Specify the file path | ||
| String filePath = application.getRealPath("/") + "/WEB-INF/classes/LanguageOptions.properties"; | ||
| // Use a BufferedReader to read the file content | ||
| try (BufferedReader bufferedReader = new BufferedReader( | ||
| new InputStreamReader(new FileInputStream(filePath), StandardCharsets.UTF_8))) { | ||
| String line; | ||
| while ((line = bufferedReader.readLine()) != null) { | ||
| // Ignore comments and empty lines | ||
| if (!line.trim().startsWith("#") && !line.trim().isEmpty()) { | ||
| // Split the line into key and value using '=' as the delimiter | ||
| String[] keyValue = line.split("=", 2); | ||
| if (keyValue.length < 2) { | ||
| continue; // Skip malformed lines | ||
| } | ||
| // Split the key further using '.' as the delimiter | ||
| String[] parts = keyValue[0].split("\\."); | ||
| if (parts.length == 0) { | ||
| continue; | ||
| } | ||
| String languageCode = parts[parts.length - 1]; | ||
| // Split the value further using ',' as the delimiter | ||
| String[] values = keyValue[1].split(","); | ||
| if (values.length < 2) { | ||
| continue; // Skip lines without proper country,displayName format | ||
| } | ||
| String country = values[0]; | ||
| // displayName is available in values[1] if needed in the future | ||
| // Add the values to the list. | ||
| supportedLanguages.put(languageCode, country); | ||
| if (!languageSupportedCountries.contains(country)) { | ||
| languageSupportedCountries.add(country); | ||
| } | ||
| } | ||
| } | ||
| request.setAttribute(SUPPORTED_LANGUAGES_ATTR, supportedLanguages); | ||
| request.setAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR, languageSupportedCountries); | ||
| } catch (Exception e) { | ||
| if (supportedLanguages.isEmpty()) { | ||
| supportedLanguages.put("en", "US"); | ||
| languageSupportedCountries.add("US"); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Caching + loading approach is brittle and likely not delivering the intended perf win.
- The code comments say “application scope”, but it’s request scope (Line 119-120), so you still do file I/O on every request (no real caching benefit).
application.getRealPath("/")+FileInputStream(Line 127-132) can fail in packaged/non-exploded deployments; prefer resource-stream loading.- Manual parsing (Line 133-159) is not .properties-correct and doesn’t trim, so whitespace can break lookups.
- Catch-all
Exceptionwith no logging (Line 164-169) can silently reduce supported languages; mid-read exceptions can leave partially-populated structures.
Recommendation: cache in application scope + load via resource stream + parse via Properties.load(Reader) + application.log(...) on failure, and ensure fallback is deterministic (clear+default).
Concrete refactor (resource-stream + application-scope cache + Properties)
- // Dynamically load supported languages from languageOptions.properties
- // Use application scope to cache the parsed language options
- Map<String, String> cachedSupportedLanguages = (Map<String, String>) request.getAttribute(SUPPORTED_LANGUAGES_ATTR);
- List<String> cachedLanguageSupportedCountries = (List<String>) request.getAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR);
+ // Dynamically load supported languages from LanguageOptions.properties
+ // Cache parsed results in application scope (ServletContext) to avoid per-request file I/O.
+ Map<String, String> cachedSupportedLanguages = (Map<String, String>) application.getAttribute(SUPPORTED_LANGUAGES_ATTR);
+ List<String> cachedLanguageSupportedCountries = (List<String>) application.getAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR);
if (cachedSupportedLanguages != null && cachedLanguageSupportedCountries != null) {
supportedLanguages = cachedSupportedLanguages;
languageSupportedCountries = cachedLanguageSupportedCountries;
} else {
- // Specify the file path
- String filePath = application.getRealPath("/") + "/WEB-INF/classes/LanguageOptions.properties";
-
- // Use a BufferedReader to read the file content
- try (BufferedReader bufferedReader = new BufferedReader(
- new InputStreamReader(new FileInputStream(filePath), StandardCharsets.UTF_8))) {
- String line;
- while ((line = bufferedReader.readLine()) != null) {
- // Ignore comments and empty lines
- if (!line.trim().startsWith("#") && !line.trim().isEmpty()) {
- // Split the line into key and value using '=' as the delimiter
- String[] keyValue = line.split("=", 2);
- if (keyValue.length < 2) {
- continue; // Skip malformed lines
- }
- // Split the key further using '.' as the delimiter
- String[] parts = keyValue[0].split("\\.");
- if (parts.length == 0) {
- continue;
- }
- String languageCode = parts[parts.length - 1];
- // Split the value further using ',' as the delimiter
- String[] values = keyValue[1].split(",");
- if (values.length < 2) {
- continue; // Skip lines without proper country,displayName format
- }
- String country = values[0];
- // displayName is available in values[1] if needed in the future
- // Add the values to the list.
- supportedLanguages.put(languageCode, country);
- if (!languageSupportedCountries.contains(country)) {
- languageSupportedCountries.add(country);
- }
- }
- }
-
- request.setAttribute(SUPPORTED_LANGUAGES_ATTR, supportedLanguages);
- request.setAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR, languageSupportedCountries);
- } catch (Exception e) {
- if (supportedLanguages.isEmpty()) {
- supportedLanguages.put("en", "US");
- languageSupportedCountries.add("US");
- }
- }
+ synchronized (application) {
+ cachedSupportedLanguages = (Map<String, String>) application.getAttribute(SUPPORTED_LANGUAGES_ATTR);
+ cachedLanguageSupportedCountries = (List<String>) application.getAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR);
+ if (cachedSupportedLanguages != null && cachedLanguageSupportedCountries != null) {
+ supportedLanguages = cachedSupportedLanguages;
+ languageSupportedCountries = cachedLanguageSupportedCountries;
+ } else {
+ Properties props = new Properties();
+ try (java.io.InputStream in = application.getResourceAsStream("/WEB-INF/classes/LanguageOptions.properties")) {
+ if (in != null) {
+ props.load(new InputStreamReader(in, StandardCharsets.UTF_8));
+ }
+ }
+
+ for (String key : props.stringPropertyNames()) {
+ String[] parts = key.split("\\.");
+ if (parts.length == 0) {
+ continue;
+ }
+ String languageCode = parts[parts.length - 1].trim();
+ String raw = String.valueOf(props.getProperty(key, "")).trim();
+ String[] values = raw.split(",", 2);
+ if (values.length < 1) {
+ continue;
+ }
+ String country = values[0].trim();
+ if (languageCode.isEmpty() || country.isEmpty()) {
+ continue;
+ }
+ supportedLanguages.put(languageCode, country);
+ if (!languageSupportedCountries.contains(country)) {
+ languageSupportedCountries.add(country);
+ }
+ }
+
+ if (supportedLanguages.isEmpty()) {
+ supportedLanguages.clear();
+ languageSupportedCountries.clear();
+ supportedLanguages.put("en", "US");
+ languageSupportedCountries.add("US");
+ }
+
+ application.setAttribute(SUPPORTED_LANGUAGES_ATTR, java.util.Collections.unmodifiableMap(new HashMap<>(supportedLanguages)));
+ application.setAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR, java.util.Collections.unmodifiableList(new ArrayList<>(languageSupportedCountries)));
+ }
+ }
}And if loading fails / returns null InputStream, consider logging once:
<%
// e.g., after attempting to read:
// application.log("LanguageOptions.properties not found at /WEB-INF/classes/LanguageOptions.properties");
%>📝 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.
Suggested change
| // Dynamically load supported languages from languageOptions.properties | |
| // Use application scope to cache the parsed language options | |
| Map<String, String> cachedSupportedLanguages = (Map<String, String>) request.getAttribute(SUPPORTED_LANGUAGES_ATTR); | |
| List<String> cachedLanguageSupportedCountries = (List<String>) request.getAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR); | |
| if (cachedSupportedLanguages != null && cachedLanguageSupportedCountries != null) { | |
| supportedLanguages = cachedSupportedLanguages; | |
| languageSupportedCountries = cachedLanguageSupportedCountries; | |
| } else { | |
| // Specify the file path | |
| String filePath = application.getRealPath("/") + "/WEB-INF/classes/LanguageOptions.properties"; | |
| // Use a BufferedReader to read the file content | |
| try (BufferedReader bufferedReader = new BufferedReader( | |
| new InputStreamReader(new FileInputStream(filePath), StandardCharsets.UTF_8))) { | |
| String line; | |
| while ((line = bufferedReader.readLine()) != null) { | |
| // Ignore comments and empty lines | |
| if (!line.trim().startsWith("#") && !line.trim().isEmpty()) { | |
| // Split the line into key and value using '=' as the delimiter | |
| String[] keyValue = line.split("=", 2); | |
| if (keyValue.length < 2) { | |
| continue; // Skip malformed lines | |
| } | |
| // Split the key further using '.' as the delimiter | |
| String[] parts = keyValue[0].split("\\."); | |
| if (parts.length == 0) { | |
| continue; | |
| } | |
| String languageCode = parts[parts.length - 1]; | |
| // Split the value further using ',' as the delimiter | |
| String[] values = keyValue[1].split(","); | |
| if (values.length < 2) { | |
| continue; // Skip lines without proper country,displayName format | |
| } | |
| String country = values[0]; | |
| // displayName is available in values[1] if needed in the future | |
| // Add the values to the list. | |
| supportedLanguages.put(languageCode, country); | |
| if (!languageSupportedCountries.contains(country)) { | |
| languageSupportedCountries.add(country); | |
| } | |
| } | |
| } | |
| request.setAttribute(SUPPORTED_LANGUAGES_ATTR, supportedLanguages); | |
| request.setAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR, languageSupportedCountries); | |
| } catch (Exception e) { | |
| if (supportedLanguages.isEmpty()) { | |
| supportedLanguages.put("en", "US"); | |
| languageSupportedCountries.add("US"); | |
| } | |
| } | |
| } | |
| // Dynamically load supported languages from LanguageOptions.properties | |
| // Cache parsed results in application scope (ServletContext) to avoid per-request file I/O. | |
| Map<String, String> cachedSupportedLanguages = (Map<String, String>) application.getAttribute(SUPPORTED_LANGUAGES_ATTR); | |
| List<String> cachedLanguageSupportedCountries = (List<String>) application.getAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR); | |
| if (cachedSupportedLanguages != null && cachedLanguageSupportedCountries != null) { | |
| supportedLanguages = cachedSupportedLanguages; | |
| languageSupportedCountries = cachedLanguageSupportedCountries; | |
| } else { | |
| synchronized (application) { | |
| cachedSupportedLanguages = (Map<String, String>) application.getAttribute(SUPPORTED_LANGUAGES_ATTR); | |
| cachedLanguageSupportedCountries = (List<String>) application.getAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR); | |
| if (cachedSupportedLanguages != null && cachedLanguageSupportedCountries != null) { | |
| supportedLanguages = cachedSupportedLanguages; | |
| languageSupportedCountries = cachedLanguageSupportedCountries; | |
| } else { | |
| Properties props = new Properties(); | |
| try (java.io.InputStream in = application.getResourceAsStream("/WEB-INF/classes/LanguageOptions.properties")) { | |
| if (in != null) { | |
| props.load(new InputStreamReader(in, StandardCharsets.UTF_8)); | |
| } | |
| } | |
| for (String key : props.stringPropertyNames()) { | |
| String[] parts = key.split("\\."); | |
| if (parts.length == 0) { | |
| continue; | |
| } | |
| String languageCode = parts[parts.length - 1].trim(); | |
| String raw = String.valueOf(props.getProperty(key, "")).trim(); | |
| String[] values = raw.split(",", 2); | |
| if (values.length < 1) { | |
| continue; | |
| } | |
| String country = values[0].trim(); | |
| if (languageCode.isEmpty() || country.isEmpty()) { | |
| continue; | |
| } | |
| supportedLanguages.put(languageCode, country); | |
| if (!languageSupportedCountries.contains(country)) { | |
| languageSupportedCountries.add(country); | |
| } | |
| } | |
| if (supportedLanguages.isEmpty()) { | |
| supportedLanguages.clear(); | |
| languageSupportedCountries.clear(); | |
| supportedLanguages.put("en", "US"); | |
| languageSupportedCountries.add("US"); | |
| } | |
| application.setAttribute(SUPPORTED_LANGUAGES_ATTR, java.util.Collections.unmodifiableMap(new HashMap<>(supportedLanguages))); | |
| application.setAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR, java.util.Collections.unmodifiableList(new ArrayList<>(languageSupportedCountries))); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
@identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
around lines 117 - 170, The code is not actually caching in application scope,
uses fragile file I/O and manual parsing, and swallows errors; change loading to
read /WEB-INF/classes/LanguageOptions.properties via
application.getResourceAsStream(...) and parse with
java.util.Properties.load(Reader) into trimmed keys/values, populate the
Map<String,String> supportedLanguages and List<String>
languageSupportedCountries deterministically (clear them first) and then store
them with application.setAttribute(SUPPORTED_LANGUAGES_ATTR, supportedLanguages)
and application.setAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR,
languageSupportedCountries) so it is cached application-wide (not per-request),
and on any load error or missing stream call application.log(...) with the
exception/message and reset supportedLanguages to a deterministic default (e.g.,
put("en","US") and languageSupportedCountries.clear(); add "US") before storing.
Comment on lines
127
to
169
| String filePath = application.getRealPath("/") + "/WEB-INF/classes/LanguageOptions.properties"; | ||
| // Use a BufferedReader to read the file content | ||
| try (BufferedReader bufferedReader = new BufferedReader( | ||
| new InputStreamReader(new FileInputStream(filePath), StandardCharsets.UTF_8))) { | ||
| String line; | ||
| while ((line = bufferedReader.readLine()) != null) { | ||
| // Ignore comments and empty lines | ||
| if (!line.trim().startsWith("#") && !line.trim().isEmpty()) { | ||
| // Split the line into key and value using '=' as the delimiter | ||
| String[] keyValue = line.split("=", 2); | ||
| if (keyValue.length < 2) { | ||
| continue; // Skip malformed lines | ||
| } | ||
| // Split the key further using '.' as the delimiter | ||
| String[] parts = keyValue[0].split("\\."); | ||
| if (parts.length == 0) { | ||
| continue; | ||
| } | ||
| String languageCode = parts[parts.length - 1]; | ||
| // Split the value further using ',' as the delimiter | ||
| String[] values = keyValue[1].split(","); | ||
| if (values.length < 2) { | ||
| continue; // Skip lines without proper country,displayName format | ||
| } | ||
| String country = values[0]; | ||
| // displayName is available in values[1] if needed in the future | ||
| // Add the values to the list. | ||
| supportedLanguages.put(languageCode, country); | ||
| if (!languageSupportedCountries.contains(country)) { | ||
| languageSupportedCountries.add(country); | ||
| } | ||
| } | ||
| } | ||
| request.setAttribute(SUPPORTED_LANGUAGES_ATTR, supportedLanguages); | ||
| request.setAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR, languageSupportedCountries); | ||
| } catch (Exception e) { | ||
| if (supportedLanguages.isEmpty()) { | ||
| supportedLanguages.put("en", "US"); | ||
| languageSupportedCountries.add("US"); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Please log failures (don’t silently swallow).
Even if you keep the current approach, add application.log("...", e) in the catch so mispackaging / missing file doesn’t silently degrade language support.
🤖 Prompt for AI Agents
In
@identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
around lines 127 - 169, The catch block that currently swallows exceptions when
loading LanguageOptions.properties should log the failure; update the
catch(Exception e) in localize.jsp to call application.log with a clear message
and the exception (e) before falling back to the default entries
(supportedLanguages.put("en","US") and languageSupportedCountries.add("US")), so
any missing file or parse error is recorded for diagnostics while preserving the
existing fallback behavior and subsequent request.setAttribute calls.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
@identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp:
- Around line 23-24: The code imports and uses FileReader (which relies on
platform default encoding) causing potential mis-parsing of non-ASCII
properties; replace the FileReader usage and import in localize.jsp with an
explicit UTF-8 reader (e.g., use InputStreamReader(new FileInputStream(...),
StandardCharsets.UTF_8) or java.nio.file.Files.newBufferedReader(path,
StandardCharsets.UTF_8)) and remove the FileReader import; update the code at
the location currently referenced (around line 129) to open the properties file
with the chosen UTF-8-aware reader and keep BufferedReader for line reading.
- Line 126: The code constructs filePath using application.getRealPath("/")
which can be null for packed WARs; instead open LanguageOptions.properties via
the servlet context resource stream: replace the filePath + file access with
ServletContext.getResourceAsStream("/WEB-INF/classes/LanguageOptions.properties")
(or use the classloader getResourceAsStream for classpath lookup) and read the
properties from the returned InputStream; add the InputStream import and ensure
you null-check the stream and close it (or use try-with-resources) where the
filePath variable and application.getRealPath("/") are currently used.
- Around line 116-124: The comment promises application-scoped caching but the
code uses request.getAttribute/setAttribute; change reads/writes to use the
servlet/application scope instead (use
servletContext.getAttribute(SUPPORTED_LANGUAGES_ATTR) and
servletContext.setAttribute(...) or equivalent) so the parsed map/list are
stored application-wide, and when instantiating default containers for
supportedLanguages and languageSupportedCountries use thread-safe collections
(e.g., ConcurrentHashMap for supportedLanguages and a thread-safe List like
CopyOnWriteArrayList or Collections.synchronizedList) before parsing and storing
them under SUPPORTED_LANGUAGES_ATTR and LANGUAGE_SUPPORTED_COUNTRIES_ATTR so
parsing happens once and is safe for concurrent requests.
- Around line 162-164: The catch(Exception e) block currently just rethrows the
exception; update it to log the exception (e.g., using a logger or
servletContext.log) and ensure any attributes populated earlier (the locale
collections placed into request/session) are either cleared or replaced with
safe fallbacks (empty lists or a default locale list) to avoid
partially-populated state; do this by catching the exception, calling
logger.error("Failed to load locale properties", e) (or servletContext.log) and
then set the same attribute names used earlier to empty collections or a known
default before rethrowing or handling the error gracefully.
🧹 Nitpick comments (1)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp (1)
129-158: Consider using Properties class for more robust parsing.The manual line-by-line parsing doesn't handle standard properties file features like escaped characters (\n, \t, unicode escapes), multi-line values, or leading/trailing whitespace. While the current implementation handles basic cases, it's fragile and may fail on edge cases.
♻️ Proposed refactor using Properties class
+ InputStream inputStream = application.getResourceAsStream("/WEB-INF/classes/LanguageOptions.properties"); + Properties properties = new Properties(); - // Use a BufferedReader to read the file content - try (BufferedReader bufferedReader = new BufferedReader(new FileReader(filePath))) { - String line; - while ((line = bufferedReader.readLine()) != null) { - // Ignore comments and empty lines - if (!line.trim().startsWith("#") && !line.trim().isEmpty()) { - // Split the line into key and value using '=' as the delimiter - String[] keyValue = line.split("=", 2); - if (keyValue.length < 2) { - continue; // Skip malformed lines - } - // Split the key further using '.' as the delimiter - String[] parts = keyValue[0].split("\\."); - if (parts.length == 0) { - continue; - } - String languageCode = parts[parts.length - 1]; - // Split the value further using ',' as the delimiter - String[] values = keyValue[1].split(","); - if (values.length < 2) { - continue; // Skip lines without proper country,displayName format - } - String country = values[0]; - // displayName is available in values[1] if needed in the future - // Add the values to the list. - supportedLanguages.put(languageCode, country); - if (!languageSupportedCountries.contains(country)) { - languageSupportedCountries.add(country); - } - } - } + try (InputStreamReader reader = new InputStreamReader(inputStream, StandardCharsets.UTF_8)) { + properties.load(reader); + + for (String key : properties.stringPropertyNames()) { + // Split the key using '.' as the delimiter + String[] parts = key.split("\\."); + if (parts.length == 0) { + continue; + } + String languageCode = parts[parts.length - 1]; + + // Split the value using ',' as the delimiter + String value = properties.getProperty(key); + String[] values = value.split(",", 2); + if (values.length < 2) { + continue; // Skip entries without proper country,displayName format + } + String country = values[0].trim(); + + // Add the values to the collections + supportedLanguages.put(languageCode, country); + if (!languageSupportedCountries.contains(country)) { + languageSupportedCountries.add(country); + } + }This approach:
- Properly handles properties file escaping and encoding
- Leverages standard Java Properties parsing
- Still extracts your custom country,displayName format
- More maintainable and less error-prone
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
🔇 Additional comments (1)
identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp (1)
104-105: LGTM!The cache attribute names are clear and well-defined.
Comment on lines
+23
to
+24
| <%@ page import="java.io.FileReader" %> | ||
| <%@ page import="java.io.BufferedReader" %> |
Contributor
There was a problem hiding this comment.
Use explicit character encoding when reading the properties file.
FileReader uses the platform default encoding, which can vary across systems and lead to incorrect parsing of non-ASCII characters in language names or values.
📝 Proposed fix using explicit UTF-8 encoding
Replace the FileReader import and usage with InputStreamReader or use java.nio:
-<%@ page import="java.io.FileReader" %>
-<%@ page import="java.io.BufferedReader" %>
+<%@ page import="java.io.InputStreamReader" %>
+<%@ page import="java.io.BufferedReader" %>
+<%@ page import="java.io.FileInputStream" %>
+<%@ page import="java.nio.charset.StandardCharsets" %>Then update line 129 to:
- try (BufferedReader bufferedReader = new BufferedReader(new FileReader(filePath))) {
+ try (BufferedReader bufferedReader = new BufferedReader(
+ new InputStreamReader(new FileInputStream(filePath), StandardCharsets.UTF_8))) {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
@identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
around lines 23 - 24, The code imports and uses FileReader (which relies on
platform default encoding) causing potential mis-parsing of non-ASCII
properties; replace the FileReader usage and import in localize.jsp with an
explicit UTF-8 reader (e.g., use InputStreamReader(new FileInputStream(...),
StandardCharsets.UTF_8) or java.nio.file.Files.newBufferedReader(path,
StandardCharsets.UTF_8)) and remove the FileReader import; update the code at
the location currently referenced (around line 129) to open the properties file
with the chosen UTF-8-aware reader and keep BufferedReader for line reading.
Comment on lines
+116
to
+124
| // Dynamically load supported languages from languageOptions.properties | ||
| // Use application scope to cache the parsed language options | ||
| Map<String, String> cachedSupportedLanguages = (Map<String, String>) request.getAttribute(SUPPORTED_LANGUAGES_ATTR); | ||
| List<String> cachedLanguageSupportedCountries = (List<String>) request.getAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR); | ||
| if (cachedSupportedLanguages != null && cachedLanguageSupportedCountries != null) { | ||
| supportedLanguages = cachedSupportedLanguages; | ||
| languageSupportedCountries = cachedLanguageSupportedCountries; | ||
| } else { |
Contributor
There was a problem hiding this comment.
Critical mismatch: Comment indicates application scope but implementation uses request scope.
The comment on line 117 states "Use application scope to cache" but the implementation uses request.getAttribute/setAttribute, which is request-scoped. This means the properties file will be read and parsed on every single request, causing significant performance overhead.
Impact:
- File I/O and parsing occur on every request
- Unnecessary resource consumption
- Reduced throughput and increased response time
Recommendation:
Either update the implementation to use application scope for true caching, or update the comment to accurately reflect request-scoped behavior. For production use, application-scoped caching is strongly recommended.
♻️ Proposed fix using application scope with thread-safe collections
- // Dynamically load supported languages from languageOptions.properties
- // Use application scope to cache the parsed language options
- Map<String, String> cachedSupportedLanguages = (Map<String, String>) request.getAttribute(SUPPORTED_LANGUAGES_ATTR);
- List<String> cachedLanguageSupportedCountries = (List<String>) request.getAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR);
+ // Dynamically load supported languages from languageOptions.properties
+ // Use application scope to cache the parsed language options
+ Map<String, String> cachedSupportedLanguages = (Map<String, String>) application.getAttribute(SUPPORTED_LANGUAGES_ATTR);
+ List<String> cachedLanguageSupportedCountries = (List<String>) application.getAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR);At line 113, initialize with thread-safe collections:
- Map<String, String> supportedLanguages = new HashMap<>();
- List<String> languageSupportedCountries = new ArrayList<>();
+ Map<String, String> supportedLanguages = new java.util.concurrent.ConcurrentHashMap<>();
+ List<String> languageSupportedCountries = new java.util.concurrent.CopyOnWriteArrayList<>();And at lines 160-161:
- request.setAttribute(SUPPORTED_LANGUAGES_ATTR, supportedLanguages);
- request.setAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR, languageSupportedCountries);
+ application.setAttribute(SUPPORTED_LANGUAGES_ATTR, supportedLanguages);
+ application.setAttribute(LANGUAGE_SUPPORTED_COUNTRIES_ATTR, languageSupportedCountries);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
@identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
around lines 116 - 124, The comment promises application-scoped caching but the
code uses request.getAttribute/setAttribute; change reads/writes to use the
servlet/application scope instead (use
servletContext.getAttribute(SUPPORTED_LANGUAGES_ATTR) and
servletContext.setAttribute(...) or equivalent) so the parsed map/list are
stored application-wide, and when instantiating default containers for
supportedLanguages and languageSupportedCountries use thread-safe collections
(e.g., ConcurrentHashMap for supportedLanguages and a thread-safe List like
CopyOnWriteArrayList or Collections.synchronizedList) before parsing and storing
them under SUPPORTED_LANGUAGES_ATTR and LANGUAGE_SUPPORTED_COUNTRIES_ATTR so
parsing happens once and is safe for concurrent requests.
| languageSupportedCountries = cachedLanguageSupportedCountries; | ||
| } else { | ||
| // Specify the file path | ||
| String filePath = application.getRealPath("/") + "/WEB-INF/classes/LanguageOptions.properties"; |
Contributor
There was a problem hiding this comment.
File path construction may fail in certain deployment scenarios.
application.getRealPath("/") returns null when the WAR file is not extracted to the filesystem (e.g., running from a packed WAR). This will cause a NullPointerException during path construction.
Recommendation:
Use ServletContext.getResourceAsStream() for more portable resource access that works with both extracted and packed WARs.
♻️ Proposed fix using getResourceAsStream
- // Specify the file path
- String filePath = application.getRealPath("/") + "/WEB-INF/classes/LanguageOptions.properties";
-
- // Use a BufferedReader to read the file content
- try (BufferedReader bufferedReader = new BufferedReader(new FileReader(filePath))) {
+ // Load the properties file as a resource stream
+ try (InputStream inputStream = application.getResourceAsStream("/WEB-INF/classes/LanguageOptions.properties");
+ BufferedReader bufferedReader = new BufferedReader(
+ new InputStreamReader(inputStream, StandardCharsets.UTF_8))) {
+ if (inputStream == null) {
+ throw new IOException("LanguageOptions.properties not found");
+ }Note: This also requires adding the InputStream import:
<%@ page import="java.io.InputStream" %>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
@identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
at line 126, The code constructs filePath using application.getRealPath("/")
which can be null for packed WARs; instead open LanguageOptions.properties via
the servlet context resource stream: replace the filePath + file access with
ServletContext.getResourceAsStream("/WEB-INF/classes/LanguageOptions.properties")
(or use the classloader getResourceAsStream for classpath lookup) and read the
properties from the returned InputStream; add the InputStream import and ensure
you null-check the stream and close it (or use try-with-resources) where the
filePath variable and application.getRealPath("/") are currently used.
Comment on lines
+162
to
+164
| } catch (Exception e) { | ||
| throw e; | ||
| } |
Contributor
There was a problem hiding this comment.
Improve exception handling with logging and consider fallback behavior.
The current exception handling simply rethrows the exception without logging or providing a fallback. This creates several issues:
- If the properties file is missing or corrupted, the locale selection logic will fail with empty collections
- No logging makes debugging difficult in production
- If an exception occurs during parsing, partially populated collections may be stored in attributes (lines 160-161), leading to inconsistent state
Recommendation:
Add logging and consider providing fallback to ensure graceful degradation if the properties file cannot be loaded.
♻️ Proposed fix with logging and empty collection handling
} catch (Exception e) {
- throw e;
+ // Log the error for debugging
+ System.err.println("Failed to load LanguageOptions.properties: " + e.getMessage());
+ e.printStackTrace();
+
+ // Ensure collections are initialized even on failure
+ // Application will fall back to default "en_US" locale
+ if (supportedLanguages.isEmpty()) {
+ supportedLanguages.put("en", "US");
+ languageSupportedCountries.add("US");
+ }
}
}Alternatively, if failure to load the properties file should prevent application startup:
} catch (Exception e) {
+ System.err.println("Critical: Failed to load LanguageOptions.properties: " + e.getMessage());
+ e.printStackTrace();
throw e;
}📝 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.
Suggested change
| } catch (Exception e) { | |
| throw e; | |
| } | |
| } catch (Exception e) { | |
| // Log the error for debugging | |
| System.err.println("Failed to load LanguageOptions.properties: " + e.getMessage()); | |
| e.printStackTrace(); | |
| // Ensure collections are initialized even on failure | |
| // Application will fall back to default "en_US" locale | |
| if (supportedLanguages.isEmpty()) { | |
| supportedLanguages.put("en", "US"); | |
| languageSupportedCountries.add("US"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
@identity-apps-core/apps/authentication-portal/src/main/webapp/includes/localize.jsp
around lines 162 - 164, The catch(Exception e) block currently just rethrows the
exception; update it to log the exception (e.g., using a logger or
servletContext.log) and ensure any attributes populated earlier (the locale
collections placed into request/session) are either cleared or replaced with
safe fallbacks (empty lists or a default locale list) to avoid
partially-populated state; do this by catching the exception, calling
logger.error("Failed to load locale properties", e) (or servletContext.log) and
then set the same attribute names used earlier to empty collections or a known
default before rethrowing or handling the error gracefully.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
Load supported languages dynamically from the
LanguageOptions.propertiesfile instead of using hardcoded mappings inlocalize.jsp. This change improves maintainability by allowing language configuration to be managed through the properties file without requiring code changes.Changes:
Related Issues
Related PRs
Checklist
Security checks
Developer Checklist (Mandatory)
product-isissue to track any behavioral change or migration impact.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.