Merge pull request #136 from box/android-escape-html

jeeyi · web-flow · commit f7e22ffbaed4 · 2016-11-07T08:44:52.000-08:00
escape html when there are variables between html tags
diff --git a/webapp/src/main/java/com/box/l10n/mojito/okapi/filters/XMLEncoder.java b/webapp/src/main/java/com/box/l10n/mojito/okapi/filters/XMLEncoder.java
@@ -1,25 +1,41 @@
 package com.box.l10n.mojito.okapi.filters;
 
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import net.sf.okapi.common.encoder.EncoderContext;
 import net.sf.okapi.filters.its.Parameters;
 import org.apache.commons.lang.StringUtils;
 
 /**
- * This overrides the {@link net.sf.okapi.common.encoder.XMLEncoder} 
- * to not to escape supported HTML elements for Android strings.
+ * This overrides the {@link net.sf.okapi.common.encoder.XMLEncoder} for Android
+ * strings.
+ *
+ * It does not escape supported HTML elements for Android strings unless there
+ * are variables within the HTML elements.
+ * For example, <b>songs</b> vs. &lt;b>%d songs&lt;/b>
+ *
  * Also it overrides the default quotemode setting so the quotes do not get escaped.
- * According to Android specification in http://developer.android.com/guide/topics/resources/string-resource.html,
- * <b>bold</b>, <i>italian</i> and <u>underline</u> should be in localized file as-is.
- * 
+ *
+ * For detailed information, see to Android specification in
+ * http://developer.android.com/guide/topics/resources/string-resource.html,
+ *
  * @author jyi
  */
 public class XMLEncoder extends net.sf.okapi.common.encoder.XMLEncoder {
-    
+
+    // trying to match variables between html tags, for example, <b>%d</b>, <i>%1$s</i>, <u>%2$s</u>
+    private static final Pattern ANDROID_VARIABLE_WITHIN_HTML = Pattern.compile("(&lt;[b|i|u]&gt;)((.*?)%(([-0+ #]?)[-0+ #]?)((\\d\\$)?)(([\\d\\*]*)(\\.[\\d\\*]*)?)[dioxXucsfeEgGpn](.*?))+(&lt;/[b|i|u]&gt;)");
+    private static final Pattern ANDROID_HTML = Pattern.compile("(&lt;)(/?)(b|i|u)(&gt;)");
+    private static final Pattern UNESCAPED_DOUBLE_QUOTE = Pattern.compile("([^\\\\])(\")");
+    private static final Pattern START_WITH_DOUBLE_QUOTE = Pattern.compile("(^\")");
+    private static final Pattern UNESCAPED_SINGLE_QUOTE = Pattern.compile("([^\\\\])(')");
+    private static final Pattern START_WITH_SINGLE_QUOTE = Pattern.compile("(^')");
+
     @Override
     public String encode(String text, EncoderContext context) {
         String encoded = super.encode(text, context);
         if (isAndroidStrings()) {
-            encoded = escape(encoded);
+            encoded = escapeAndroid(encoded);
         }
         return encoded;
     }
@@ -32,39 +48,44 @@ private boolean isAndroidStrings() {
             return false;
         }
     }
-    
-    public String escape(String text) {
+
+    public String escapeAndroid(String text) {
         boolean enclosedInDoubleQuotes = StringUtils.startsWith(text, "\"") && StringUtils.endsWith(text, "\"");
         if (enclosedInDoubleQuotes) {
             text = text.substring(1, text.length() - 1);
         }
-        
-        String pattern = "(&lt;)(/?)(b|i|u)(&gt;)";
-        String replacement = "<$2$3>";
-        text = text.replaceAll(pattern, replacement);
+
+        String replacement;
+        if (needsAndroidEscapeHTML(text)) {
+            replacement = "$1$2$3>";
+        } else {
+            replacement = "<$2$3>";
+        }
+        text = ANDROID_HTML.matcher(text).replaceAll(replacement);
         text = text.replaceAll("\n", "\\\\n");
         text = text.replaceAll("\r", "\\\\r");
         text = escapeDoubleQuotes(text);
         if (!enclosedInDoubleQuotes) {
             text = escapeSingleQuotes(text);
         }
         return enclosedInDoubleQuotes ? "\"" + text + "\"" : text;
-        
+
     }
-    
+
+    private boolean needsAndroidEscapeHTML(String text) {
+        Matcher matcher = ANDROID_VARIABLE_WITHIN_HTML.matcher(text);
+        return matcher.find();
+    }
+
     private String escapeDoubleQuotes(String text) {
-        String pattern1 = "([^\\\\])(\")";
-        String pattern2 = "(^\")";  
-        String escaped = text.replaceAll(pattern1, "$1\\\\$2");
-        escaped = escaped.replaceFirst(pattern2, "\\\\$1");
+        String escaped = UNESCAPED_DOUBLE_QUOTE.matcher(text).replaceAll("$1\\\\$2");
+        escaped = START_WITH_DOUBLE_QUOTE.matcher(escaped).replaceFirst("\\\\$1");
         return escaped;
     }
     
     private String escapeSingleQuotes(String text) {
-        String pattern1 = "([^\\\\])(')";
-        String pattern2 = "(^')";    
-        String escaped = text.replaceAll(pattern1, "$1\\\\$2");
-        escaped = escaped.replaceFirst(pattern2, "\\\\$1");
+        String escaped = UNESCAPED_SINGLE_QUOTE.matcher(text).replaceAll("$1\\\\$2");
+        escaped = START_WITH_SINGLE_QUOTE.matcher(escaped).replaceFirst("\\\\$1");
         return escaped;
     }
 }
diff --git a/webapp/src/main/resources/com/box/l10n/mojito/okapi/filters/AndroidStrings_mojito.fprm b/webapp/src/main/resources/com/box/l10n/mojito/okapi/filters/AndroidStrings_mojito.fprm
@@ -29,7 +29,8 @@
  <its:preserveSpaceRule selector="resources//plurals//item" space="preserve"/>
  
  <okp:options escapeQuotes="no"/>
- <okp:codeFinder useCodeFinder="yes">#v1
+ <okp:options escapeGT="no"/>
+ <okp:codeFinder useCodeFinder="no">#v1
 count.i=2
 rule0=%(([-0+ #]?)[-0+ #]?)((\d\$)?)(([\d\*]*)(\.[\d\*]*)?)[dioxXucsfeEgGpn]
 rule1=&lt;(/?)\w[^&lt;]*?&gt;
diff --git a/webapp/src/test/java/com/box/l10n/mojito/service/assetExtraction/AssetExtractionServiceTest.java b/webapp/src/test/java/com/box/l10n/mojito/service/assetExtraction/AssetExtractionServiceTest.java
@@ -446,6 +446,50 @@ public void testAndroidStringsWithNotTranslatable() throws Exception {
 
     }
 
+    @Test
+    public void testAndroidStringsWithEscapedHTMLTags() throws Exception {
+
+        Repository repository = repositoryService.createRepository(testIdWatcher.getEntityName("repository"));
+
+        String content = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+                + "<resources>\n"
+                + "  <string name=\"Test\">Hello, %1$s! You have &lt;b>%2$d new messages&lt;/b>.</string>\n"
+                + "</resources>";
+        Asset asset = assetService.createAsset(repository.getId(), content, "path/to/fake/res/strings.xml");
+
+        PollableFuture<Asset> processResult = assetExtractionService.processAsset(asset.getId(), null, PollableTask.INJECT_CURRENT_TASK);
+        Asset processedAsset = processResult.get();
+
+        List<AssetTextUnit> assetTextUnits = assetTextUnitRepository.findByAssetExtraction(processedAsset.getLastSuccessfulAssetExtraction());
+
+        assertEquals("Processing should have extracted 1 text units", 1, assetTextUnits.size());
+        assertEquals("Test", assetTextUnits.get(0).getName());
+        assertEquals("Hello, %1$s! You have <b>%2$d new messages</b>.", assetTextUnits.get(0).getContent());
+
+    }
+
+    @Test
+    public void testAndroidStringsWithCDATA() throws Exception {
+
+        Repository repository = repositoryService.createRepository(testIdWatcher.getEntityName("repository"));
+
+        String content = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+                + "<resources>\n"
+                + "  <string name=\"Test\">Hello, %1$s! You have <![CDATA[<b>%2$d new messages</b>]]>.</string>\n"
+                + "</resources>";
+        Asset asset = assetService.createAsset(repository.getId(), content, "path/to/fake/res/strings.xml");
+
+        PollableFuture<Asset> processResult = assetExtractionService.processAsset(asset.getId(), null, PollableTask.INJECT_CURRENT_TASK);
+        Asset processedAsset = processResult.get();
+
+        List<AssetTextUnit> assetTextUnits = assetTextUnitRepository.findByAssetExtraction(processedAsset.getLastSuccessfulAssetExtraction());
+
+        assertEquals("Processing should have extracted 1 text units", 1, assetTextUnits.size());
+        assertEquals("Test", assetTextUnits.get(0).getName());
+        assertEquals("Hello, %1$s! You have <b>%2$d new messages</b>.", assetTextUnits.get(0).getContent());
+
+    }
+
     @Test
     public void testXliffNoResname() throws Exception {
 
diff --git a/webapp/src/test/java/com/box/l10n/mojito/service/tm/TMServiceTest.java b/webapp/src/test/java/com/box/l10n/mojito/service/tm/TMServiceTest.java
@@ -674,11 +674,66 @@ public void testLocalizeAndroidStringsWithSpecialCharacters() throws Exception {
         logger.debug("localized=\n{}", localizedAsset);
         assertEquals(assetContent, localizedAsset);
     }
-    
+
+    /**
+     * This test is to test {@link XMLEncoder} with escaped HTML and CDATA
+     *
+     * @throws Exception
+     */
+    @Test
+    public void testLocalizeAndroidStringsWithEscapedHTMLAndCDATA() throws Exception {
+
+        Repository repo = repositoryService.createRepository(testIdWatcher.getEntityName("repository"));
+        RepositoryLocale repoLocale;
+        try {
+            repoLocale = repositoryService.addRepositoryLocale(repo, "en-GB");
+        } catch (RepositoryLocaleCreationException e) {
+            throw new RuntimeException(e);
+        }
+
+        String assetContent = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+                + "<resources>\n"
+                + "    <string name=\"welcome_messages0\">Hello, %1$s! You have <b>%2$d new messages</b>.</string>\n"
+                + "    <string name=\"welcome_messages1\">Hello, %1$s! You have &lt;b>%2$d new messages&lt;/b>.</string>\n"
+                + "    <string name=\"welcome_messages2\">Hello, %1$s! You have <![CDATA[<b>%2$d new messages</b>]]>.</string>\n"
+                + "</resources>";
+        String expectedLocalizedAsset = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+                + "<resources>\n"
+                + "    <string name=\"welcome_messages0\">Hello, %1$s! You have &lt;b>%2$d new messages&lt;/b>.</string>\n"
+                + "    <string name=\"welcome_messages1\">Hello, %1$s! You have &lt;b>%2$d new messages&lt;/b>.</string>\n"
+                + "    <string name=\"welcome_messages2\">Hello, %1$s! You have &lt;b>%2$d new messages&lt;/b>.</string>\n"
+                + "</resources>";
+        asset = assetService.createAsset(repo.getId(), assetContent, "res/values/strings.xml");
+        asset = assetRepository.findOne(asset.getId());
+        assetId = asset.getId();
+        tmId = repo.getTm().getId();
+
+        PollableFuture<Asset> assetResult = assetService.addOrUpdateAssetAndProcessIfNeeded(repo.getId(), assetContent, asset.getPath());
+        try {
+            pollableTaskService.waitForPollableTask(assetResult.getPollableTask().getId());
+        } catch (PollableTaskException | InterruptedException e) {
+            throw new RuntimeException(e);
+        }
+        assetResult.get();
+
+        TextUnitSearcherParameters textUnitSearcherParameters = new TextUnitSearcherParameters();
+        textUnitSearcherParameters.setRepositoryIds(repo.getId());
+        textUnitSearcherParameters.setStatusFilter(StatusFilter.FOR_TRANSLATION);
+        List<TextUnitDTO> textUnitDTOs = textUnitSearcher.search(textUnitSearcherParameters);
+        for (TextUnitDTO textUnitDTO : textUnitDTOs) {
+            logger.info("source=[{}]", textUnitDTO.getSource());
+            assertEquals("Hello, %1$s! You have <b>%2$d new messages</b>.", textUnitDTO.getSource());
+        }
+
+        String localizedAsset = tmService.generateLocalized(asset, assetContent, repoLocale, "en-GB");
+        logger.info("localized=\n{}", localizedAsset);
+        assertEquals(expectedLocalizedAsset, localizedAsset);
+    }
+
     /**
      * This test is to test AndroidStrings array with empty item
-     * 
-     * @throws Exception 
+     *
+     * @throws Exception
      */
     @Test
     public void testLocalizeAndroidStringsArrayWithEmptyItem() throws Exception {
