Skip to content

Commit

Permalink
URL-encode job ID when forwarding from UIController
Browse files Browse the repository at this point in the history
Motivation:
Download of files from the web UI fails if the job ID contains certain characters (such as '+').
This is due to URL decoding performed on the "forward URL" string returned by UIController (before passing the URL to JobRestController).

Change:
URL-encode the ID contained in the forward string.
This results in the ID being decoded back by Spring MVC to it's original format, and passed intact to the downstream JobRestController.
  • Loading branch information
mprimi committed Jul 24, 2017
1 parent 7e0fb64 commit 395c373
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/
package com.netflix.genie.web.controllers;

import lombok.extern.slf4j.Slf4j;
import org.springframework.security.core.Authentication;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.PathVariable;
Expand All @@ -28,6 +29,8 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.validation.constraints.NotNull;
import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;

/**
* Controller for forwarding UI requests.
Expand All @@ -36,6 +39,7 @@
* @since 3.0.0
*/
@Controller
@Slf4j
public class UIController {

/**
Expand Down Expand Up @@ -71,9 +75,19 @@ public String getIndex(@NotNull final HttpServletResponse response, @Nullable fi
* @param id The id of the job
* @param request the servlet request to get path information from
* @return The forward address to go to at the API endpoint
* @throws UnsupportedEncodingException if URL-encoding of the job id fails
*/
@RequestMapping(value = "/file/{id}/**", method = RequestMethod.GET)
public String getFile(@PathVariable("id") final String id, final HttpServletRequest request) {
return "forward:/api/v3/jobs/" + id + "/" + ControllerUtils.getRemainingPath(request);
public String getFile(
@PathVariable("id") final String id,
final HttpServletRequest request
) throws UnsupportedEncodingException {
// When forwarding, the downstream ApplicationContext will perform URL decoding.
// If the job ID contains special characters (such as '+'), they will be interpreted as url-encoded and
// decoded, resulting in an invalid job ID that cannot be found.
// To prevent this, always perform URL encoding on the ID before forwarding.
final String encodedId = URLEncoder.encode(id, "UTF-8");
final String path = "/api/v3/jobs/" + encodedId + "/" + ControllerUtils.getRemainingPath(request);
return "forward:" + path;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.net.URLEncoder;
import java.util.UUID;

/**
Expand Down Expand Up @@ -75,9 +76,10 @@ public void canGetIndex() {

/**
* Make sure the getFile method returns the right forward command.
* @throws Exception if an error occurs
*/
@Test
public void canGetFile() {
public void canGetFile() throws Exception {
final String id = UUID.randomUUID().toString();
final HttpServletRequest request = Mockito.mock(HttpServletRequest.class);

Expand All @@ -88,9 +90,12 @@ public void canGetFile() {
.when(request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE))
.thenReturn("/file/{id}/**");

final String encodedId = URLEncoder.encode(id, "UTF-8");
final String expectedPath = "/api/v3/jobs/" + encodedId + "/output/genie/log.out";

Assert.assertThat(
this.controller.getFile(id, request),
Matchers.is("forward:/api/v3/jobs/" + id + "/output/genie/log.out")
Matchers.is("forward:" + expectedPath)
);
}
}

0 comments on commit 395c373

Please sign in to comment.