----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68501/#review207884 -----------------------------------------------------------
I would expect the decoding to be handled by default (by the container?). This filter might end up decoding again, which might result in incorrect result. Please verify this with few usecases, like queryParamName or paramValue having some encoded character. webapp/src/main/java/org/apache/atlas/web/filters/AtlasDecoderFilter.java Lines 44 (patched) <https://reviews.apache.org/r/68501/#comment291497> Given 'configuration' is needed only to initialize 'isDECODING_ENABLED' and 'isDECODING_ENABLED' is not needed in static context, please avoid 'static' and update as below: public class AtlasDecoderFilter implements Filter { private static final Logger LOG = LoggerFactory.getLogger(AtlasDecoderFilter.class); private static final String DECODING = "UTF-8"; private final isUriParamsDecodingEnabled; public AtlasDecoderFilter() { Configuration configuration = null; try { configuration = ApplicationProperties.get(); } catch (AtlasException excp) { LOG.error("Failed to get ApplicationProperties", e); } isUriParamsDecodingEnabled = configuration != null configuration.getBoolean("atlas.rest-decoding.enabled", true); : true? } } webapp/src/main/java/org/apache/atlas/web/filters/AtlasDecoderFilter.java Lines 120 (patched) <https://reviews.apache.org/r/68501/#comment291500> requestURI or requestQueryParam could be null here - see line #87, #98 above. Consider the following alternative impl: public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { boolean isUriDecoded = false; if (isUriDecodingEnabled) { HttpServletRequest httpServletRequest = (HttpServletRequest) request; String requestURI = httpServletRequest.getRequestURI(); String requestQueryParam = httpServletRequest.getQueryString(); if (requestURI != null) { try { String decodedURI = URLDecoder.decode(requestURI, DECODING); if (!requestURI.equals(decodedURI)) { isUriDecoded = true; requestURI = decodedURI; } } catch (UnsupportedEncodingException excp) { LOG.warn("URI decoding failed", excp); } } if (requestQueryParam != null) { try { String decodedQueryParam = URLDecoder.decode(requestQueryParam, DECODING); if (!requestQueryParam.equals(decodedQueryParam)) { isUriDecoded = true; requestQueryParam = decodedQueryParam; } } catch (UnsupportedEncodingException excp) { LOG.warn("URI decoding failed", excp); } } } if (isUriDecoded) { final String dispatchUri; if (requestURI == null) { dispatchUri = requestQueryParam; } else if (requestQueryParam == null) { dispatchUri = requestURI; } else { dispatchUri = requestURI.concat(requestQueryParam); } request.getRequestDispatcher(dispatchUri).forward(request, response); } else { chain.doFilter(request, response); } } - Madhan Neethiraj On Aug. 24, 2018, 1:48 p.m., keval bhatt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68501/ > ----------------------------------------------------------- > > (Updated Aug. 24, 2018, 1:48 p.m.) > > > Review request for atlas, Apoorv Naik, Ashutosh Mestry, Madhan Neethiraj, > Nixon Rodrigues, and Sarath Subramanian. > > > Bugs: ATLAS-2837 > https://issues.apache.org/jira/browse/ATLAS-2837 > > > Repository: atlas > > > Description > ------- > > When we are trying to access > [http://localhost:21000/api/atlas/entities?type%3Dhive_table] then it shows > error. > > ``` > { > error: "Entity type cannot be null", > stackTrace: "java.lang.NullPointerException: Entity type cannot be null at > com.google.common.base.Preconditions.checkNotNull(Preconditions.java:208) at > org.apache.atlas.web.resources.EntityResource.getEntityListByType(EntityResource.java:509) > at > org.apache.atlas.web.resources.EntityResource.getEntity(EntityResource.java:547) > at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at > sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) > at > sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.lang.reflect.Method.invoke(Method.java:498) at > com.sun.jersey.spi.container.JavaMethodInvokerFactory$1.invoke(JavaMethodInvokerFactory.java:60) > at > com.sun.jersey.server.impl.model.method.dispatch.AbstractResourceMethodDispatchProvider$ResponseOutInvoker._dispatch(AbstractResourceMethodDispatchProvider.java:205) > at > com.sun.jersey.server.impl.model.method.dispatch.ResourceJavaMethodDispatcher.dispatch(ResourceJavaMethodDispatc her.java:75) at com.sun.jersey.server.impl.uri.rules.HttpMethodRule.accept(HttpMethodRule.java:302) at com.sun.jersey.server.impl.uri.rules.ResourceClassRule.accept(ResourceClassRule.java:108) at com.sun.jersey.server.impl.uri.rules.RightHandPathRule.accept(RightHandPathRule.java:147) at com.sun.jersey.server.impl.uri.rules.RootResourceClassesRule.accept(RootResourceClassesRule.java:84) at com.sun.jersey.server.impl.application.WebApplicationImpl._handleRequest(WebApplicationImpl.java:1542) at com.sun.jersey.server.impl.application.WebApplicationImpl._handleRequest(WebApplicationImpl.java:1473) at com.sun.jersey.server.impl.application.WebApplicationImpl.handleRequest(WebApplicationImpl.java:1419) at com.sun.jersey.server.impl.application.WebApplicationImpl.handleRequest(WebApplicationImpl.java:1409) at com.sun.jersey.spi.container.servlet.WebComponent.service(WebComponent.java:409) at com.sun.jersey.spi.container.servlet.ServletContainer.service(ServletContainer.java:558) at com.su n.jersey.spi.container.servlet.ServletContainer.service(ServletContainer.java:733) at javax.servlet.http.HttpServlet.service(HttpServlet.java:790) at com.google.inject.servlet.ServletDefinition.doServiceImpl(ServletDefinition.java:287) at com.google.inject.servlet.ServletDefinition.doService(ServletDefinition.java:277) at com.google.inject.servlet.ServletDefinition.service(ServletDefinition.java:182) at com.google.inject.servlet.ManagedServletPipeline.service(ManagedServletPipeline.java:91) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:85) at org.apache.atlas.web.filters.AuditFilter.doFilter(AuditFilter.java:71) at com.google.inject.servlet.FilterChainInvocation.doFilter(FilterChainInvocation.java:82) at com.google.inject.servlet.ManagedFilterPipeline.dispatch(ManagedFilterPipeline.java:119) at com.google.inject.servlet.GuiceFilter$1.call(GuiceFilter.java:133) at com.google.inject.servlet.GuiceFilter$1.call(GuiceFilter.java:130) at com.google .inject.servlet.GuiceFilter$Context.call(GuiceFilter.java:203) at com.google.inject.servlet.GuiceFilter.doFilter(GuiceFilter.java:130) at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652) at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:330) at org.apache.atlas.web.filters.AtlasAuthorizationFilter.doFilter(AtlasAuthorizationFilter.java:154) at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342) at org.springframework.security.web.access.intercept.FilterSecurityInterceptor.invoke(FilterSecurityInterceptor.java:118) at org.springframework.security.web.access.intercept.FilterSecurityInterceptor.doFilter(FilterSecurityInterceptor.java:84) at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342) at org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFi lter.java:113) at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342) at org.springframework.security.web.session.SessionManagementFilter.doFilter(SessionManagementFilter.java:103) at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342) at org.springframework.security.web.authentication.AnonymousAuthenticationFilter.doFilter(AnonymousAuthenticationFilter.java:113) at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342) at org.apache.atlas.web.filters.AtlasCSRFPreventionFilter$ServletFilterHttpInteraction.proceed(AtlasCSRFPreventionFilter.java:232) at org.apache.atlas.web.filters.AtlasCSRFPreventionFilter.handleHttpInteraction(AtlasCSRFPreventionFilter.java:177) at org.apache.atlas.web.filters.AtlasCSRFPreventionFilter.doFilter(AtlasCSRFPreventionFilter.java:187) at org.springframework.security.web.FilterChainProxy$VirtualFilterC hain.doFilter(FilterChainProxy.java:342) at org.apache.atlas.web.filters.AtlasAuthenticationFilter.doFilter(AtlasAuthenticationFilter.java:305) at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342) at org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter.doFilter(SecurityContextHolderAwareRequestFilter.java:54) at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342) at org.springframework.security.web.savedrequest.RequestCacheAwareFilter.doFilter(RequestCacheAwareFilter.java:45) at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342) at org.springframework.security.web.authentication.www.BasicAuthenticationFilter.doFilter(BasicAuthenticationFilter.java:150) at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342) at org.springframework.security.web .authentication.AbstractAuthenticationProcessingFilter.doFilter(AbstractAuthenticationProcessingFilter.java:183) at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342) at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:105) at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342) at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:87) at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:342) at org.springframework.security.web.FilterChainProxy.doFilterInternal(FilterChainProxy.java:192) at org.springframework.security.web.FilterChainProxy.doFilter(FilterChainProxy.java:160) at org.springframework.web.filter.DelegatingFilterProxy.invokeDelegate(DelegatingFilterProxy.java:346) at org.springframework.web.filter.Delega tingFilterProxy.doFilter(DelegatingFilterProxy.java:259) at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1652) at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:585) at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143) at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:577) at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:223) at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1127) at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:515) at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185) at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1061) at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141) at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:97) at org.eclipse.jetty.server.Ser ver.handle(Server.java:499) at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:310) at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:257) at org.eclipse.jetty.io.AbstractConnection$2.run(AbstractConnection.java:540) at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:635) at org.eclipse.jetty.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:555) at java.lang.Thread.run(Thread.java:745) " > } > ``` > > > Diffs > ----- > > webapp/src/main/java/org/apache/atlas/web/filters/AtlasDecoderFilter.java > PRE-CREATION > webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java 82d6f3594 > webapp/src/main/java/org/apache/atlas/web/security/AtlasSecurityConfig.java > 64c95203c > > > Diff: https://reviews.apache.org/r/68501/diff/1/ > > > Testing > ------- > > API test case worked properly > https://builds.apache.org/job/PreCommit-ATLAS-Build-Test/606/consoleFull > > > Thanks, > > keval bhatt > >
