Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uploading a too-large file should be a 4xx client error, not a 500 Internal Server Error #27170

Closed
mjustin opened this issue Jul 14, 2021 · 1 comment
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@mjustin
Copy link

mjustin commented Jul 14, 2021

When uploading a file that is too large (exceeding either spring.servlet.multipart.max-file-size or spring.servlet.multipart.max-request-size), a 500 Internal Server Error is thrown. This seems odd to me, as this is an error due to an unsupported value sent by the client, not an unexpected server issue.

It feels like a 4xx client error would be more appropriate for this situation, such as 413 Payload Too Large.

Workaround

This can be manually implemented in a Spring Boot application today by creating a custom @ExceptionHandler for MaxUploadSizeExceededException. If the handler is set in the controller class, the spring.servlet.multipart.resolve-lazily property must also be set to true:

@ExceptionHandler
public void maxUploadSizeExceeded(MaxUploadSizeExceededException e, HttpServletResponse response) throws IOException {
    response.sendError(HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE);
}

Example of current behavior

import org.springframework.web.bind.annotation.*;
import org.springframework.web.multipart.MultipartFile;

@RestController
@RequestMapping("/fileUpload")
public class FileUploadController {
    @PostMapping
    public String handleFileUpload(@RequestParam("file") MultipartFile file) {
        return String.format("Uploaded %s (%s bytes)", file.getName(), file.getSize());
    }
}
import com.fasterxml.jackson.databind.JsonNode;
import org.junit.jupiter.api.*;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.web.client.TestRestTemplate;
import org.springframework.core.io.FileSystemResource;
import org.springframework.http.*;
import org.springframework.test.context.TestPropertySource;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.web.multipart.MaxUploadSizeExceededException;

import java.io.*;
import java.nio.file.*;

import static org.junit.jupiter.api.Assertions.assertEquals;

class UploadTest {
    @Nested
    @TestPropertySource(properties = "spring.servlet.multipart.max-file-size:1000B")
    class MaxFileSize extends AbstractUploadTest {
        @Test
        void uploadFileLargerThanMaxFileSize() {
            ResponseEntity<JsonNode> response = uploadFileWithError(" ".repeat(2000).getBytes());
            assertEquals(MaxUploadSizeExceededException.class.getName(), response.getBody().get("exception").textValue());
            assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode());
        }
    }

    @Nested
    @TestPropertySource(properties = "spring.servlet.multipart.max-request-size:1000B")
    class MaxRequestSize extends AbstractUploadTest  {
        @Test
        void uploadFileLargerThanMaxRequestSize() {
            ResponseEntity<JsonNode> response = uploadFileWithError(" ".repeat(2000).getBytes());
            assertEquals(MaxUploadSizeExceededException.class.getName(), response.getBody().get("exception").textValue());
            assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode());
        }
    }

    @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
    @TestPropertySource(properties = "server.error.include-exception=true")
    private static abstract class AbstractUploadTest {
        @Autowired
        private TestRestTemplate testRestTemplate;

        protected ResponseEntity<JsonNode> uploadFileWithError(byte[] bytes) {
            return testRestTemplate.postForEntity("/fileUpload",
                    getRequestEntity(bytes), JsonNode.class);
        }

        private HttpEntity<LinkedMultiValueMap<String, Object>> getRequestEntity(byte[] data) {
            LinkedMultiValueMap<String, Object> parameters = new LinkedMultiValueMap<>();
            parameters.add("file", createTempFile(data));

            HttpHeaders headers = new HttpHeaders();
            headers.setContentType(MediaType.MULTIPART_FORM_DATA);

            return new HttpEntity<>(parameters, headers);
        }

        private FileSystemResource createTempFile(byte[] data) {
            try {
                Path file = Files.createTempFile("test", ".txt");
                Files.write(file, data);
                return new FileSystemResource(file);
            } catch (IOException e) {
                throw new UncheckedIOException(e);
            }
        }
    }
}
@wilkinsona
Copy link
Member

Thanks for the suggestion.

If there is to be a default mapping to a 413 response, I think it should be done in Spring Framework, possibly in the existing org.springframework.web.servlet.mvc.support.DefaultHandlerExceptionResolver class. We'll transfer this issue to the Framework team for their consideration.

@snicoll snicoll transferred this issue from spring-projects/spring-boot Jul 14, 2021
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 14, 2021
@rstoyanchev rstoyanchev added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Nov 8, 2021
@sdeleuze sdeleuze self-assigned this Oct 24, 2023
@sdeleuze sdeleuze added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 24, 2023
@sdeleuze sdeleuze added this to the 6.1.0-RC2 milestone Oct 24, 2023
sdeleuze added a commit to sdeleuze/spring-framework that referenced this issue Oct 25, 2023
This commit refines MaxUploadSizeExceededException
handling in order to translate to a "413 Payload Too Large"
status code instead of "500 Internal Server Error", with
related ProblemDetail body.

Closes spring-projectsgh-27170
sdeleuze added a commit to sdeleuze/spring-framework that referenced this issue Oct 25, 2023
This commit refines MaxUploadSizeExceededException
handling in order to translate to a "413 Payload Too Large"
status code instead of "500 Internal Server Error", with
related ProblemDetail body.

Closes spring-projectsgh-27170
tdonohue added a commit to tdonohue/DSpace that referenced this issue Mar 12, 2024
tdonohue added a commit to tdonohue/DSpace that referenced this issue Mar 20, 2024
floriangantner pushed a commit to uniba-ub/DSpace that referenced this issue Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants