This is an automated email from the ASF dual-hosted git repository.

ddekany pushed a commit to branch 2.3-gae
in repository https://gitbox.apache.org/repos/asf/freemarker.git


The following commit(s) were added to refs/heads/2.3-gae by this push:
     new bd3907b  FREEMARKER-169: If incomplatible_improvements is set to 
2.3.31 (or higher), when you set the number_format setting to "computer" (or 
you call Environment.getCNumberFormat()), the format now matches the behavior 
of ?c, when formatting infinite (positive and negative), and NaN. Matching the 
behavior of ?c was always the intent, but before this incompatible improvement, 
the computer format always behaved like ?c before incompatible improvements 
2.3.21, where instead of "INF [...]
bd3907b is described below

commit bd3907b5338a4db675e8ecc6c98b06e4073956b1
Author: ddekany <[email protected]>
AuthorDate: Sat Feb 6 11:45:44 2021 +0100

    FREEMARKER-169: If incomplatible_improvements is set to 2.3.31 (or higher), 
when you set the number_format setting to "computer" (or you call 
Environment.getCNumberFormat()), the format now matches the behavior of ?c, 
when formatting infinite (positive and negative), and NaN. Matching the 
behavior of ?c was always the intent, but before this incompatible improvement, 
the computer format always behaved like ?c before incompatible improvements 
2.3.21, where instead of "INF", and "NaN",  [...]
---
 .../freemarker/core/BuiltInsForMultipleTypes.java  |  4 +-
 src/main/java/freemarker/core/Environment.java     | 46 +++++++++++++++++----
 .../core/JavaTemplateNumberFormatFactory.java      | 12 ++++--
 .../java/freemarker/template/Configuration.java    | 11 +++++
 src/manual/en_US/book.xml                          | 19 +++++++++
 .../java/freemarker/core/NumberFormatTest.java     | 47 +++++++++++++++++++++-
 6 files changed, 123 insertions(+), 16 deletions(-)

diff --git a/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java 
b/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java
index a8b1130..fbd528c 100644
--- a/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java
+++ b/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java
@@ -57,7 +57,7 @@ class BuiltInsForMultipleTypes {
 
     static class cBI extends AbstractCBI implements ICIChainMember {
         
-        static class BIBeforeICE2d3d21 extends AbstractCBI {
+        static class BIBeforeICI2d3d21 extends AbstractCBI {
 
             @Override
             protected TemplateModel formatNumber(Environment env, 
TemplateModel model) throws TemplateModelException {
@@ -72,7 +72,7 @@ class BuiltInsForMultipleTypes {
             
         }
         
-        private final BIBeforeICE2d3d21 prevICIObj = new BIBeforeICE2d3d21();
+        private final BIBeforeICI2d3d21 prevICIObj = new BIBeforeICI2d3d21();
 
         @Override
         TemplateModel _eval(Environment env) throws TemplateException {
diff --git a/src/main/java/freemarker/core/Environment.java 
b/src/main/java/freemarker/core/Environment.java
index 13bde79..b6b5b44 100644
--- a/src/main/java/freemarker/core/Environment.java
+++ b/src/main/java/freemarker/core/Environment.java
@@ -72,6 +72,7 @@ import freemarker.template.TemplateScalarModel;
 import freemarker.template.TemplateSequenceModel;
 import freemarker.template.TemplateTransformModel;
 import freemarker.template.TransformControl;
+import freemarker.template.Version;
 import freemarker.template._TemplateAPI;
 import freemarker.template.utility.DateUtil;
 import freemarker.template.utility.DateUtil.DateToISO8601CalendarFactory;
@@ -104,13 +105,24 @@ public final class Environment extends Configurable {
 
     // Do not use this object directly; clone it first! DecimalFormat isn't
     // thread-safe.
-    private static final DecimalFormat C_NUMBER_FORMAT = new DecimalFormat(
+    /** "c" number format as it was before Incompatible Improvements 2.3.21. */
+    private static final DecimalFormat C_NUMBER_FORMAT_ICI_2_3_20 = new 
DecimalFormat(
             "0.################",
             new DecimalFormatSymbols(Locale.US));
+    static {
+        C_NUMBER_FORMAT_ICI_2_3_20.setGroupingUsed(false);
+        C_NUMBER_FORMAT_ICI_2_3_20.setDecimalSeparatorAlwaysShown(false);
+    }
 
+    // Do not use this object directly; clone it first! DecimalFormat isn't
+    // thread-safe.
+    /** "c" number format as it was starting from Incompatible Improvements 
2.3.21. */
+    private static final DecimalFormat C_NUMBER_FORMAT_ICI_2_3_21 = 
(DecimalFormat) C_NUMBER_FORMAT_ICI_2_3_20.clone();
     static {
-        C_NUMBER_FORMAT.setGroupingUsed(false);
-        C_NUMBER_FORMAT.setDecimalSeparatorAlwaysShown(false);
+        DecimalFormatSymbols symbols = 
C_NUMBER_FORMAT_ICI_2_3_21.getDecimalFormatSymbols();
+        symbols.setInfinity("INF");
+        symbols.setNaN("NaN");
+        C_NUMBER_FORMAT_ICI_2_3_21.setDecimalFormatSymbols(symbols);
     }
 
     private final Configuration configuration;
@@ -151,6 +163,7 @@ public final class Environment extends Configurable {
     /** Caches the result of {@link #isSQLDateAndTimeTimeZoneSameAsNormal()}. 
*/
     private Boolean cachedSQLDateAndTimeTimeZoneSameAsNormal;
 
+    @Deprecated
     private NumberFormat cNumberFormat;
 
     /**
@@ -1659,18 +1672,35 @@ public final class Environment extends Configurable {
     }
 
     /**
-     * Returns the {@link NumberFormat} used for the <tt>c</tt> built-in. This 
is always US English
-     * <code>"0.################"</code>, without grouping and without 
superfluous decimal separator.
+     * Returns the {@link NumberFormat} used for the <tt>c</tt> built-in, 
except, if
+     * {@linkplain Configuration#setIncompatibleImprovements(Version) 
Incompatible Improvements} is less than 2.3.31,
+     * this will wrongly give the format that the <tt>c</tt> built-in used 
before Incompatible Improvements 2.3.21.
+     * See more at {@link Configuration#Configuration(Version)}.
      */
     public NumberFormat getCNumberFormat() {
-        // It can't be cached in a static field, because DecimalFormat-s aren't
-        // thread-safe.
+        // Note: DecimalFormat-s aren't thread-safe, so you must clone the 
static field value.
         if (cNumberFormat == null) {
-            cNumberFormat = (DecimalFormat) C_NUMBER_FORMAT.clone();
+            if (configuration.getIncompatibleImprovements().intValue() >= 
_TemplateAPI.VERSION_INT_2_3_31) {
+                cNumberFormat = (DecimalFormat) 
C_NUMBER_FORMAT_ICI_2_3_21.clone();
+            } else {
+                cNumberFormat = (DecimalFormat) 
C_NUMBER_FORMAT_ICI_2_3_20.clone();
+            }
         }
         return cNumberFormat;
     }
 
+    /**
+     * As we have a number format cache that's shared between {@link 
Configuration}-s, if the interpretation of a format
+     * is impacted by Incompatible Improvements, we must change the cache key.
+     */
+    String transformNumberFormatGlobalCacheKey(String keyPart) {
+        if (configuration.getIncompatibleImprovements().intValue() >= 
_TemplateAPI.VERSION_INT_2_3_31
+                && JavaTemplateNumberFormatFactory.COMPUTER.equals(keyPart)) {
+            return "computer\u00002";
+        }
+        return keyPart;
+    }
+
     @Override
     public void setTimeFormat(String timeFormat) {
         String prevTimeFormat = getTimeFormat();
diff --git a/src/main/java/freemarker/core/JavaTemplateNumberFormatFactory.java 
b/src/main/java/freemarker/core/JavaTemplateNumberFormatFactory.java
index 42262ed..8e8a327 100644
--- a/src/main/java/freemarker/core/JavaTemplateNumberFormatFactory.java
+++ b/src/main/java/freemarker/core/JavaTemplateNumberFormatFactory.java
@@ -31,13 +31,15 @@ import freemarker.log.Logger;
 class JavaTemplateNumberFormatFactory extends TemplateNumberFormatFactory {
     
     static final JavaTemplateNumberFormatFactory INSTANCE = new 
JavaTemplateNumberFormatFactory();
-    
+
+    static final String COMPUTER = "computer";
+
     private static final Logger LOG = Logger.getLogger("freemarker.runtime");
 
     private static final ConcurrentHashMap<CacheKey, NumberFormat> 
GLOBAL_FORMAT_CACHE
             = new ConcurrentHashMap<>();
     private static final int LEAK_ALERT_NUMBER_FORMAT_CACHE_SIZE = 1024;
-    
+
     private JavaTemplateNumberFormatFactory() {
         // Not meant to be instantiated
     }
@@ -45,7 +47,9 @@ class JavaTemplateNumberFormatFactory extends 
TemplateNumberFormatFactory {
     @Override
     public TemplateNumberFormat get(String params, Locale locale, Environment 
env)
             throws InvalidFormatParametersException {
-        CacheKey cacheKey = new CacheKey(params, locale);
+        CacheKey cacheKey = new CacheKey(
+                env != null ? env.transformNumberFormatGlobalCacheKey(params) 
: params,
+                locale);
         NumberFormat jFormat = GLOBAL_FORMAT_CACHE.get(cacheKey);
         if (jFormat == null) {
             if ("number".equals(params)) {
@@ -54,7 +58,7 @@ class JavaTemplateNumberFormatFactory extends 
TemplateNumberFormatFactory {
                 jFormat = NumberFormat.getCurrencyInstance(locale);
             } else if ("percent".equals(params)) {
                 jFormat = NumberFormat.getPercentInstance(locale);
-            } else if ("computer".equals(params)) {
+            } else if (COMPUTER.equals(params)) {
                 jFormat = env.getCNumberFormat();
             } else {
                 try {
diff --git a/src/main/java/freemarker/template/Configuration.java 
b/src/main/java/freemarker/template/Configuration.java
index bd7585a..eb189ee 100644
--- a/src/main/java/freemarker/template/Configuration.java
+++ b/src/main/java/freemarker/template/Configuration.java
@@ -931,6 +931,17 @@ public class Configuration extends Configurable implements 
Cloneable, ParserConf
      *           {@code <#if x[0]>} works correctly without this fix as well. 
      *     </ul>
      *   </li>
+     *   <li><p>
+     *     2.3.31 (or higher):
+     *     <ul>
+     *       <li><p>When you set the {@code number_format} setting to {@code 
"computer"} (or you call
+     *       {@link Environment#getCNumberFormat()}), the format now matches 
the behavior of {@code ?c}, when formatting
+     *       infinite (positive and negative), and NaN. Matching the behavior 
of {@code ?c} was always the intent,
+     *       but before this incompatible improvement, the {@code "computer"} 
format always behaved like {@code ?c}
+     *       before Incompatible Improvements 2.3.21, where instead of INF, 
and NaN, the results used unicode characters
+     *       U+221E, and U+FFFD.
+     *     </ul>
+     *   </li>
      * </ul>
      * 
      * @throws IllegalArgumentException
diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index 2818aab..048b8f8 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -29411,6 +29411,25 @@ TemplateModel x = env.getVariable("x");  // get 
variable x</programlisting>
               work for all JSON-s, but can be a security problem. <link
               linkend="ref_builtin_eval_json">See more here...</link></para>
             </listitem>
+
+            <listitem>
+              <para><link
+              
xlink:href="https://issues.apache.org/jira/projects/FREEMARKER/issues/FREEMARKER-169";>FREEMARKER-169</link>:
+              If <link
+              
linkend="pgui_config_incompatible_improvements_how_to_set"><literal>incomplatible_improvements</literal></link>
+              is set to 2.3.31 (or higher), when you set the
+              <literal>number_format</literal> setting to
+              <literal>computer</literal> (or you call
+              <literal>Environment.getCNumberFormat()</literal>), the format
+              now matches the behavior of <literal>?c</literal>, when
+              formatting infinite (positive and negative), and NaN. Matching
+              the behavior of <literal>?c</literal> was always the intent, but
+              before this incompatible improvement, the
+              <literal>computer</literal> format always behaved like
+              <literal>?c</literal> before incompatible improvements 2.3.21,
+              where instead of <quote>INF</quote>, and <quote>NaN</quote>, the
+              results used Unicode characters U+221E, and U+FFFD.</para>
+            </listitem>
           </itemizedlist>
         </section>
 
diff --git a/src/test/java/freemarker/core/NumberFormatTest.java 
b/src/test/java/freemarker/core/NumberFormatTest.java
index 29920c4..184fc21 100644
--- a/src/test/java/freemarker/core/NumberFormatTest.java
+++ b/src/test/java/freemarker/core/NumberFormatTest.java
@@ -44,11 +44,13 @@ import freemarker.template.TemplateException;
 import freemarker.template.TemplateModel;
 import freemarker.template.TemplateModelException;
 import freemarker.template.TemplateNumberModel;
+import freemarker.template.Version;
 import freemarker.test.TemplateTest;
+import net.jcip.annotations.Immutable;
 
 @SuppressWarnings("boxing")
 public class NumberFormatTest extends TemplateTest {
-    
+
     @Before
     public void setup() {
         Configuration cfg = getConfiguration();
@@ -313,7 +315,48 @@ public class NumberFormatTest extends TemplateTest {
             assertOutput("${0.0000123?string.@printfG}", "1.23000E-05");
         }
     }
-    
+
+    @Test
+    public void testCFormatOfSpecialNumbers() throws IOException, 
TemplateException {
+        addToDataModel("pInf", Double.POSITIVE_INFINITY);
+        addToDataModel("nInf", Double.NEGATIVE_INFINITY);
+        addToDataModel("nan", Double.NaN);
+
+        Configuration cfg = getConfiguration();
+        for (Version ici : new Version[] {
+                Configuration.VERSION_2_3_20,
+                Configuration.VERSION_2_3_21, Configuration.VERSION_2_3_30,
+                Configuration.VERSION_2_3_31 } ) {
+            cfg.setIncompatibleImprovements(ici);
+
+            boolean cBuiltInBroken = ici.intValue() < 
Configuration.VERSION_2_3_21.intValue();
+            boolean cNumberFormatBroken = ici.intValue() < 
Configuration.VERSION_2_3_31.intValue();
+
+            String humanAudienceOutput = "\u221e -\u221e \ufffd";
+            String computerAudienceOutput = "INF -INF NaN";
+
+            assertOutput(
+                    "${pInf?c} ${nInf?c} ${nan?c}",
+                    cBuiltInBroken ? humanAudienceOutput : 
computerAudienceOutput);
+
+            assertOutput(
+                    "<#setting numberFormat='computer'>${pInf} ${nInf} ${nan}",
+                    cNumberFormatBroken ? humanAudienceOutput : 
computerAudienceOutput);
+
+            assertOutput(
+                    "${pInf} ${nInf} ${nan}",
+                    humanAudienceOutput);
+
+            Environment env = new Template(null, "", cfg)
+                    .createProcessingEnvironment(null, null);
+            assertEquals(
+                    cNumberFormatBroken ? humanAudienceOutput : 
computerAudienceOutput,
+                    env.getCNumberFormat().format(Double.POSITIVE_INFINITY)
+                            + " " + 
env.getCNumberFormat().format(Double.NEGATIVE_INFINITY)
+                            + " " + env.getCNumberFormat().format(Double.NaN));
+        }
+    }
+
     private static class MutableTemplateNumberModel implements 
TemplateNumberModel {
         
         private Number number;

Reply via email to