Skip to content

Inconsistent validation of query parameters when using MockMvc #24000

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

Closed
mharward opened this issue Nov 14, 2019 · 7 comments
Closed

Inconsistent validation of query parameters when using MockMvc #24000

mharward opened this issue Nov 14, 2019 · 7 comments
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) status: feedback-provided Feedback has been provided status: invalid An issue that we don't feel is valid

Comments

@mharward
Copy link

mharward commented Nov 14, 2019

Affects: spring-test:5.1.6


When using MockMvc to perform test requests of a @Validated controller endpoint, validations (javax.validation.constraints) are not applied in certain cases depending on the formatting of the URI.

mockMvc.perform(get("/my-endpoint?param={param}", "value"))... // Works
mockMvc.perform(get(format("/my-endpoint?param=%s", "value")))... // Works 
mockMvc.perform(get("/my-endpoint?param=%s", "value"))... // Works, but not when doing validation
mockMvc.perform(get("/my-endpoint?param={}", "value"))... // Works, but not when doing validation

Just raising this in case it can be made more consistent or if this might help someone else in the future.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 14, 2019
@sbrannen sbrannen added in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) labels Nov 14, 2019
@sbrannen
Copy link
Member

Can you please provide the code for the mapped controller method that you are testing?

@sbrannen sbrannen added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 14, 2019
@mharward
Copy link
Author

mharward commented Nov 14, 2019

@sbrannen here is a stripped down example of the controller...

@RestController
@Validated
public class MyController {

    @GetMapping("/my-endpoint")
    public AResponse myEndpoint(
            @RequestParam
            @Pattern(regexp = "\\d+", message = "Invalid param, must be number")
            String param)
            {...}
}

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 14, 2019
@sbrannen
Copy link
Member

What happens if you move the @Validated annotation to the method parameter to be validated like the following?

@RestController
public class MyController {

    @GetMapping("/my-endpoint")
    public AResponse myEndpoint(
            @Validated
            @RequestParam
            @Pattern(regexp = "\\d+", message = "Invalid param, must be number")
            String param)
            {...}
}

@sbrannen sbrannen added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Nov 14, 2019
@mharward
Copy link
Author

mharward commented Nov 15, 2019

@sbrannen, in that case the validation does not appear happen in any of the above examples.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 15, 2019
@sbrannen
Copy link
Member

Thanks for trying that and reporting back.

And... I apologize: that's not supposed to work. @Validated is only supported directly on handler method parameters for model attribute, @RequestBody, and @RequestPart arguments. See #11041.

@sbrannen sbrannen added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 15, 2019
@sbrannen
Copy link
Member

All of the examples you have supplied pass with proper validation for me, as can be seen in the following passing test class.

import javax.validation.ConstraintViolationException;
import javax.validation.constraints.Pattern;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.context.junit.jupiter.web.SpringJUnitWebConfig;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.validation.annotation.Validated;
import org.springframework.validation.beanvalidation.MethodValidationPostProcessor;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.context.WebApplicationContext;
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
import org.springframework.web.util.NestedServletException;

import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.setup.MockMvcBuilders.webAppContextSetup;

@SpringJUnitWebConfig
class ValidatedControllerTests {

	MockMvc mockMvc;


	@BeforeEach
	void setUp(WebApplicationContext wac) {
		this.mockMvc = webAppContextSetup(wac).build();
	}

	@Test
	void validationIsApplied() throws Exception {
		assertThatExceptionOfType(NestedServletException.class)//
				.isThrownBy(() -> mockMvc.perform(get("/my-endpoint?param={param}", "value")))//
				.withCauseExactlyInstanceOf(ConstraintViolationException.class);

		assertThatExceptionOfType(NestedServletException.class)//
				.isThrownBy(() -> mockMvc.perform(get(String.format("/my-endpoint?param=%s", "value"))))//
				.withCauseExactlyInstanceOf(ConstraintViolationException.class);

		assertThatExceptionOfType(NestedServletException.class)//
				.isThrownBy(() -> mockMvc.perform(get("/my-endpoint?param=%s", "value")))//
				.withCauseExactlyInstanceOf(ConstraintViolationException.class);
		assertThatExceptionOfType(NestedServletException.class)//
				.isThrownBy(() -> mockMvc.perform(get("/my-endpoint?param={}", "value")))//
				.withCauseExactlyInstanceOf(ConstraintViolationException.class);
	}


	@Configuration
	@EnableWebMvc
	static class WebConfig {

		@Bean
		MethodValidationPostProcessor methodValidationPostProcessor() {
			return new MethodValidationPostProcessor();
		}

		@Bean
		MyController myController() {
			return new MyController();
		}
	}

	@RestController
	@Validated
	static class MyController {

		@GetMapping("/my-endpoint")
		String myEndpoint(@RequestParam @Pattern(regexp = "\\d+", message = "Must be a number") String param) {
			return "results";
		}
	}

}

I am therefore closing this issue.

@sbrannen sbrannen added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 20, 2019
@sbrannen
Copy link
Member

Note, however, that %s and empty {} are not supported as placeholders in URL templates. Thus, you cannot use those within the urlTemplate you supply to org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get(String, Object...), since the literal string values "%s" and "{}" are not numbers as required by your controller method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) status: feedback-provided Feedback has been provided status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

3 participants