Testing x-request-id validity.

yahoo · Oct 17, 2016 · 3109113 · 3109113
1 parent be0807f
commit 3109113
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 2 deletions.
diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/web/filters/BardLoggingFilter.java b/fili-core/src/main/java/com/yahoo/bard/webservice/web/filters/BardLoggingFilter.java
@@ -211,13 +211,23 @@ public void filter(ClientRequestContext request, ClientResponseContext response)
      * @param requestId  Request id to add as queryId prefix to druid
      */
     private void appendRequestId(String requestId) {
-        if (requestId == null || requestId.isEmpty() || requestId.length() > 200
-            || !VALID_REQUEST_ID.matcher(requestId).matches()) {
+        if (isValidRequestId(requestId)) {
             return; // Ignore according to https://devcenter.heroku.com/articles/http-request-id
         }
         RequestLog.addIdPrefix(requestId);
     }
 
+    /**
+     * Validate whether or not a string is acceptable as an x-request-id header argument.
+     *
+     * @param requestId  Request id to validate
+     * @return True if valid, false otherwise
+     */
+    private boolean isValidRequestId(String requestId) {
+        return requestId == null || requestId.isEmpty() || requestId.length() > 200
+               || !VALID_REQUEST_ID.matcher(requestId).matches();
+    }
+
     /**
      * Render the URI as a string.
      *

diff --git a/...t/groovy/com/yahoo/bard/webservice/web/endpoints/RequestIdPrefixesDruidQueryIdSpec.groovy b/...t/groovy/com/yahoo/bard/webservice/web/endpoints/RequestIdPrefixesDruidQueryIdSpec.groovy
@@ -7,6 +7,7 @@ import com.yahoo.bard.webservice.data.dimension.DimensionDictionary
 import com.yahoo.bard.webservice.util.JsonSlurper
 import com.yahoo.bard.webservice.util.JsonSortStrategy
 import com.yahoo.bard.webservice.web.filters.BardLoggingFilter
+import org.apache.commons.lang.StringUtils
 
 import javax.ws.rs.core.MultivaluedHashMap
 
@@ -75,4 +76,12 @@ class RequestIdPrefixesDruidQueryIdSpec extends BaseDataServletComponentSpec {
     MultivaluedHashMap<String, String> getAdditionalApiRequestHeaders() {
         return ["x-request-id": prefixId]
     }
+
+    def "verify invalid x-request-id values"() {
+        BardLoggingFilter filter = new BardLoggingFilter()
+        assert !filter.isValidRequestId('abcd$') // Invalid char
+        assert !filter.isValidRequestId(StringUtils.leftPad('a', 200, 'a')) // Too long
+        assert !filter.isValidRequestId('') // empty string
+        assert !filter.isValidRequestId(null) // null
+    }
 }