Repository: atlas
Updated Branches:
  refs/heads/master 05514255c -> 657e0f127


ATLAS-2174: Query length validations and path normalization

Signed-off-by: apoorvnaik <[email protected]>


Project: http://git-wip-us.apache.org/repos/asf/atlas/repo
Commit: http://git-wip-us.apache.org/repos/asf/atlas/commit/657e0f12
Tree: http://git-wip-us.apache.org/repos/asf/atlas/tree/657e0f12
Diff: http://git-wip-us.apache.org/repos/asf/atlas/diff/657e0f12

Branch: refs/heads/master
Commit: 657e0f1272d1361dacee61f3bf328f8a3185cfab
Parents: 0551425
Author: nixonrodrigues <[email protected]>
Authored: Mon Oct 2 09:03:50 2017 -0700
Committer: apoorvnaik <[email protected]>
Committed: Mon Oct 2 09:06:14 2017 -0700

----------------------------------------------------------------------
 .../main/java/org/apache/atlas/AtlasClient.java |  6 ++--
 .../java/org/apache/atlas/AtlasClientTest.java  |  1 +
 .../java/org/apache/atlas/AtlasBaseClient.java  | 21 ++++++++-----
 .../org/apache/atlas/AtlasServiceException.java |  2 +-
 .../org/apache/atlas/repository/Constants.java  |  3 ++
 .../java/org/apache/atlas/AtlasErrorCode.java   |  1 +
 .../notification/NotificationHookConsumer.java  |  8 ++---
 .../atlas/web/resources/EntityResource.java     |  2 +-
 .../apache/atlas/web/rest/DiscoveryREST.java    | 32 +++++++++++++++-----
 9 files changed, 51 insertions(+), 25 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/atlas/blob/657e0f12/client/client-v1/src/main/java/org/apache/atlas/AtlasClient.java
----------------------------------------------------------------------
diff --git a/client/client-v1/src/main/java/org/apache/atlas/AtlasClient.java 
b/client/client-v1/src/main/java/org/apache/atlas/AtlasClient.java
index 6d0b83d..8bbc89b 100644
--- a/client/client-v1/src/main/java/org/apache/atlas/AtlasClient.java
+++ b/client/client-v1/src/main/java/org/apache/atlas/AtlasClient.java
@@ -356,7 +356,7 @@ public class AtlasClient extends AtlasBaseClient {
         JSONObject response = callAPIWithRetries(api, null, new 
ResourceCreator() {
             @Override
             public WebResource createResource() {
-                WebResource resource = getResource(api.getPath());
+                WebResource resource = getResource(api.getNormalizedPath());
                 resource = resource.queryParam(TYPE, category.name());
                 return resource;
             }
@@ -924,12 +924,12 @@ public class AtlasClient extends AtlasBaseClient {
 
     @VisibleForTesting
     public WebResource getResource(API api, String... params) {
-        return getResource(api.getPath(), params);
+        return getResource(api.getNormalizedPath(), params);
     }
 
     @VisibleForTesting
     public WebResource getResource(API_V1 apiV1, String... params) {
-        return getResource(apiV1.getPath(), params);
+        return getResource(apiV1.getNormalizedPath(), params);
     }
 
     @VisibleForTesting

http://git-wip-us.apache.org/repos/asf/atlas/blob/657e0f12/client/client-v1/src/test/java/org/apache/atlas/AtlasClientTest.java
----------------------------------------------------------------------
diff --git 
a/client/client-v1/src/test/java/org/apache/atlas/AtlasClientTest.java 
b/client/client-v1/src/test/java/org/apache/atlas/AtlasClientTest.java
index 1da6b45..e327666 100644
--- a/client/client-v1/src/test/java/org/apache/atlas/AtlasClientTest.java
+++ b/client/client-v1/src/test/java/org/apache/atlas/AtlasClientTest.java
@@ -108,6 +108,7 @@ public class AtlasClientTest {
 
     private WebResource.Builder setupBuilder(AtlasClient.API_V1 api, 
WebResource webResource) {
         when(webResource.path(api.getPath())).thenReturn(service);
+        when(webResource.path(api.getNormalizedPath())).thenReturn(service);
         return getBuilder(service);
     }
 

http://git-wip-us.apache.org/repos/asf/atlas/blob/657e0f12/client/common/src/main/java/org/apache/atlas/AtlasBaseClient.java
----------------------------------------------------------------------
diff --git a/client/common/src/main/java/org/apache/atlas/AtlasBaseClient.java 
b/client/common/src/main/java/org/apache/atlas/AtlasBaseClient.java
index 5e1d101..1616029 100644
--- a/client/common/src/main/java/org/apache/atlas/AtlasBaseClient.java
+++ b/client/common/src/main/java/org/apache/atlas/AtlasBaseClient.java
@@ -46,6 +46,7 @@ import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriBuilder;
 import java.io.IOException;
 import java.net.ConnectException;
+import java.nio.file.Paths;
 import java.util.List;
 import java.util.Map;
 
@@ -151,7 +152,7 @@ public abstract class AtlasBaseClient {
     }
 
     public boolean isServerReady() throws AtlasServiceException {
-        WebResource resource   = getResource(API_VERSION.getPath());
+        WebResource resource   = getResource(API_VERSION.getNormalizedPath());
         try {
             callAPIWithResource(API_VERSION, resource, null, JSONObject.class);
             return true;
@@ -175,7 +176,7 @@ public abstract class AtlasBaseClient {
      */
     public String getAdminStatus() throws AtlasServiceException {
         String      result    = AtlasBaseClient.UNKNOWN_STATUS;
-        WebResource resource  = getResource(service, API_STATUS.getPath());
+        WebResource resource  = getResource(service, 
API_STATUS.getNormalizedPath());
         JSONObject  response  = callAPIWithResource(API_STATUS, resource, 
null, JSONObject.class);
         try {
             result = response.getString("Status");
@@ -315,7 +316,7 @@ public abstract class AtlasBaseClient {
         int i = 0;
         do {
             if (LOG.isDebugEnabled()) {
-                LOG.debug("Calling API [ {} : {} ] {}", api.getMethod(), 
api.getPath(), requestObject != null ? "<== " + requestObject : "");
+                LOG.debug("Calling API [ {} : {} ] {}", api.getMethod(), 
api.getNormalizedPath(), requestObject != null ? "<== " + requestObject : "");
             }
 
             WebResource.Builder requestBuilder = resource.getRequestBuilder();
@@ -375,7 +376,7 @@ public abstract class AtlasBaseClient {
     }
 
     protected WebResource getResource(API api, MultivaluedMap<String, String> 
queryParams, String... pathParams) {
-        WebResource resource = service.path(api.getPath());
+        WebResource resource = service.path(api.getNormalizedPath());
         resource = appendPathParams(resource, pathParams);
         resource = appendQueryParams(queryParams, resource);
         return resource;
@@ -447,7 +448,7 @@ public abstract class AtlasBaseClient {
                 if (i == (getNumberOfRetries() - 1)) {
                     throw che;
                 }
-                LOG.warn("Handled exception in calling api {}", api.getPath(), 
che);
+                LOG.warn("Handled exception in calling api {}", 
api.getNormalizedPath(), che);
                 LOG.warn("Exception's cause: {}", che.getCause().getClass());
                 handleClientHandlerException(che);
             }
@@ -515,13 +516,13 @@ public abstract class AtlasBaseClient {
 
     // Modify URL to include the path params
     private WebResource getResource(WebResource service, API api, String... 
pathParams) {
-        WebResource resource = service.path(api.getPath());
+        WebResource resource = service.path(api.getNormalizedPath());
         resource = appendPathParams(resource, pathParams);
         return resource;
     }
 
     private WebResource getResource(API api, String queryParamKey, 
List<String> queryParamValues) {
-        WebResource resource = service.path(api.getPath());
+        WebResource resource = service.path(api.getNormalizedPath());
         for (String queryParamValue : queryParamValues) {
             if (StringUtils.isNotBlank(queryParamKey) && 
StringUtils.isNotBlank(queryParamValue)) {
                 resource = resource.queryParam(queryParamKey, queryParamValue);
@@ -541,7 +542,7 @@ public abstract class AtlasBaseClient {
 
     // Modify URL to include the query params
     private WebResource getResource(WebResource service, API api, 
MultivaluedMap<String, String> queryParams) {
-        WebResource resource = service.path(api.getPath());
+        WebResource resource = service.path(api.getNormalizedPath());
         resource = appendQueryParams(queryParams, resource);
         return resource;
     }
@@ -578,6 +579,10 @@ public abstract class AtlasBaseClient {
             return path;
         }
 
+        public String getNormalizedPath() {
+            return Paths.get(path).normalize().toString();
+        }
+
         public Response.Status getExpectedStatus() {
             return status;
         }

http://git-wip-us.apache.org/repos/asf/atlas/blob/657e0f12/client/common/src/main/java/org/apache/atlas/AtlasServiceException.java
----------------------------------------------------------------------
diff --git 
a/client/common/src/main/java/org/apache/atlas/AtlasServiceException.java 
b/client/common/src/main/java/org/apache/atlas/AtlasServiceException.java
index 83f4f8d..33d0a21 100755
--- a/client/common/src/main/java/org/apache/atlas/AtlasServiceException.java
+++ b/client/common/src/main/java/org/apache/atlas/AtlasServiceException.java
@@ -28,7 +28,7 @@ public class AtlasServiceException extends Exception {
     private ClientResponse.Status status;
 
     public AtlasServiceException(AtlasBaseClient.API api, Exception e) {
-        super("Metadata service API " + api.getMethod() + " : " + 
api.getPath() + " failed", e);
+        super("Metadata service API " + api.getMethod() + " : " + 
api.getNormalizedPath() + " failed", e);
     }
 
     public AtlasServiceException(AtlasBaseClient.API api, 
WebApplicationException e) throws JSONException {

http://git-wip-us.apache.org/repos/asf/atlas/blob/657e0f12/common/src/main/java/org/apache/atlas/repository/Constants.java
----------------------------------------------------------------------
diff --git a/common/src/main/java/org/apache/atlas/repository/Constants.java 
b/common/src/main/java/org/apache/atlas/repository/Constants.java
index d5283c1..5475514 100644
--- a/common/src/main/java/org/apache/atlas/repository/Constants.java
+++ b/common/src/main/java/org/apache/atlas/repository/Constants.java
@@ -101,6 +101,9 @@ public final class Constants {
     public static final String INDEX_SEARCH_TYPES_MAX_QUERY_STR_LENGTH = 
"atlas.graph.index.search.types.max-query-str-length";
     public static final String INDEX_SEARCH_TAGS_MAX_QUERY_STR_LENGTH  = 
"atlas.graph.index.search.tags.max-query-str-length";
 
+    public static final String MAX_FULLTEXT_QUERY_STR_LENGTH  = 
"atlas.graph.fulltext-max-query-str-length";
+    public static final String MAX_DSL_QUERY_STR_LENGTH  = 
"atlas.graph.dsl-max-query-str-length";
+
     private Constants() {
     }
 

http://git-wip-us.apache.org/repos/asf/atlas/blob/657e0f12/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java
----------------------------------------------------------------------
diff --git a/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java 
b/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java
index bf09806..fb741f8 100644
--- a/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java
+++ b/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java
@@ -102,6 +102,7 @@ public enum AtlasErrorCode {
     INVALID_ENTITY_FOR_CLASSIFICATION (400, "ATLAS-400-00-055", "Entity 
(guid=‘{0}‘,typename=‘{1}‘) cannot be classified by Classification 
‘{2}‘, because ‘{1}‘ is not in the ClassificationDef's restrictions."),
     SAVED_SEARCH_CHANGE_USER(400, "ATLAS-400-00-056", "saved-search {0} can 
not be moved from user {1} to {2}"),
     INVALID_QUERY_PARAM_LENGTH(400, "ATLAS-400-00-057" , "Length of query 
param {0} exceeds the limit"),
+    INVALID_QUERY_LENGTH(400, "ATLAS-400-00-057" , "Invalid query length, 
update {0} to change the limit" ),
 
     // All Not found enums go here
     TYPE_NAME_NOT_FOUND(404, "ATLAS-404-00-001", "Given typename {0} was 
invalid"),

http://git-wip-us.apache.org/repos/asf/atlas/blob/657e0f12/webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
----------------------------------------------------------------------
diff --git 
a/webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
 
b/webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
index 5fd5087..6790512 100644
--- 
a/webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
+++ 
b/webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java
@@ -355,7 +355,7 @@ public class NotificationHookConsumer implements Service, 
ActiveStateChangeHandl
 
                                 if (numRetries == 0) { // audit only on the 
first attempt
                                     AtlasBaseClient.API api = 
AtlasClient.API_V1.CREATE_ENTITY;
-                                    audit(messageUser, api.getMethod(), 
api.getPath());
+                                    audit(messageUser, api.getMethod(), 
api.getNormalizedPath());
                                 }
 
                                 entities = 
instanceConverter.toAtlasEntities(createRequest.getEntities());
@@ -369,7 +369,7 @@ public class NotificationHookConsumer implements Service, 
ActiveStateChangeHandl
                                 if (numRetries == 0) { // audit only on the 
first attempt
                                     AtlasBaseClient.API api = 
UPDATE_ENTITY_BY_ATTRIBUTE;
                                     audit(messageUser, api.getMethod(),
-                                          String.format(api.getPath(), 
partialUpdateRequest.getTypeName()));
+                                          
String.format(api.getNormalizedPath(), partialUpdateRequest.getTypeName()));
                                 }
 
                                 Referenceable referenceable = 
partialUpdateRequest.getEntity();
@@ -394,7 +394,7 @@ public class NotificationHookConsumer implements Service, 
ActiveStateChangeHandl
                                 if (numRetries == 0) { // audit only on the 
first attempt
                                     AtlasBaseClient.API api = 
DELETE_ENTITY_BY_ATTRIBUTE;
                                     audit(messageUser, api.getMethod(),
-                                          String.format(api.getPath(), 
deleteRequest.getTypeName()));
+                                          
String.format(api.getNormalizedPath(), deleteRequest.getTypeName()));
                                 }
 
                                 try {
@@ -413,7 +413,7 @@ public class NotificationHookConsumer implements Service, 
ActiveStateChangeHandl
 
                                 if (numRetries == 0) { // audit only on the 
first attempt
                                     AtlasBaseClient.API api = UPDATE_ENTITY;
-                                    audit(messageUser, api.getMethod(), 
api.getPath());
+                                    audit(messageUser, api.getMethod(), 
api.getNormalizedPath());
                                 }
 
                                 entities = 
instanceConverter.toAtlasEntities(updateRequest.getEntities());

http://git-wip-us.apache.org/repos/asf/atlas/blob/657e0f12/webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java
----------------------------------------------------------------------
diff --git 
a/webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java 
b/webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java
index 67c9b27..8b56507 100755
--- a/webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java
+++ b/webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java
@@ -215,7 +215,7 @@ public class EntityResource {
             UriBuilder ub = uriInfo.getAbsolutePathBuilder();
             locationURI = CollectionUtils.isEmpty(guids) ? null : 
ub.path(guids.get(0)).build();
         } else {
-            String uriPath = AtlasClient.API_V1.GET_ENTITY.getPath();
+            String uriPath = AtlasClient.API_V1.GET_ENTITY.getNormalizedPath();
             locationURI = guids.isEmpty() ? null : UriBuilder
                 .fromPath(AtlasConstants.DEFAULT_ATLAS_REST_ADDRESS)
                 .path(uriPath).path(guids.get(0)).build();

http://git-wip-us.apache.org/repos/asf/atlas/blob/657e0f12/webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java
----------------------------------------------------------------------
diff --git a/webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java 
b/webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java
index 1780c67..ee68d63 100644
--- a/webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java
+++ b/webapp/src/main/java/org/apache/atlas/web/rest/DiscoveryREST.java
@@ -17,7 +17,6 @@
  */
 package org.apache.atlas.web.rest;
 
-import org.apache.atlas.AtlasConfiguration;
 import org.apache.atlas.AtlasErrorCode;
 import org.apache.atlas.SortOrder;
 import org.apache.atlas.discovery.AtlasDiscoveryService;
@@ -25,9 +24,11 @@ import org.apache.atlas.exception.AtlasBaseException;
 import org.apache.atlas.model.discovery.AtlasSearchResult;
 import org.apache.atlas.model.discovery.SearchParameters;
 import org.apache.atlas.model.profile.AtlasUserSavedSearch;
+import org.apache.atlas.repository.Constants;
 import org.apache.atlas.utils.AtlasPerfTracer;
 import org.apache.atlas.web.util.Servlets;
 import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.configuration.Configuration;
 import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.springframework.stereotype.Service;
@@ -58,13 +59,17 @@ public class DiscoveryREST {
     private static final Logger PERF_LOG = 
AtlasPerfTracer.getPerfLogger("rest.DiscoveryREST");
 
     @Context
-    private HttpServletRequest httpServletRequest;
+    private       HttpServletRequest httpServletRequest;
+    private final int                maxFullTextQueryLength;
+    private final int                maxDslQueryLength;
 
     private final AtlasDiscoveryService atlasDiscoveryService;
 
     @Inject
-    public DiscoveryREST(AtlasDiscoveryService discoveryService) {
-        this.atlasDiscoveryService = discoveryService;
+    public DiscoveryREST(AtlasDiscoveryService atlasDiscoveryService, 
Configuration configuration) {
+        this.atlasDiscoveryService = atlasDiscoveryService;
+        maxFullTextQueryLength = 
configuration.getInt(Constants.MAX_FULLTEXT_QUERY_STR_LENGTH, 4096);
+        maxDslQueryLength = 
configuration.getInt(Constants.MAX_DSL_QUERY_STR_LENGTH, 4096);
     }
 
     /**
@@ -90,10 +95,13 @@ public class DiscoveryREST {
                                             @QueryParam("classification") 
String classification,
                                             @QueryParam("limit")          int  
  limit,
                                             @QueryParam("offset")         int  
  offset) throws AtlasBaseException {
-        Servlets.validateQueryParamLength("query", query);
         Servlets.validateQueryParamLength("typeName", typeName);
         Servlets.validateQueryParamLength("classification", classification);
 
+        if (StringUtils.isNotEmpty(query) && query.length() > 
maxDslQueryLength) {
+            throw new AtlasBaseException(AtlasErrorCode.INVALID_QUERY_LENGTH, 
Constants.MAX_DSL_QUERY_STR_LENGTH);
+        }
+
         AtlasPerfTracer perf = null;
 
         try {
@@ -132,7 +140,10 @@ public class DiscoveryREST {
                                                  
@QueryParam("excludeDeletedEntities") boolean excludeDeletedEntities,
                                                  @QueryParam("limit")          
        int     limit,
                                                  @QueryParam("offset")         
        int     offset) throws AtlasBaseException {
-        Servlets.validateQueryParamLength("query", query);
+        // Validate FullText query for max allowed length
+        if(StringUtils.isNotEmpty(query) && query.length() > 
maxFullTextQueryLength){
+            throw new AtlasBaseException(AtlasErrorCode.INVALID_QUERY_LENGTH, 
Constants.MAX_FULLTEXT_QUERY_STR_LENGTH );
+        }
 
         AtlasPerfTracer perf = null;
 
@@ -172,9 +183,11 @@ public class DiscoveryREST {
                                               
@QueryParam("excludeDeletedEntities") boolean excludeDeletedEntities,
                                               @QueryParam("limit")             
     int     limit,
                                               @QueryParam("offset")            
     int     offset) throws AtlasBaseException {
-        Servlets.validateQueryParamLength("query", query);
         Servlets.validateQueryParamLength("typeName", typeName);
         Servlets.validateQueryParamLength("classification", classification);
+        if (StringUtils.isNotEmpty(query) && query.length() > 
maxFullTextQueryLength) {
+            throw new AtlasBaseException(AtlasErrorCode.INVALID_QUERY_LENGTH, 
Constants.MAX_FULLTEXT_QUERY_STR_LENGTH);
+        }
 
         AtlasPerfTracer perf = null;
 
@@ -556,7 +569,10 @@ public class DiscoveryREST {
         if (parameters != null) {
             Servlets.validateQueryParamLength("typeName", 
parameters.getTypeName());
             Servlets.validateQueryParamLength("classification", 
parameters.getClassification());
-            Servlets.validateQueryParamLength("query", parameters.getQuery());
+            if (StringUtils.isNotEmpty(parameters.getQuery()) && 
parameters.getQuery().length() > maxFullTextQueryLength) {
+                throw new 
AtlasBaseException(AtlasErrorCode.INVALID_QUERY_LENGTH, 
Constants.MAX_FULLTEXT_QUERY_STR_LENGTH);
+            }
+
         }
     }
 }

Reply via email to