Uploaded image for project: 'Jackrabbit Oak'
  1. Jackrabbit Oak
  2. OAK-3510

Troublesome ExternalIdentityRef.equals(Object) implementation

Attach filesAttach ScreenshotVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 1.3.9, 1.4
    • auth-external
    • None

    Description

      in the light of OAK-3508 i looked at the ExternalIdentifyRef class and found the following implementation of Object.equals(Object):

      public boolean equals(Object o) {
              try {
                  // assuming that we never compare other types of classes
                  return this == o || string.equals(((ExternalIdentityRef) o).string);
              } catch (Exception e) {
                  return false;
              }
          }
      

      since this class is public and exported as part of a public API, i don't think the assumption made in the code is justified. also i would argue that catching Exception is bad style as is exception driven development. in this particular case it was IMHO perfectly trivial to just get rid of the catch clause altogether.

      Attachments

        1. OAK-3510_2.patch
          9 kB
          Angela Schreiber
        2. OAK-3510.patch
          2 kB
          Angela Schreiber

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            angela Angela Schreiber
            angela Angela Schreiber
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment