Merge pull request #35777 from vespa-engine/hmusum/throw-IllegalArgumentException-when-getting-unknown-language-tag

Throw IllegalArgumentException when unknown language tag is used in set_language

Author: bratseth

indexinglanguage/src/test/java/com/yahoo/vespa/indexinglanguage/expressions/SetLanguageTestCase.java

@@ -10,8 +10,8 @@
 import static com.yahoo.vespa.indexinglanguage.expressions.ExpressionAssert.assertVerify;
 import static com.yahoo.vespa.indexinglanguage.expressions.ExpressionAssert.assertVerifyThrows;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertThrows;
 
 /**
  * @author Simon Thoresen Hult
@@ -57,4 +57,11 @@ public void testSettingMissingValue() {
         assertEquals(Language.UNKNOWN, ctx.getLanguage());
     }
 
+    @Test
+    public void testSettingIllegalValue() {
+        ExecutionContext ctx = new ExecutionContext(new SimpleTestAdapter());
+        ctx.setCurrentValue(new StringFieldValue("this is part of a long string which is mistakenly used as language"));
+        assertThrows(IllegalArgumentException.class, () -> new SetLanguageExpression().execute(ctx));
+    }
+
 }

linguistics/src/main/java/com/yahoo/language/LocaleFactory.java

@@ -1,6 +1,9 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.language;
 
+import com.yahoo.text.Text;
+
+import java.util.IllformedLocaleException;
 import java.util.Locale;
 import java.util.Objects;
 
@@ -21,6 +24,15 @@ private LocaleFactory() {}
      */
     public static Locale fromLanguageTag(String tag) {
         Objects.requireNonNull(tag, "tag cannot be null");
+        // Tags can have variants and extension making them arbitrarily long,
+        // but tags exceeding 35-40 characters are extremely rare in production systems.
+        // Truncate in case people mistakenly use large text fields as tags
+        var truncatedTag = Text.truncate(tag, 45);
+        // truncate adds whitespace, so check original tag for whitespace
+        if (tag.contains(" ")) {
+            throw new IllegalArgumentException("Illegal language tag '" + truncatedTag + "', language tags corresponding to RFC5646 cannot contain whitespace");
+        }
+        tag = truncatedTag;
 
         tag = tag.trim();
         if (tag.isEmpty()) return UNKNOWN;
@@ -45,7 +57,11 @@ public static Locale fromLanguageTag(String tag) {
             }
         }
         if (language.isEmpty()) return UNKNOWN;
-        return new Locale.Builder().setLanguage(language).setScript(script).setRegion(region).build();
+        try {
+            return new Locale.Builder().setLanguage(language).setScript(script).setRegion(region).build();
+        } catch (IllformedLocaleException e) {
+            throw new IllegalArgumentException("Illegal language tag '" + tag + "', it must be a language tag corresponding to RFC5646");
+        }
     }
 
 }

linguistics/src/test/java/com/yahoo/language/LocaleFactoryTestCase.java

@@ -6,6 +6,7 @@
 import java.util.Locale;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -31,16 +32,22 @@ public void requireThatLocaleCanBeCreatedFromLanguageTag() {
         assertLocale("es", "es", "", "");
         assertLocale("es-419", "es", "", "419");
 
-        try {
-            LocaleFactory.fromLanguageTag(null);
-            fail();
-        } catch (NullPointerException e) {
-
-        }
+        assertThrows(NullPointerException.class, () ->LocaleFactory.fromLanguageTag(null));
 
         assertLocale("", "", "", "");
         assertLocale("z-foo", "", "", "");
         assertLocale("ojeroierhoiherohjdadsfodsfoifiopeoipefwoipfwe", "", "", "");
+
+        var exception = assertThrows(IllegalArgumentException.class,
+                                     () -> LocaleFactory.fromLanguageTag("foo-13-ojeroierhoiherohjdadsfodsfoifiopeoipefwoipfwe"));
+        assertEquals("Illegal language tag 'foo-13-ojeroierhoiherohjdadsfodsfoifiopeo ...', it must be a language tag corresponding to RFC5646",
+                     exception.getMessage());
+
+        var languageTag = "this is part of a long string which should be cut off at 20 characters";
+        exception = assertThrows(IllegalArgumentException.class,
+                                     () -> LocaleFactory.fromLanguageTag(languageTag));
+        assertEquals("Illegal language tag 'this is part of a long string which shoul ...', language tags corresponding to RFC5646 cannot contain whitespace",
+                     exception.getMessage());
     }
 
     private static void assertLocale(String tag, String language, String variant, String country) {