Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8189250

Exception java.util.UnknownFormatFlagsException is never thrown

    Details

    • Subcomponent:
    • CPU:
      x86_64
    • OS:
      os_x

      Description

      FULL PRODUCT VERSION :
      java version "9"
      Java(TM) SE Runtime Environment (build 9+181)
      Java HotSpot(TM) 64-Bit Server VM (build 9+181, mixed mode)

      ADDITIONAL OS VERSION INFORMATION :
      macOS High Sierra v10.13

      A DESCRIPTION OF THE PROBLEM :
      Exception java.util.UnknownFormatFlagsException is never thrown.
      Main class: java.util.Formatter
      Inner class: java.util.Formatter$Flags
      Method: java.util.Formatter$Flags#parse(char c)
      This method is called to check all the flags specified in a message format - if a flag is invalid, then UnknownFormatFlagsException should be thrown.
      The problem is that Formatter#parse(String s) uses a Matcher that includes formatSpecifier:
      private static final String formatSpecifier = "%(\\d+\\$)?([-#+ 0,(\\<]*)?(\\d+)?(\\.\\d+)?([tT])?([a-zA-Z%])".
      Then, it iterates over the supplied string (using Matcher.find(..)) trying to match the format-specifier (%[argument_index$][flags][width][.precision]conversion). Since formatSpecifier only includes valid flags, then any invalid flag will not be detected:
          /**
           * Finds format specifiers in the format string.
           */
          private List<FormatString> parse(String s) {
              ArrayList<FormatString> al = new ArrayList<>();
              Matcher m = fsPattern.matcher(s);
              for (int i = 0, len = s.length(); i < len; ) {
                  if (m.find(i)) {
                      :
                      al.add(new FormatSpecifier(s, m)); /* !!! FormatSpecifiers is the one checking for invalid flags !!! */
                      i = m.end();
                  } else {
                     /* !!! When string "s" contains invalid flags, m.find(i) is always false, since formatSpecifier only includes valid flags. !!! */
                      :
                      checkText(s, i, len); /* !!! This method will throw UnknownFormatConversionException !!! */
                      al.add(new FixedString(s, i, s.length()));
                      break;
                  }
              }
              return al;
          }

      Since the string didn't match, then checkText(..) verifies the string - since it started with a % and didn't match formatSpecifier, Formatter#checkText(String, int, int) will throw UnknownFormatConversionException.

      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      Supply a String containing invalid flags to String.format(..) or using Formatter.

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      Exception UnknownFormatFlagsException should be thrown.
      NOTE:
      ACTUAL -
      Exception UnknownFormatConversionException is thrown.

      ERROR MESSAGES/STACK TRACES THAT OCCUR :
      Exception in thread "main" java.util.UnknownFormatConversionException: Conversion = ')'
      at java.base/java.util.Formatter.checkText(Formatter.java:2640)
      at java.base/java.util.Formatter.parse(Formatter.java:2626)
      at java.base/java.util.Formatter.format(Formatter.java:2563)
      at java.base/java.util.Formatter.format(Formatter.java:2517)
      at java.base/java.lang.String.format(String.java:2747)
      at test.StringTest.main(StringTest.java:6)

      REPRODUCIBILITY :
      This bug can be reproduced always.

      ---------- BEGIN SOURCE ----------
      package test;
      public class StringTest {
          public static void main(String[] _args) {
              String invalid = "This message has an error %)s";
              String.format(invalid);
          }
      }
      ---------- END SOURCE ----------

      CUSTOMER SUBMITTED WORKAROUND :
      This is not critical since UnknownFormatConversionException is thrown in case the format is invalid.
      It is highly recommended to remove java.util.UnknownFormatFlagsException since it will never be used. Also, update related javadoc.

        Activity

        Hide
        psonal Pallavi Sonal added a comment -
        To reproduce the issue, run the attached test case.
        JDK 8u144- Fail
        JDK 9+181 - Fail

        Exception in thread "main" java.util.UnknownFormatConversionException: Conversion = ')'
                at java.base/java.util.Formatter.checkText(Formatter.java:2640)
                at java.base/java.util.Formatter.parse(Formatter.java:2626)
                at java.base/java.util.Formatter.format(Formatter.java:2563)
                at java.base/java.util.Formatter.format(Formatter.java:2517)
                at java.base/java.lang.String.format(String.java:2747)
                at JI9051166.main(JI9051166.java:6)
        Show
        psonal Pallavi Sonal added a comment - To reproduce the issue, run the attached test case. JDK 8u144- Fail JDK 9+181 - Fail Exception in thread "main" java.util.UnknownFormatConversionException: Conversion = ')'         at java.base/java.util.Formatter.checkText(Formatter.java:2640)         at java.base/java.util.Formatter.parse(Formatter.java:2626)         at java.base/java.util.Formatter.format(Formatter.java:2563)         at java.base/java.util.Formatter.format(Formatter.java:2517)         at java.base/java.lang.String.format(String.java:2747)         at JI9051166.main(JI9051166.java:6)
        Hide
        psonal Pallavi Sonal added a comment -
        Additional Information from submitter :
        The default-case in "Formatter$Flags#parse(char)" is never reached due to the Matcher.find(..) condition.
        To keep things aligned with other methods, it should throw java.util.IllegalFormatFlagsException instead:

                // parse those flags which may be provided by users
                private static Flags parse(char c) {
                    switch (c) {
                    case '-': return LEFT_JUSTIFY;
                    case '#': return ALTERNATE;
                    case '+': return PLUS;
                    case ' ': return LEADING_SPACE;
                    case '0': return ZERO_PAD;
                    case ',': return GROUP;
                    case '(': return PARENTHESES;
                    case '<': return PREVIOUS;
                    default:
                        // [REPLACE] throw new UnknownFormatFlagsException(String.valueOf(c));
                        throw new IllegalFormatFlagsException(String.valueOf(c));
                    }
                }

        Rationale: java.util.UnknownFormatFlagsException is only used within "Formatter$Flags#parse(char)" (please, verify) - Removing this exception and reusing "java.util.IllegalFormatFlagsException" makes more sense and decreases by 1 the number of Java9 exceptions (module java.base)
        The default-case should remain - doesn't make any sense adding an additional Flags.INVALID.
        Also, include javadoc/dev-notes on both "Formatter#formatSpecifier" and "Flags#parse(char)" since the characters included in the pattern must be aligned with the switch-cases in "#parse(char)" - [at least on the source-code distributed within the Java9 JDK there aren't any hints about this relationship]

        Full dependency tree (please, re-check):
        Formatter#format(Locale l, String format, Object ... args)
            Formatter#parse(String s)
                Formatter$FormatSpecifier$<constructor>(String s, Matcher m)
                    Formatter$FomatSpecifier#flags(String s, int start, int end)
                        Formatter$Flags#parse(String s, int start, int end)
                            Formatter$Flags#parse(char c)

        Ps. Maybe should be a good idea to enable markdown on these bugreport forms - it should be easier to read....
        Show
        psonal Pallavi Sonal added a comment - Additional Information from submitter : The default-case in "Formatter$Flags#parse(char)" is never reached due to the Matcher.find(..) condition. To keep things aligned with other methods, it should throw java.util.IllegalFormatFlagsException instead:         // parse those flags which may be provided by users         private static Flags parse(char c) {             switch (c) {             case '-': return LEFT_JUSTIFY;             case '#': return ALTERNATE;             case '+': return PLUS;             case ' ': return LEADING_SPACE;             case '0': return ZERO_PAD;             case ',': return GROUP;             case '(': return PARENTHESES;             case '<': return PREVIOUS;             default:                 // [REPLACE] throw new UnknownFormatFlagsException(String.valueOf(c));                 throw new IllegalFormatFlagsException(String.valueOf(c));             }         } Rationale: java.util.UnknownFormatFlagsException is only used within "Formatter$Flags#parse(char)" (please, verify) - Removing this exception and reusing "java.util.IllegalFormatFlagsException" makes more sense and decreases by 1 the number of Java9 exceptions (module java.base) The default-case should remain - doesn't make any sense adding an additional Flags.INVALID. Also, include javadoc/dev-notes on both "Formatter#formatSpecifier" and "Flags#parse(char)" since the characters included in the pattern must be aligned with the switch-cases in "#parse(char)" - [at least on the source-code distributed within the Java9 JDK there aren't any hints about this relationship] Full dependency tree (please, re-check): Formatter#format(Locale l, String format, Object ... args)     Formatter#parse(String s)         Formatter$FormatSpecifier$<constructor>(String s, Matcher m)             Formatter$FomatSpecifier#flags(String s, int start, int end)                 Formatter$Flags#parse(String s, int start, int end)                     Formatter$Flags#parse(char c) Ps. Maybe should be a good idea to enable markdown on these bugreport forms - it should be easier to read....

          People

          • Assignee:
            sherman Xueming Shen
            Reporter:
            webbuggrp Webbug Group
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated: