glassfish
  1. glassfish
  2. GLASSFISH-3967

Method RegStoreFileParser.getPersistedEntries() is not thread safe

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 9.0pe
    • Fix Version/s: future release
    • Component/s: security
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      3,967

      Description

      The method
      com.sun.enterprise.security.jmac.config.RegStoreFileParser.getPersistedEntries()
      is not thread safe. It has two problems: (1) it accesses the field entries
      without synchronizing on confFile, and (2) it should return a copy of the list,
      not a pointer to the list, because you cannot trust the caller of the method to
      access the list with the correct synchronization.

      The method should be written as

      List<EntryInfo> getPersistedEntries() {
      synchronized (confFile)

      { return Collections.unmodifiableList(new ArrayList<EntryInfo>(entries)); }

      }

      This version returns an unmodifiable copy of the list of entries. Technically
      you just need to return a copy, but there isn't any good reason to allow the
      caller to modify the copy, it will likely just cause problems.

      If you really don't want to return a copy, you can instead return a wrapped
      version of the list that is properly protected:

      List<EntryInfo> getPersistedEntries() {
      synchronized (confFile)

      { return Collections.unmodifiableList(Collections.synchronizedList(entries, confFile)); }

      }

      This version avoids copying, prevents the caller from modifying the list, and
      prevents the caller from viewing the list while the list is being modified
      inside RegStoreFileParser.

      I think the copying solution is less likely to cause future issues, but I can
      understand if you want to avoiding copying. I don't know enough about how often
      or even when this method is called to make a better recommendation.

        Activity

        Hide
        kumarjayanti added a comment -

        started

        Show
        kumarjayanti added a comment - started
        Hide
        harpreet added a comment -

        assigning to Kumar

        Show
        harpreet added a comment - assigning to Kumar
        Hide
        harpreet added a comment -

        Based on input from Kumar - issue not critical to v2.1. Marking it for next release.

        Show
        harpreet added a comment - Based on input from Kumar - issue not critical to v2.1. Marking it for next release.
        Hide
        sanandal added a comment -

        "Reclassifying as P4 because this issue is not deemed "must fix" for this v2.1
        release whose primary release driver is SailFin.
        This issue will be scrubbed after this release and will be given the right
        priority for the next release."

        Show
        sanandal added a comment - "Reclassifying as P4 because this issue is not deemed "must fix" for this v2.1 release whose primary release driver is SailFin. This issue will be scrubbed after this release and will be given the right priority for the next release."
        Hide
        kumarjayanti added a comment -

        marking for 3.2

        Show
        kumarjayanti added a comment - marking for 3.2

          People

          • Assignee:
            JeffTancill
            Reporter:
            aarongreenhouse
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated: