From 642fe838979fd3bc0890f53ef31737a0ed4354c1 Mon Sep 17 00:00:00 2001
From: Suneet Saldanha <44787917+suneet-s@users.noreply.github.com>
Date: Fri, 10 Apr 2020 10:36:26 -0700
Subject: [PATCH] Indexing Service validates externally received taskId (#9666)

Addresses issues flagged by https://lgtm.com/rules/5970070/
---
 .../overlord/http/security/TaskResourceFilter.java        | 2 ++
 .../apache/druid/indexing/worker/http/WorkerResource.java | 8 +++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/TaskResourceFilter.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/TaskResourceFilter.java
index af1f8222d1f..042c2a2b6f5 100644
--- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/TaskResourceFilter.java
+++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/TaskResourceFilter.java
@@ -25,6 +25,7 @@ import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
 import com.google.inject.Inject;
 import com.sun.jersey.spi.container.ContainerRequest;
+import org.apache.druid.indexer.TaskIdUtils;
 import org.apache.druid.indexing.common.task.Task;
 import org.apache.druid.indexing.overlord.TaskStorageQueryAdapter;
 import org.apache.druid.java.util.common.StringUtils;
@@ -81,6 +82,7 @@ public class TaskResourceFilter extends AbstractResourceFilter
                ).getPath()
     );
     taskId = StringUtils.urlDecode(taskId);
+    TaskIdUtils.validateId("taskId", taskId);
 
     Optional<Task> taskOptional = taskStorageQueryAdapter.getTask(taskId);
     if (!taskOptional.isPresent()) {
diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/worker/http/WorkerResource.java b/indexing-service/src/main/java/org/apache/druid/indexing/worker/http/WorkerResource.java
index cafabc9e525..dd108f3f188 100644
--- a/indexing-service/src/main/java/org/apache/druid/indexing/worker/http/WorkerResource.java
+++ b/indexing-service/src/main/java/org/apache/druid/indexing/worker/http/WorkerResource.java
@@ -27,6 +27,7 @@ import com.google.common.collect.Lists;
 import com.google.common.io.ByteSource;
 import com.google.inject.Inject;
 import com.sun.jersey.spi.container.ResourceFilters;
+import org.apache.druid.indexer.TaskIdUtils;
 import org.apache.druid.indexing.overlord.TaskRunner;
 import org.apache.druid.indexing.overlord.TaskRunnerWorkItem;
 import org.apache.druid.indexing.worker.Worker;
@@ -184,10 +185,11 @@ public class WorkerResource
   @Produces(HttpMediaType.TEXT_PLAIN_UTF8)
   @ResourceFilters(StateResourceFilter.class)
   public Response doGetLog(
-      @PathParam("taskid") String taskid,
+      @PathParam("taskid") String taskId,
       @QueryParam("offset") @DefaultValue("0") long offset
   )
   {
+    TaskIdUtils.validateId("taskId", taskId);
     if (!(taskRunner instanceof TaskLogStreamer)) {
       return Response.status(501)
                      .entity(StringUtils.format(
@@ -197,7 +199,7 @@ public class WorkerResource
                      .build();
     }
     try {
-      final Optional<ByteSource> stream = ((TaskLogStreamer) taskRunner).streamTaskLog(taskid, offset);
+      final Optional<ByteSource> stream = ((TaskLogStreamer) taskRunner).streamTaskLog(taskId, offset);
 
       if (stream.isPresent()) {
         return Response.ok(stream.get().openStream()).build();
@@ -206,7 +208,7 @@ public class WorkerResource
       }
     }
     catch (IOException e) {
-      log.warn(e, "Failed to read log for task: %s", taskid);
+      log.warn(e, "Failed to read log for task: %s", taskId);
       return Response.serverError().build();
     }
   }