You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-issues@jackrabbit.apache.org by "angela (JIRA)" <ji...@apache.org> on 2015/10/15 12:23:05 UTC

[jira] [Commented] (OAK-3510) Troublesome ExternalIdentityRef.equals(Object) implementation

    [ https://issues.apache.org/jira/browse/OAK-3510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14958686#comment-14958686 ] 

angela commented on OAK-3510:
-----------------------------

uh... i wasn't aware that 1.3.8 was already cut as it still appeared in the list of unreleased versions. i guess this issue will only be in 1.3.9

> Troublesome ExternalIdentityRef.equals(Object) implementation
> -------------------------------------------------------------
>
>                 Key: OAK-3510
>                 URL: https://issues.apache.org/jira/browse/OAK-3510
>             Project: Jackrabbit Oak
>          Issue Type: Bug
>          Components: auth-external
>            Reporter: angela
>            Assignee: angela
>             Fix For: 1.3.8
>
>         Attachments: OAK-3510.patch, OAK-3510_2.patch
>
>
> in the light of OAK-3508 i looked at the {{ExternalIdentifyRef}} class and found the following implementation of {{Object.equals(Object)}}:
> {code}
> 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;
>         }
>     }
> {code}
> 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.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)