validate language subtag in GwtLocale fromString

[Samin061]

On the server the active locale is read straight from the request by GwtServletBase.getGwtLocale (the locale query parameter or a cookie) and stored as the thread 'locale' property, and GwtLocaleFactoryImpl.fromString then parses that string into subtags. It checks the script, region and variant components against length and character rules but takes the primary language subtag as-is with only a lower-casing pass, and the code still carries a TODO to verify the language tag. Because the split only happens on '-' and '_', a value such as locale=java.lang.Runtime survives intact as the language, and LocalizableInstantiator concatenates that language into class names it passes to Class.forName().newInstance() when server code calls GWT.create on a Localizable, so '.', '$' and '/' can be smuggled into the reflective lookup. I came across it while tracing where the untrusted locale property finally lands. The fix validates the assembled language tag against the same BCP47 subtag shape the other components already require and throws IllegalArgumentException otherwise, kept inside fromString so every caller is covered without disturbing the parse flow. Existing locales, including extended-language and private-use forms like zh-cmn and x-foo123, are unaffected.

[Samin061]

any update?

[niloc132]

failing style check on this line - add an assertion about the exception?

[zbynek]

The rule description https://errorprone.info/bugpattern/EmptyCatch specifically suggests using assertThrows. Since we're stuck with old JUnit for the time being, it might make sense to add an own com.google.gwt.testing.server.Assertions utility class that provides assertThrows (assertDoesNotThrow can be added later if needed). Note that some version of assertThrows was added to the emul tests, but one that's really equivalent to the Jupiter implementation can only be provided for server tests.

Then this would be more readable:

assertThrows(IllegalArgumentException.class, () -> factory.fromString(locale), 
    "Should have thrown IllegalArgumentException on " + locale);

[Samin061]

Added com.google.gwt.testing.server.Assertions with a Jupiter-style assertThrows(Class, Executable, String) that returns the thrown exception, and the test now calls it instead of the try/fail/empty-catch, so the EmptyCatch check is gone. Kept it to assertThrows for now and can add assertDoesNotThrow later if it's useful.

[niloc132]

This is not a safe way to validate BCP47, as it will permit non-ascii letters and numbers

https://docs.oracle.com/javase/8/docs/api/java/lang/Character.html#isLetterOrDigit-char-

for example, https://docs.oracle.com/javase/8/docs/api/java/lang/Character.html#isDigit-char- permits

• '\u0030' through '\u0039', ISO-LATIN-1 digits ('0' through '9')
• '\u0660' through '\u0669', Arabic-Indic digits
• '\u06F0' through '\u06F9', Extended Arabic-Indic digits
• '\u0966' through '\u096F', Devanagari digits
• '\uFF10' through '\uFF19', Fullwidth digits

and so on

[zbynek]

I didn't mention this one since the code is using Character.* methods also around line 70. But since the PR description mentions BCP47, it would be nice to stick to it as closely as possible 👍

[niloc132]

Right - if we're improving validation, let's validate it all the way.

[Samin061]

Good catch. Switched isAlphaNumeric to explicit ASCII ranges (a-z, A-Z, 0-9) so it no longer accepts the non-ASCII letters and digits Character.isLetterOrDigit allows. I left the existing isAlpha/isDigit alone since you noted those weren't in scope here. Confirmed a tag like en٠ (Arabic-Indic zero) now throws, while en, zh-cmn, i-klingon, x-foo123 and es-419 still parse unchanged.

[zbynek]

Other than the copyright header, this looks good.

[zbynek]

Suggested change

	* Copyright 2026 Google Inc.
	* Copyright 2026 GWT Project Authors

[Samin061]

Done, updated to GWT Project Authors.

[niloc132]

This is still in a client package despite being in a package named "server" - tests are failing, but only the one suite that uses the testing package

Tracing compile failure path for type 'com.google.gwt.testing.server.Assertions'
   [ERROR] Errors in 'file:/home/runner/work/gwt/gwt/gwt/user/test/com/google/gwt/testing/server/Assertions.java'
      [ERROR] Line 42: The method isInstance(Throwable) is undefined for the type Class<T>
      [ERROR] Line 43: The method cast(Throwable) is undefined for the type Class<T>
[ERROR] Aborting compile due to errors in some input files
Tracing compile failure path for type 'com.google.gwt.testing.server.Assertions'
   [ERROR] Errors in 'file:/home/runner/work/gwt/gwt/gwt/user/test/com/google/gwt/testing/server/Assertions.java'
      [ERROR] Line 42: The method isInstance(Throwable) is undefined for the type Class<T>
      [ERROR] Line 43: The method cast(Throwable) is undefined for the type Class<T>
[ERROR] Aborting compile due to errors in some input files

Worth nothing, this isnt "server" code anyway, just jvm code, but if you want a class like this, it needs to be in a non-client package. Or, if you are okay with it working in GWT too, get rid of the isInstance/cast usage so reflection isn't required.

[Samin061]

Since the point of the class was the Jupiter-equivalent semantics for JVM tests, I kept isInstance/cast and took the package out of the translatable path instead: TestUtils.gwt.xml now excludes server/** from its source, so the GWT compiler no longer sees it and the suite that inherits the testing module compiles again. Can move it to a different package outright if you'd rather not carry the exclude.