Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.0.11
    • Fix Version/s: 1.0.12
    • Component/s: Jsvc
    • Labels:
      None
    • Environment:

      Oracle Java JRE 1.7.0_04
      CentOS Linux 6.3, 64-bit

      Description

      Having upgraded to commons-daemon 1.0.11 from 1.0.10, we are now no longer able to start our service because Java Home cannot be located.

      In our particular use case we do not set JAVA_HOME as an environment variable but instead specify it using the -home parameter on the jsvc command line. However I have tried using an environment variable and the result is the same: the jsvc fails to find the Java Home directory.

      Having looked at the source code, I think the error is due to a recent change to home.c. In particular, the following on lines 215 to 221:

      if (path == NULL || *path == '\0' || *path == '/') {
         log_debug("Home not specified on command line, using environment");
         path = getenv("JAVA_HOME");
         if (path == NULL || *path == '\0' || *path == '/') {
             /* guard against empty JAVA_HOME */
             path = NULL;
         }
      }
      

      The use of '==' to compare strings here is incorrect; the strcmp() function should be used instead. Currently, the code will fail if the first character of the path is '/', which is invariably the case on Linux, UNIX or MacOS systems. I suspect the reason that this has not been detected sooner is because MacOS and Ubuntu/Debian have been special-cased elsewhere to hard-code the expected Java Home location.

      For us this is a blocker so we have reverted to using 1.0.10 until it is fixed. We also have a patch which I will attach to this issue shortly.

      1. patch.txt
        0.7 kB
        Richard Lowe

        Issue Links

          Activity

          Hide
          Mladen Turk added a comment -

          Fixed in the SVN.
          Will be part of next release

          Show
          Mladen Turk added a comment - Fixed in the SVN. Will be part of next release
          Hide
          Richard Lowe added a comment -

          That seems sensible to me. Thanks for addressing this so quickly, it's appreciated.

          Show
          Richard Lowe added a comment - That seems sensible to me. Thanks for addressing this so quickly, it's appreciated.
          Hide
          Mladen Turk added a comment -

          Right, seems we'll be releasing 1.0.12 pretty soon. It's a serious regression.

          • if (path == NULL || *path == '\0' || *path == '/') {
            + if (path == NULL || strcmp(path, "") == 0 || strcmp(path, "/") == 0) {

          Well, strcmp should be used only for the last part. strcmp(path, "") is the same as *path == '\0'

          Show
          Mladen Turk added a comment - Right, seems we'll be releasing 1.0.12 pretty soon. It's a serious regression. if (path == NULL || *path == '\0' || *path == '/') { + if (path == NULL || strcmp(path, "") == 0 || strcmp(path, "/") == 0) { Well, strcmp should be used only for the last part. strcmp(path, "") is the same as *path == '\0'
          Hide
          Richard Lowe added a comment -

          The attached patch.txt fixes this issue by replacing == with strcmp() to compare strings.

          Show
          Richard Lowe added a comment - The attached patch.txt fixes this issue by replacing == with strcmp() to compare strings.

            People

            • Assignee:
              Mladen Turk
              Reporter:
              Richard Lowe
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development