Derby
  1. Derby
  2. DERBY-4796

Missing escape for apostrophes in many messages

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.6.1.0
    • Fix Version/s: 10.7.1.1
    • Component/s: Localization
    • Labels:
      None

      Description

      The properties file format requires that single-quote characters are escaped with an extra single-quote. A number of messages don't follow this rule.

      Take this example from the Italian localization:

      42X34=Esiste un parametro ? nell'elenco di selezione. Ci\u00F2 non \u00E8 consentito.

      Note that there's only a single apostrophe in "nell'elenco" above. When this message is printed, the apostrophe will be omitted:

      $ LC_ALL=it_IT.UTF-8 java -jar derbyrun.jar ij
      Versione ij 10.6
      ij> connect 'jdbc:derby:db;create=true';
      ij> select ? from sys.systables;
      ERRORE 42X34: Esiste un parametro ? nellelenco di selezione. Ciò non è consentito.

      We should go through the message files and add extra apostrophes where they are missing.

      1. engine-messages.diff
        32 kB
        Knut Anders Hatlen
      2. tools-drda-messages.stat
        2 kB
        Knut Anders Hatlen
      3. tools-drda-messages.diff
        241 kB
        Knut Anders Hatlen
      4. tools-drda-messages-2.diff
        245 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Hide
          Knut Anders Hatlen added a comment -

          Committed revision 1033977.

          Show
          Knut Anders Hatlen added a comment - Committed revision 1033977.
          Hide
          Knut Anders Hatlen added a comment -

          Attaching new patch for tools and drda messages (tools-drda-messages-2.diff). The new patch updates the canons for derbyrunjartest and NetworkServerControlAPITest. This was needed because the patch adds a missing space to one of the usage messages in NetworkServerControl.

          All the regression tests ran cleanly with the updated patch.

          Show
          Knut Anders Hatlen added a comment - Attaching new patch for tools and drda messages (tools-drda-messages-2.diff). The new patch updates the canons for derbyrunjartest and NetworkServerControlAPITest. This was needed because the patch adds a missing space to one of the usage messages in NetworkServerControl. All the regression tests ran cleanly with the updated patch.
          Hide
          Knut Anders Hatlen added a comment -

          Attaching a patch to fix the tools and drda messages. I haven't run any tests yet.

          The patch makes the following changes:

          M java/tools/org/apache/derby/iapi/tools/i18n/LocalizedResource.java

          Send all messages through java.text.MessageFormat for orthogonality, also the messages that don't have parameters.

          M build.xml
          A java/build/org/apache/derbyBuild/MessageVetter.java

          Added code to sanity check the message files at build-time. MessageVetter goes through every message and checks (1) that they don't contain lone single-quote characters, and (2) that no exception is raised when constructing a java.text.MessageFormat using the message. Exceptions from (1) were added for three messages in which single-quotes were needed to quote curly braces that should appear literally in the message. For these messages, MessageVetter checks that single-quotes either are doubled (to produce an apostrophe) or placed adjacent to curly braces.

          M java/tools/org/apache/derby/loc/*.properties
          M java/drda/org/apache/derby/loc/drda/*.properties

          Doubled occurrences of lone single-quotes. Also added single-quotes around curly braces where that was needed.

          M java/engine/org/apache/derby/loc/messages_it.properties

          Fixed a malformed parameter marker, noticed by MessageVetter.

          Show
          Knut Anders Hatlen added a comment - Attaching a patch to fix the tools and drda messages. I haven't run any tests yet. The patch makes the following changes: M java/tools/org/apache/derby/iapi/tools/i18n/LocalizedResource.java Send all messages through java.text.MessageFormat for orthogonality, also the messages that don't have parameters. M build.xml A java/build/org/apache/derbyBuild/MessageVetter.java Added code to sanity check the message files at build-time. MessageVetter goes through every message and checks (1) that they don't contain lone single-quote characters, and (2) that no exception is raised when constructing a java.text.MessageFormat using the message. Exceptions from (1) were added for three messages in which single-quotes were needed to quote curly braces that should appear literally in the message. For these messages, MessageVetter checks that single-quotes either are doubled (to produce an apostrophe) or placed adjacent to curly braces. M java/tools/org/apache/derby/loc/*.properties M java/drda/org/apache/derby/loc/drda/*.properties Doubled occurrences of lone single-quotes. Also added single-quotes around curly braces where that was needed. M java/engine/org/apache/derby/loc/messages_it.properties Fixed a malformed parameter marker, noticed by MessageVetter.
          Hide
          Knut Anders Hatlen added a comment -

          All the regression tests ran cleanly with the changes in engine-messages.diff. Committed revision 1029820.

          Show
          Knut Anders Hatlen added a comment - All the regression tests ran cleanly with the changes in engine-messages.diff. Committed revision 1029820.
          Hide
          Knut Anders Hatlen added a comment -

          I'm considering writing a test that would tell us about such problems when updating the messages in the future. Where would such a test go? In a JUnit suite? I'm not all that keen on writing code to get all message files known to the classloader... The easier option is probably to make a check at build time, for which we have some kind of precedence with MessageBundleTest. I think I'll go for the latter option, and if it turns out to slow down the build unacceptably, we can consider moving it to a JUnit suite later.

          Show
          Knut Anders Hatlen added a comment - I'm considering writing a test that would tell us about such problems when updating the messages in the future. Where would such a test go? In a JUnit suite? I'm not all that keen on writing code to get all message files known to the classloader... The easier option is probably to make a check at build time, for which we have some kind of precedence with MessageBundleTest. I think I'll go for the latter option, and if it turns out to slow down the build unacceptably, we can consider moving it to a JUnit suite later.
          Hide
          Dag H. Wanvik added a comment -

          I think the idea of always running it through MessageFormat makes thigns more orthogonal, +1.

          Show
          Dag H. Wanvik added a comment - I think the idea of always running it through MessageFormat makes thigns more orthogonal, +1.
          Hide
          Knut Anders Hatlen added a comment -

          Here's a patch that fixes the problems I found by grepping the engine messages. It simply changes all single single-quotes to double single-quotes, as that seemed to be the correct handling of all the cases I found.

          For the drda and tools messages, the solution won't be that simple. That's because they get the messages through LocalizedResource, which handles messages with arguments in a different way than messages with no arguments. The messages that have arguments will be passed through MessageFormat, and therefore require escaping. The messages with no arguments are taken literally, and therefore shouldn't be escaped.

          I'm leaning towards changing LocalizedResource to always go through MessageFormat (just like MessageService does with the engine messages). That means we would have to escape single-quotes in all tools messages and drda messages, but that sounds easier to maintain than having some messages that should be escaped and some that shouldn't.

          Show
          Knut Anders Hatlen added a comment - Here's a patch that fixes the problems I found by grepping the engine messages. It simply changes all single single-quotes to double single-quotes, as that seemed to be the correct handling of all the cases I found. For the drda and tools messages, the solution won't be that simple. That's because they get the messages through LocalizedResource, which handles messages with arguments in a different way than messages with no arguments. The messages that have arguments will be passed through MessageFormat, and therefore require escaping. The messages with no arguments are taken literally, and therefore shouldn't be escaped. I'm leaning towards changing LocalizedResource to always go through MessageFormat (just like MessageService does with the engine messages). That means we would have to escape single-quotes in all tools messages and drda messages, but that sounds easier to maintain than having some messages that should be escaped and some that shouldn't.
          Hide
          Knut Anders Hatlen added a comment -

          A correction to what the bug description says: The escaping of single-quotes is not required by the properties file format. It is java.text.MessageFormat that requires the quotes to be escaped.

          Show
          Knut Anders Hatlen added a comment - A correction to what the bug description says: The escaping of single-quotes is not required by the properties file format. It is java.text.MessageFormat that requires the quotes to be escaped.

            People

            • Assignee:
              Knut Anders Hatlen
              Reporter:
              Knut Anders Hatlen
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development