#000 - Validate stage counter during upload artifacts

gocd · Oct 22, 2021 · c22e042 · c22e042
1 parent c47c61c
commit c22e042
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 0 deletions.
diff --git a/server/src/main/java/com/thoughtworks/go/server/controller/ArtifactsController.java b/server/src/main/java/com/thoughtworks/go/server/controller/ArtifactsController.java
@@ -33,6 +33,7 @@
 import com.thoughtworks.go.server.web.ResponseCodeView;
 import com.thoughtworks.go.util.ArtifactLogUtil;
 import com.thoughtworks.go.util.SystemEnvironment;
+import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -149,6 +150,9 @@ public ModelAndView postArtifact(@RequestParam("pipelineName") String pipelineNa
         if (!headerConstraint.isSatisfied(request)) {
             return ResponseCodeView.create(HttpServletResponse.SC_BAD_REQUEST, "Missing required header 'Confirm'");
         }
+        if (!isValidStageCounter(stageCounter)) {
+            return buildNotFound(pipelineName, pipelineCounter, stageName, stageCounter, buildName);
+        }
         try {
             jobIdentifier = restfulService.findJob(pipelineName, pipelineCounter, stageName, stageCounter,
                     buildName, buildId);
@@ -224,6 +228,10 @@ public ModelAndView putArtifact(@RequestParam("pipelineName") String pipelineNam
             return FileModelAndView.forbiddenUrl(filePath);
         }
 
+        if (!isValidStageCounter(stageCounter)) {
+            return buildNotFound(pipelineName, pipelineCounter, stageName, stageCounter, buildName);
+        }
+
         JobIdentifier jobIdentifier;
         try {
             jobIdentifier = restfulService.findJob(pipelineName, pipelineCounter, stageName, stageCounter, buildName, buildId);
@@ -341,4 +349,17 @@ private ModelAndView logsNotFound(JobIdentifier identifier) {
         String notFound = String.format("Console log for %s is unavailable as it may have been purged by Go or deleted externally.", identifier.toFullString());
         return ResponseCodeView.create(SC_NOT_FOUND, notFound);
     }
+
+    private boolean isValidStageCounter(String stageCounter) {
+        if (StringUtils.isEmpty(stageCounter)) {
+            return true;
+        }
+
+        try {
+            int value = Integer.parseInt(stageCounter);
+            return value > 0;
+        } catch (Exception e) {
+            return false;
+        }
+    }
 }
diff --git a/server/src/test-fast/java/com/thoughtworks/go/server/controller/ArtifactsControllerTest.java b/server/src/test-fast/java/com/thoughtworks/go/server/controller/ArtifactsControllerTest.java
@@ -39,6 +39,7 @@
 
 import static com.thoughtworks.go.util.GoConstants.*;
 import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
+import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
 import static org.hamcrest.Matchers.*;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.mockito.Mockito.any;
@@ -120,6 +121,21 @@ public void shouldReturnHttpErrorCodeWhenChecksumFileSaveFails() throws Exceptio
         assertThat(view.getContent(), is("Error saving checksum file for the artifact at path 'some-path'"));
     }
 
+    @Test
+    void shouldFailToPostAndPutWhenStageCounterIsNotAPositiveInteger() throws Exception {
+        ModelAndView modelAndView = artifactsController.postArtifact("pipeline-1", "1", "stage-1", "NOT_AN_INTEGER", "job-1", 122L, "some-path", 1, null);
+        assertThat(((ResponseCodeView) modelAndView.getView()).getStatusCode(), is(SC_NOT_FOUND));
+
+        modelAndView = artifactsController.postArtifact("pipeline-1", "1", "stage-1", "-123", "job-1", 122L, "some-path", 1, null);
+        assertThat(((ResponseCodeView) modelAndView.getView()).getStatusCode(), is(SC_NOT_FOUND));
+
+        modelAndView = artifactsController.putArtifact("pipeline-1", "1", "stage-1", "NOT_AN_INTEGER", "job-1", 122L, "some-path", "1", null);
+        assertThat(((ResponseCodeView) modelAndView.getView()).getStatusCode(), is(SC_NOT_FOUND));
+
+        modelAndView = artifactsController.putArtifact("pipeline-1", "1", "stage-1", "-123", "job-1", 122L, "some-path", "1", null);
+        assertThat(((ResponseCodeView) modelAndView.getView()).getStatusCode(), is(SC_NOT_FOUND));
+    }
+
     @Test
     public void shouldFunnelAll_GET_calls() throws Exception {
         final ModelAndView returnVal = new ModelAndView();