[ 
https://issues.apache.org/jira/browse/DIRSERVER-1154?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12581219#action_12581219
 ] 

Emmanuel Lecharny commented on DIRSERVER-1154:
----------------------------------------------

I don't think this patch is enough. 

FYI, the way referrals are handled in ADS is far from being perfect, but at 
least, it's fast. The idea is to avoid a complex checking everytime. We are 
caching the referral's DN in order to avoid to check the existence of a 
specific attribute in the entry being manipulated. 

If you want to remove the referral interceptor, you are clearly exposed to NPE 
just because the ServerLdapContext is using this cache (because it's faster to 
do so). 

Hre, the main issue is that we are managing this cache within the 
ReferralInterceptor, while it should be transversal. I don't really like the 
idea of calling an interceptor's method from outside of this interceptor, as an 
interceptor is supposed to isolate its code. 

We will add a new cache system soon, and we will delegate such a verification 
to another subsystem, but this is not exactly a 5 minutes kind of job.

I would suggest that you keep the referral interceptor in place at the moment.



> Declaration and instantiation of refService in ServerLdapContext limits 
> extensibility
> -------------------------------------------------------------------------------------
>
>                 Key: DIRSERVER-1154
>                 URL: https://issues.apache.org/jira/browse/DIRSERVER-1154
>             Project: Directory ApacheDS
>          Issue Type: Bug
>          Components: core
>    Affects Versions: bigbang
>            Reporter: Icky Dude
>             Fix For: bigbang
>
>
> I ran into a problem that I think needs some simple design work or a simple 
> fix.  For my project is is not necessary to handle referrals, so I decided to 
> simply eliminate the ReferralIntercepter from my InterceptorChain.  As soon 
> as I did this, myDirectoryService started crapping on a NullPointerException 
> buried in the bowels of the DefaultSearchHandler (something I definitely 
> don't want to mess with for my project).
> 1) At DefaultSearchHandler.java:357 of  there is an instantiation of a new 
> SearchResponseIterator.  
> 2) The constructor for SearchResponseIterator calls 
> ServerLdapContex.isReferral() at SearchResponseItereator:117
> 3) ServerLdapContext.isReferral() results in a NPE at 
> ServerLdapContext.java:264 unless your DirectoryService's InterceptorChain 
> includes a ReferralInterceptor.  Take a look at the constructor and you'll 
> see why:
> public ServerLdapContext( DirectoryService service, Hashtable<String, Object> 
> env ) throws NamingException
> {
>     super( service, env );
>     refService = ( ( ReferralInterceptor ) service.getInterceptorChain().get( 
> ReferralInterceptor.class.getName() ) );
> }
> Is there any chance that we can simply Check refService for null before it's 
> used ServerLdapContext.isReferral().  If it's refService==null, return false?
> There's also similar a similar problem in PartitionNexusProxy.java:891 and 
> 901.  Here the code checks the chain for null, and returns, but it doesn't 
> check the for null before invoking the interceptor method. 
> Here's the patch:
> $ svn diff ServerLdapContext.java 
> Index: ServerLdapContext.java
> ===================================================================
> --- ServerLdapContext.java      (revision 638966)
> +++ ServerLdapContext.java      (working copy)
> @@ -261,7 +261,11 @@
>       */
>      public boolean isReferral( String name ) throws NamingException
>      {
> -        return refService.isReferral( name );
> +       if( refService == null )
> +        {
> +               return false;
> +        }
> +       return refService.isReferral( name );
>      }
>  
>      /**
> @@ -272,7 +276,11 @@
>       */
>      public boolean isReferral( LdapDN name ) throws NamingException
>      {
> -        return refService.isReferral( name );
> +        if( refService == null )
> +        {
> +               return false;
> +        }
> +       return refService.isReferral( name );
> $ svn diff PartitionNexusProxy.java
> Index: PartitionNexusProxy.java
> ===================================================================
> --- PartitionNexusProxy.java    (revision 638966)
> +++ PartitionNexusProxy.java    (working copy)
> @@ -889,6 +889,10 @@
>      {
>          InterceptorChain chain = service.getInterceptorChain();
>          EventInterceptor interceptor = ( EventInterceptor ) chain.get( 
> EventInterceptor.class.getName() );
> +        if( interceptor == null )
> +        {
> +               return;
> +        }
>          interceptor.addNamingListener( ctx, name, filter, searchControls, 
> namingListener );
>      }
>  
> @@ -901,6 +905,10 @@
>              return;
>          }
>          EventInterceptor interceptor = ( EventInterceptor ) chain.get( 
> EventInterceptor.class.getName() );
> +        if( interceptor == null )
> +        {
> +               return;
> +        }
>          interceptor.removeNamingListener( ctx, namingListener );
>      }
>  }

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to