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

Data binding does not filter HTTP headers for constructor binding #34292

Closed
jannikFuellgrafEnvite opened this issue Jan 21, 2025 · 10 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Milestone

Comments

@jannikFuellgrafEnvite
Copy link

Summary

Since upgrading to Spring Boot 3.4.1, we have observed unexpected behavior where HTTP headers with names matching @JsonProperty annotations on POJO fields are being automatically deserialized into those fields. This behavior was not present in earlier versions.


Observations

  1. Affected Behavior:

    • An HTTP request with a priority header results in deserialization into the TaskQueryFilterParameter.priorityIn field, which has the annotation @JsonProperty("priority").
    • Similarly, a planned header is deserialized into the TaskQueryFilterParameter.plannedWithin field, annotated with @JsonProperty("planned").
  2. Reproducibility:

    • This behavior occurs whenever request headers match the names used in @JsonProperty annotations on request object fields.
  3. Impact:

    • This behavior can lead to unintended data mapping, affecting applications that rely on these headers for different purposes.
  4. Analysis Results:

    • After reviewing the dependencies and updates introduced with Spring Boot 3.4.1, we were unable to identify the specific cause (e.g., a library or code change).
    • It is unclear whether this is an intentional feature or a regression.

Expected Behavior

It is unclear whether HTTP headers with names matching @JsonProperty annotations should be automatically deserialized into POJO fields. If this is the intended behavior, guidance on how to configure or disable it would be helpful.

Actual Behavior

HTTP headers with names matching @JsonProperty annotations are deserialized into the respective POJO fields, even without explicit configuration.


Questions

  1. Is this behavior an intentional change in Spring Boot 3.4.1?
  2. If so, what is its purpose, and how can it be disabled if undesired?
  3. If this is a bug, is there a planned fix or workaround?

Thank you for your time and assistance! 😊

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 21, 2025
@bclozel bclozel transferred this issue from spring-projects/spring-boot Jan 21, 2025
@bclozel
Copy link
Member

bclozel commented Jan 21, 2025

This is a new feature introduced in Spring Framework 6.2.

We have refined this behavior in 6.2.2. The priority header should not be bound anymore. As for the "planned" header, I've never seen this header, is this something common? In any case, you can get more control over the header binding with the following: #34182 (comment)

This week's Spring Boot release will use this version, but you can already upgrade in your build with the "spring-framework.version" property.

@bclozel bclozel added status: waiting-for-feedback We need additional information before we can continue in: web Issues in web modules (web, webmvc, webflux, websocket) labels Jan 21, 2025
@jannikFuellgrafEnvite
Copy link
Author

Thank you for the quick and detailed response! 😊

Regarding the “planned” header, it is not a standard header but rather a custom one we introduced specifically to test and confirm the behavior alongside the “priority” header. Its sole purpose was to validate that headers matching @JsonProperty annotations are consistently being bound, regardless of whether they are commonly used or not.

@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 Jan 21, 2025
@bclozel
Copy link
Member

bclozel commented Jan 21, 2025

Alright thanks for the feedback.
I'll close this issue now, since the situation is already resolved for "priority" and that apps can opt-out of this feature for all other headers as well.

Thanks for reaching out!

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Jan 21, 2025
@bclozel bclozel added status: duplicate A duplicate of another issue and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Jan 21, 2025
@jannikFuellgrafEnvite
Copy link
Author

Dear @bclozel,

Unfortunately, the issue has not been resolved yet in our setup. I am attaching the error message below, which indicates that the priority header is still being applied.
Is there a specific configuration that needs to be considered to achieve the expected behavior?

Since I do not have the necessary permissions to reopen the ticket, I hope this comment reaches you and that we can still solve the issue.

org.springframework.web.bind.MethodArgumentNotValidException: Validation failed for argument [1] in public org.springframework.http.ResponseEntity<io.kadai.task.rest.models.TaskSummaryPagedRepresentationModel> io.kadai.task.rest.TaskController.getTasks(jakarta.servlet.http.HttpServletRequest,io.kadai.task.rest.TaskQueryFilterParameter,io.kadai.task.rest.TaskQueryFilterCustomFields,io.kadai.task.rest.TaskQueryFilterCustomIntFields,io.kadai.task.rest.TaskQueryGroupByParameter,io.kadai.task.rest.TaskController$TaskQuerySortParameter,io.kadai.common.rest.QueryPagingParameter<io.kadai.task.api.models.TaskSummary, io.kadai.task.api.TaskQuery>): [Field error in object 'taskQueryFilterParameter' on field 'priority': rejected value [u=3, i]; codes [typeMismatch.taskQueryFilterParameter.priority,typeMismatch.priority,typeMismatch.[I,typeMismatch]; arguments [org.springframework.context.support.DefaultMessageSourceResolvable: codes [taskQueryFilterParameter.priority,priority]; arguments []; default message [priority]]; default message [Failed to convert value of type 'java.lang.String' to required type 'int[]'; For input string: "u=3,i"]]

@bclozel
Copy link
Member

bclozel commented Jan 28, 2025

Can you point me to something I can run in order to reproduce the issue?

@bclozel bclozel reopened this Jan 28, 2025
@bclozel bclozel added status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged or decided on and removed status: duplicate A duplicate of another issue labels Jan 28, 2025
@jannikFuellgrafEnvite
Copy link
Author

Hi @bclozel,

Thanks for looking into this!

Here is the Pull Request with the update to Spring Boot 3.4.2:
🔗 PR #478 - Update to Spring Boot 3.4.2

To check out the code and run the application, follow these steps:

git clone https://github.com/kadai-io/kadai.git
gh pr checkout 478
cd kadai
./mvnw clean install -DskipTests
./mvnw spring-boot:run -pl :kadai-rest-spring-example-boot

This will start our application. Navigate to http://localhost:8080/kadai/login

About Kadai

We are building an open-source enterprise task management solution called Kadai.

Issue Details

The error occurs when using Safari and Firefox.

Steps to Reproduce
1. Click on the Burger Menu in the top-left corner.
2. Navigate to Workplace.
3. Switch between Workbaskets and Task Search.

At this point, an error should appear, caused by the priority header being applied.

Let us know if you need any further details! We appreciate your help in resolving this issue.

@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 Jan 28, 2025
@bclozel
Copy link
Member

bclozel commented Jan 28, 2025

What am I supposed to use as credentials? I have tried "admin/admin" and then I'm getting a 404 with

{
  "type" : "about:blank",
  "title" : "Not Found",
  "status" : 404,
  "detail" : "No static resource index.html.",
  "instance" : "/kadai/index.html"
}

Maybe it would be easier to share a minimal sample created with start.spring.io and a simple controller that replicates what you're binding to?

@bclozel bclozel added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jan 28, 2025
@jannikFuellgrafEnvite
Copy link
Author

Sorry, I'm still new to the project.

You are correct about the credentials: admin/admin.

The correct steps before running the application are:

cd kadai
cd web
yarn install
yarn build:prod
cd ..
./mvnw clean install -DskipTests
./mvnw spring-boot:run -pl :kadai-rest-spring-example-boot

If that still doesn’t work, I will create a minimal example to improve reproducibility.

@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 Jan 29, 2025
@bclozel bclozel self-assigned this Jan 29, 2025
@bclozel bclozel removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 29, 2025
@bclozel bclozel added type: bug A general bug and removed status: feedback-provided Feedback has been provided labels Jan 29, 2025
@bclozel bclozel added this to the 6.2.3 milestone Jan 29, 2025
@bclozel bclozel changed the title Unexpected Header to Parameter Deserialization Behavior in Spring Boot 3.4.1 Data binding does not filter HTTP headers for constructor binding Jan 29, 2025
@bclozel
Copy link
Member

bclozel commented Jan 29, 2025

Thanks for your help @jannikFuellgrafEnvite , I managed to reproduce this with a simple setup:

@RestController
public class TestController {

	@GetMapping("/tasks")
	public TaskQueryFilterParameter getTasks(TaskQueryFilterParameter taskQueryFilterParameter) {
		return taskQueryFilterParameter;
	}

	record TaskQueryFilterParameter(int[] priority) {
	}

}
$ http :8080/tasks 'Priority:u=0'

{
    "error": "Bad Request",
    "path": "/tasks",
    "status": 400,
    "timestamp": "2025-01-29T15:24:33.276+00:00"
}

The fix should be available in 6.2.3-SNAPSHOT in a few minutes. In the meantime you can apply this workaround to your application.

Cheers!

@jannikFuellgrafEnvite
Copy link
Author

Thank you @bclozel ! :-)

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: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants