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

java.time.Duration.parse() parses negative sub-second text as positive value

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: P4
    • Resolution: Duplicate
    • Affects Version/s: 8, 9
    • Fix Version/s: None
    • Component/s: core-libs
    • Labels:
    • Subcomponent:
    • Introduced In Version:
      8
    • CPU:
      x86
    • OS:
      generic

      Description

      FULL PRODUCT VERSION :
      java version "1.8.0_45"
      Java(TM) SE Runtime Environment (build 1.8.0_45-b14)
      Java HotSpot(TM) 64-Bit Server VM (build 25.45-b02, mixed mode)


      ADDITIONAL OS VERSION INFORMATION :
      Darwin zpc0046.local 14.3.0 Darwin Kernel Version 14.3.0: Mon Mar 23 11:59:05 PDT 2015; root:xnu-2782.20.48~5/RELEASE_X86_64 x86_64


      A DESCRIPTION OF THE PROBLEM :
      The java.time.parse(CharSequence) method does not parse negative subsecond values correctly. It returns the absolute value instead. For example, PT-0.1S returns a duration of 100 milliseconds instead of -100.

      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      java.time.Duration.parse("PT-0.1S").toMillis()


      REPRODUCIBILITY :
      This bug can be reproduced always.

      ---------- BEGIN SOURCE ----------
      I wanted to be absolutely sure when I worked around this issue in our code base that the new implementation was correct, so I wrote an exhaustive test that parses over 167K possible values. This is certainly overkill but since I took the time to write it, if it's useful to you, here it is:

      import java.time.Duration;
      import org.junit.Test;

      import static java.lang.Math.abs;
      import static java.util.concurrent.TimeUnit.DAYS;
      import static java.util.concurrent.TimeUnit.HOURS;
      import static java.util.concurrent.TimeUnit.MINUTES;
      import static org.hamcrest.CoreMatchers.equalTo;
      import static org.junit.Assert.assertThat;

      public class WorkaroundDurationParserTest {

          @Test
          public void testBoundaries() { // 167,614 test cases; many admittedly redundant but definitely thorough
              for (boolean negateAll : new boolean[] { false, true }) {
                  for (Long days : new Long[] { null, -100L, -1L, 0L, 1L, 100L }) {
                      for (Long hours : new Long[] { null, -100L, -61L, -60L, -59L, -1L, 0L, 1L, 59L, 60L, 61L, 100L }) {
                          for (Long minutes : new Long[] { null, -100L, -61L, -60L, -59L, -1L, 0L, 1L, 59L, 60L, 61L, 100L }) {
                              for (boolean negateSeconds : new boolean[] { false, true }) { // necessary to test negative zero
                                  for (Long secondsMagnitude : new Long[] { null, 0L, 1L, 59L, 60L, 61L, 100L }) {
                                      for (Long nanosecondsMagnitude : new Long[] {
                                              null, 1L, 10L, 100L, 999L, 1000L, 1001L, 999_999_999L }) {

                                          if (days == null
                                                  && hours == null
                                                  && minutes == null
                                                  && secondsMagnitude == null
                                                  && nanosecondsMagnitude == null) {
                                              continue;
                                          }

                                          Long seconds = (secondsMagnitude == null)
                                                  ? null
                                                  : secondsMagnitude * (negateSeconds ? -1 : 1);
                                          Long nanoseconds = (nanosecondsMagnitude == null)
                                                  ? null
                                                  : nanosecondsMagnitude * (negateSeconds ? -1 : 1);

                                          if (seconds == null && negateSeconds) continue; // a duplicate test case; skip it

                                          long totalSeconds = DAYS.toSeconds(days == null ? 0 : days);
                                          totalSeconds += HOURS.toSeconds(hours == null ? 0 : hours);
                                          totalSeconds += MINUTES.toSeconds(minutes == null ? 0 : minutes);
                                          totalSeconds += (seconds == null ? 0 : seconds);

                                          long nanoAdjustment = (nanoseconds == null ? 0 : nanoseconds);

                                          if (negateAll) {
                                              totalSeconds *= -1;
                                              nanoAdjustment *= -1;
                                          }

                                          String components = String.format("%s %s day(s) %s hour(s) %s minute(s) "
                                                  + "%s second(s) %s nanosecond(s); total = %s seconds + %s nano adjustment",
                                                  negateAll ? "-" : '+', days, hours, minutes, seconds, nanoseconds,
                                                  totalSeconds, nanoAdjustment);

                                          Duration expectedDuration = Duration.ofSeconds(totalSeconds, nanoAdjustment);

                                          StringBuilder sb = new StringBuilder();
                                          if (negateAll) sb.append('-');
                                          sb.append('P');
                                          if (days != null) sb.append(days).append('D');
                                          if (hours != null || minutes != null || seconds != null || nanoseconds != null) {
                                              sb.append('T');
                                              if (hours != null) sb.append(hours).append('H');
                                              if (minutes != null) sb.append(minutes).append('M');
                                              if (seconds != null || nanoseconds != null) {
                                                  if (negateSeconds) sb.append('-');
                                                  sb.append(seconds == null ? 0 : abs(seconds));
                                                  if (nanoseconds != null) {
                                                      // if nanos are non-null, then null/0 seconds are the same test
                                                      if (seconds == null) continue; // so, remove the dupe
                                                      sb.append(String.format(".%09d", abs(nanoseconds)));
                                                      while (sb.charAt(sb.length() - 1) == '0') sb.setLength(sb.length() - 1);
                                                  }
                                                  sb.append('S');
                                              }
                                          }

                                          String expectedString = sb.toString();

                                          Duration actualDuration = WorkaroundDurationParser.parse(expectedString);

                                          assertThat(String.format("parse(`%s`) failed; %s", expectedString, components),
                                                  actualDuration, equalTo(expectedDuration));

                                          // automated verification of toString() is complicated by canonicalization, but
                                          // since we're confident in parse() at this point, we can assert that
                                          // parse(toString(d)) is an identity function and that will give us some assurance.

                                          String actualString = expectedDuration.toString();
                                          Duration identity = WorkaroundDurationParser.parse(actualString);

                                          assertThat(String.format("`%s`.toString produced `%s` which did not parse to the "
                                                          + "expected duration", components, actualString),
                                                  identity, equalTo(expectedDuration));
                                      }
                                  }
                              }
                          }
                      }
                  }
              }
          }

      }

      ---------- END SOURCE ----------

      CUSTOMER SUBMITTED WORKAROUND :
      The only known workaround is to clone the java.time.Duration.parse() method, fix the bug, and use the corrected code. This is only possible when the client code is under the developer's control.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                rriggs Roger Riggs
                Reporter:
                webbuggrp Webbug Group
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: