Use Java mechanisms to read language files and default to UTF-8 #21755

ahus1 · 2023-07-17T16:55:19Z

Discussion: #9270

ahus1 · 2023-07-17T16:57:19Z

While this is still in draft, it already contains updates to all property files and the docs.

services/src/main/java/org/keycloak/theme/FolderTheme.java

-            Charset encoding = PropertiesUtil.detectEncoding(new FileInputStream(file));
-            try (Reader reader = new InputStreamReader(new FileInputStream(file), encoding)) {
-                m.load(reader);
+            try (InputStream stream = Files.newInputStream(file.toPath())) {


ahus1 · 2023-07-18T08:02:46Z

It would be great to have a review from one of the maintainers on the Java side on this. Thanks!

hmlnarik

Approving for the Java part

jonkoops · 2023-07-19T09:13:21Z

@ahus1 looks like the code scanner has some issues with the changes introduced here.

ghost

Unreported flaky test detected, please review

ghost · 2023-07-20T11:34:08Z

Unreported flaky test detected

If the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.forms.LoginTest#loginDifferentUserAfterDisabledUserThrownOut

Keycloak CI - Forms IT (firefox)

java.lang.AssertionError: Expected LoginPage but was Keycloak Account Management (https://localhost:8543/auth/realms/test/account/#/)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at org.keycloak.testsuite.pages.LoginPage.open(LoginPage.java:236)
...

Report flaky test

ghost · 2023-07-20T13:17:51Z

Unreported flaky test detected

If the below flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.forms.LoginTest#loginDifferentUserAfterDisabledUserThrownOut

Keycloak CI - Forms IT (firefox)

java.lang.AssertionError: Expected LoginPage but was Keycloak Account Management (https://localhost:8543/auth/realms/test/account/#/)
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.keycloak.testsuite.pages.AbstractPage.assertCurrent(AbstractPage.java:47)
	at org.keycloak.testsuite.pages.LoginPage.open(LoginPage.java:236)
...

Report flaky test

ghost

Unreported flaky test detected, please review

ahus1 · 2023-07-22T14:10:24Z

@jonkoops - I've look at the CodeQL alert, and also added a Regex check for filenames - CodeQL still complains. The logic to create the filename didn't change in this PR, so I assume it is as safe as before. I'll follow up with Bruno to find out why CodeQL complains here.

jonkoops · 2023-07-22T14:25:11Z

@ahus1 sounds good to me, should we go ahead and merge this then?

ahus1 · 2023-07-22T14:27:37Z

@jonkoops - yes, I think this is the way to go. Please approve it for the UI part.
I'll also get an re-approval of a maintainer/Java developer, and then it will be ready to be merged.

jonkoops

LGTM

ahus1 · 2023-07-23T14:02:23Z

@hmlnarik - I've added one more commit to make CodeQL happy, but it continues to warn. I'd like to keep that change, and have it squashed when committing. I'll contact @abstractj separately to figure out how to make CodeQL happy.

Could you please re-approve and merge? Thanks!

ahus1 · 2023-07-26T07:42:56Z

Rebased to resolve conflicts on the docs. Also squashed the two commits.

mhajas

thank you for the PR @ahus1, I added two comments. Also, it seems a rebase is needed (sorry for the late review).

docs/documentation/release_notes/topics/23_0_0.adoc

docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc

Closes keycloak#21753

mhajas

Thank you @ahus1

ahus1 · 2023-07-31T11:56:51Z

@jonkoops - could you please re-review before we run into another merge conflict? Thank you very much!

ahus1 self-assigned this Jul 17, 2023

jonkoops previously approved these changes Jul 17, 2023

View reviewed changes

github-advanced-security bot found potential problems Jul 17, 2023

View reviewed changes

ghost added the flaky-test label Jul 17, 2023

This was referenced Jul 17, 2023

Flaky test: org.keycloak.testsuite.admin.authentication.FlowTest#testAddRemoveFlow #20763

Closed

Flaky test: org.keycloak.testsuite.model.session.SessionTimeoutsTest#testOfflineUserClientIdleTimeoutSmallerThanSessionNoRefresh #20874

Closed

ahus1 marked this pull request as ready for review July 18, 2023 08:01

ahus1 requested review from a team as code owners July 18, 2023 08:01

ghost added the team/ui label Jul 18, 2023

jonkoops mentioned this pull request Jul 18, 2023

Encoded special chars in utf-8 and improved some translations #21446

Closed

hmlnarik reviewed Jul 19, 2023

View reviewed changes

hmlnarik previously approved these changes Jul 19, 2023

View reviewed changes

jonkoops mentioned this pull request Jul 19, 2023

Message properties aren't getting parsed in UTF-8 #10821

Closed

ahus1 dismissed stale reviews from hmlnarik and jonkoops via d8bfd8c July 19, 2023 18:17

ahus1 force-pushed the is-21753-standardize-translation-handling branch from d8bfd8c to 36c1c05 Compare July 20, 2023 10:07

ghost reviewed Jul 20, 2023

View reviewed changes

ahus1 force-pushed the is-21753-standardize-translation-handling branch from 36c1c05 to 471daad Compare July 22, 2023 12:29

This was referenced Jul 22, 2023

Flaky test: org.keycloak.testsuite.broker.KcOidcBrokerTest#testReauthenticationBothBrokersWithOTPRequired #21834

Closed

Flaky test: org.keycloak.testsuite.broker.KcOidcBrokerTest#testReauthenticationOIDCBrokerWithOTPRequired #21835

Closed

jonkoops previously approved these changes Jul 22, 2023

View reviewed changes

ahus1 dismissed jonkoops’s stale review via e7c31c1 July 26, 2023 07:42

ahus1 force-pushed the is-21753-standardize-translation-handling branch from 471daad to e7c31c1 Compare July 26, 2023 07:42

mhajas requested changes Jul 31, 2023

View reviewed changes

docs/documentation/release_notes/topics/23_0_0.adoc Outdated Show resolved Hide resolved

docs/documentation/upgrading/topics/keycloak/changes-23_0_0.adoc Outdated Show resolved Hide resolved

ahus1 force-pushed the is-21753-standardize-translation-handling branch from e7c31c1 to badab44 Compare July 31, 2023 10:05

Use Java mechanisms to read language files and default to UTF-8

21eda42

Closes keycloak#21753

ahus1 force-pushed the is-21753-standardize-translation-handling branch from badab44 to 21eda42 Compare July 31, 2023 10:05

ahus1 requested a review from mhajas July 31, 2023 10:07

mhajas approved these changes Jul 31, 2023

View reviewed changes

mhajas enabled auto-merge (squash) July 31, 2023 11:27

ahus1 requested a review from jonkoops July 31, 2023 11:56

jonkoops approved these changes Aug 1, 2023

View reviewed changes

mhajas merged commit 748c53d into keycloak:main Aug 1, 2023
62 of 64 checks passed

ahus1 deleted the is-21753-standardize-translation-handling branch September 21, 2023 17:11

This was referenced Nov 1, 2023

Localize Spanish language. #24467

Closed

Remove UTF-8 encoding header from property files #24471

Merged

jonkoops mentioned this pull request Feb 26, 2024

FolderTheme does not support Locales with extensions #27284

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Java mechanisms to read language files and default to UTF-8 #21755

Use Java mechanisms to read language files and default to UTF-8 #21755

ahus1 commented Jul 17, 2023 •

edited

Loading

ahus1 commented Jul 17, 2023

ahus1 commented Jul 18, 2023

hmlnarik left a comment

jonkoops commented Jul 19, 2023

ghost left a comment

ghost commented Jul 20, 2023

ghost commented Jul 20, 2023

ghost left a comment

ahus1 commented Jul 22, 2023

jonkoops commented Jul 22, 2023

ahus1 commented Jul 22, 2023

jonkoops left a comment

ahus1 commented Jul 23, 2023

ahus1 commented Jul 26, 2023

mhajas left a comment

mhajas left a comment

ahus1 commented Jul 31, 2023

Use Java mechanisms to read language files and default to UTF-8 #21755

Use Java mechanisms to read language files and default to UTF-8 #21755

Conversation

ahus1 commented Jul 17, 2023 • edited Loading

ahus1 commented Jul 17, 2023

ahus1 commented Jul 18, 2023

hmlnarik left a comment

Choose a reason for hiding this comment

jonkoops commented Jul 19, 2023

ghost left a comment

Choose a reason for hiding this comment

ghost commented Jul 20, 2023

Unreported flaky test detected

org.keycloak.testsuite.forms.LoginTest#loginDifferentUserAfterDisabledUserThrownOut

ghost commented Jul 20, 2023

Unreported flaky test detected

org.keycloak.testsuite.forms.LoginTest#loginDifferentUserAfterDisabledUserThrownOut

ghost left a comment

Choose a reason for hiding this comment

ahus1 commented Jul 22, 2023

jonkoops commented Jul 22, 2023

ahus1 commented Jul 22, 2023

jonkoops left a comment

Choose a reason for hiding this comment

ahus1 commented Jul 23, 2023

ahus1 commented Jul 26, 2023

mhajas left a comment

Choose a reason for hiding this comment

mhajas left a comment

Choose a reason for hiding this comment

ahus1 commented Jul 31, 2023

ahus1 commented Jul 17, 2023 •

edited

Loading