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

Priority header causes binding exception after upgrade to Spring Framework 6.2.0 #34039

Closed
leonchen83 opened this issue Dec 6, 2024 · 4 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: regression A bug that is also a regression
Milestone

Comments

@leonchen83
Copy link
Contributor

leonchen83 commented Dec 6, 2024

Overview

After upgrading to Spring Framework 6.2, I encountered an issue where request headers such as priority are automatically bound to the parameters in my controller. This behavior wasn't present in earlier versions (e.g., 6.1). My intention is to treat missing parameters (like priority) as default values (e.g., 0), but now the framework seems to bind unexpected header values like priority: u=1, i, which leads to unexpected behavior.

I would like to know if there’s a way to disable automatic header binding or configure Spring to ignore specific headers like priority when processing requests.

  • spring framework version: 6.2.0
  • java version: java21

Example

public class HoldingAllocatorPo {
	private short priority;
	public short getPriority() {
		return priority;
	}
	public void setPriority(short priority) {
		this.priority = priority;
	}
        ....
public class ShortConverter implements org.springframework.core.convert.converter.Converter<String, Short> {
	 @Override
	 public Short convert(String source) {
		 return source == null || source.length() == 0 ? 0 : Short.valueOf(source);
	 }
 }

When we register this ShortConverter and submit a form request like this:
type=1&status=1&companyId=1&name=tt&abbreviation=tt&tradeTypes=7&sides=3&orderTypes=85
Our intention is that when priority is not set, it should be treated as 0. This worked correctly in Spring Framework versions prior to 6.2. However, after upgrading to 6.2, it no longer works as expected.

For example, in Chrome, the complete request looks like this:

curl 'https://xxx.xxx.xx/rest/holding/allocator/validate' \
  -H 'accept: application/json, text/plain, */*' \
  -H 'accept-language: zh-CN,zh;q=0.9,en;q=0.8' \
  -H 'content-type: application/x-www-form-urlencoded' \
  -H 'cookie: lang=1; _ga_0C4M1PWYZ7=GS1.1.1701684974.1.1.1701685076.0.0.0; _ga_K2SPJK2C73=GS1.1.1701684974.1.1.1701685076.59.0.0; _ga_T11SF3WXX2=GS1.1.1701684974.1.1.1701685076.59.0.0; _yjsu_yjad=1704697517.59e4b353-f159-4b04-aae9-2abff30d463b; _uetvid=47cc51e0adf411eeafdf4df33d978593; _ga_975G4RTB2H=GS1.1.1725435510.1.0.1725435669.60.0.0; _ga=GA1.2.2093050243.1701684974; JSESSIONID.admin=admin9@2OFM5mJyDln.1qFXXfVSXUs.azV0fNQr0Qh' \
  -H 'origin: https://xxx.xxx.xx' \
  -H 'priority: u=1, i' \
  -H 'referer: https://xxx.xxx.xx/' \
  -H 'sec-ch-ua: "Google Chrome";v="131", "Chromium";v="131", "Not_A Brand";v="24"' \
  -H 'sec-ch-ua-mobile: ?0' \
  -H 'sec-ch-ua-platform: "Windows"' \
  -H 'sec-fetch-dest: empty' \
  -H 'sec-fetch-mode: cors' \
  -H 'sec-fetch-site: same-origin' \
  -H 'user-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/131.0.0.0 Safari/537.36' \
  -H 'x-browser: Chrome 131.0.6778.87' \
  -H 'x-platform: Windows 15.0.0' \
  -H 'x-version: 6fb5f6eb' \
  --data-raw 'type=1&status=1&companyId=1&name=tt&abbreviation=tt&tradeTypes=7&sides=3&orderTypes=85'

In this request, the priority header is present but contains unexpected values like u=1, i. After the upgrade, this causes issues with handling the request in Spring Framework 6.2.

related issue 32676

Question

Is there a way to prevent binding request headers in Spring Framework?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 6, 2024
@snicoll snicoll added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Dec 6, 2024
@quaff

This comment was marked as outdated.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Dec 10, 2024

Thanks for the report.

There is a way, but it is not very convenient. Headers and path vars are added from ExtendedServletRequestDataBinder#addBindValues, which is a protected method. To plug that subclass you need to override RequestMappingHandlerAdapter#createDataBinderFactory. In Spring Boot, WebMvcStrategies lets you provide an extension of RequestMappingHandlerAdapter.

To make this easier, we can add a Predicate on ExtendedServletRequestDataBinder, and you could then customize that locally on a controller, or globally through a controller advice class:

@ControllerAdvice
public class MyControllerAdvice {

    @InitBinder
    public void initBinder(ExtendedServletRequestDataBinder binder) {

        binder.addHeaderPredicate(header -> ... );
    }
}

The "Priority" header seems to be RFC-defined and common to both this and the report in #33961. We can exclude it by default as it is likely to cause more surprise.

@rstoyanchev rstoyanchev added type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 10, 2024
@rstoyanchev rstoyanchev added this to the 6.2.1 milestone Dec 10, 2024
@rstoyanchev rstoyanchev changed the title WebDataBinder binding exception after upgrade spring 6.2.x Priority header causes binding exception after upgrade to Spring Framework 6.2.0 Dec 10, 2024
rstoyanchev added a commit that referenced this issue Dec 11, 2024
Make it public and move it down to the annotations package alongside
InitBinderBindingContext. This is mirrors the hierarchy in Spring MVC
with the ExtendedServletRequestDataBinder. The change will allow
customization of the header names to include/exclude in data binding.

See gh-34039
@leonchen83
Copy link
Contributor Author

leonchen83 commented Dec 13, 2024

@rstoyanchev
We aim to customize a global header filtering strategy by extending RequestMappingHandlerAdapter#createDataBinderFactory.
To achieve this, we implemented a custom extension class XRequestMappingHandlerAdapter.

public class XRequestMappingHandlerAdapter extends RequestMappingHandlerAdapter {
	
	private Set<String> headers = new HashSet<>();
	
	public void setHeaders (Set<String> headers) {
		this.headers = headers;
	}
	
	@Override
	protected InitBinderDataBinderFactory createDataBinderFactory (List<InvocableHandlerMethod> a) {
		return new XServletRequestDataBinderFactory (a , getWebBindingInitializer() , this.headers);
	}
	
	public static final class XServletRequestDataBinderFactory extends InitBinderDataBinderFactory {
		
		private final Set<String> headers;
		
		public XServletRequestDataBinderFactory (List<InvocableHandlerMethod> a, WebBindingInitializer b, Set<String> c) {
			super(a, b); this.headers = c;
		}
		
		@Override
		protected ServletRequestDataBinder createBinderInstance(Object a, String b, NativeWebRequest c) throws Exception {
			var r = new ExtendedServletRequestDataBinder(a , b); r.setHeaderPredicate(h -> headers.contains(h)); return r;
		}
	}
}

However, we encountered issues when attempting to register this class.

We are using traditional Spring MVC XML configuration to manage dependencies. for example

    <mvc:annotation-driven conversion-service="web.admin.conversion.service">
        
        <mvc:argument-resolvers>
            <bean class="xx.xx.XXArgumentResolver"/>
            
        </mvc:argument-resolvers>
        
        <mvc:path-matching suffix-pattern = "false" path-matcher = "web.admin.path.matcher"/>
        
        <mvc:message-converters>
            <bean class="xx.glossary.spring.converter.OctetHttpMessageConverter">
            </bean>
            <bean class="xx.glossary.spring.converter.JsonHttpMessageConverter" >
                <property name="marshaller" ref="web.admin.json.marshaller" />
            </bean>
        </mvc:message-converters>
        
        <mvc:async-support default-timeout="90000" task-executor="web.admin.async.executor">
            <mvc:callable-interceptors>
                <bean id="web.admin.async.aspect" class="xx.aspect.AsyncInterceptor"/>
            </mvc:callable-interceptors>
        </mvc:async-support>
    </mvc:annotation-driven>

Is there a simple way to register XRequestMappingHandlerAdapter in the XML configuration?

@bclozel
Copy link
Member

bclozel commented Dec 13, 2024

@leonchen83 unfortunately no, there is no way to register a custom RequestMappingHandlerAdapter bean with the XML mvc namespace. We don't intend to evolve this configuration variant and it's going to be officially deprecated in the next major version with #34063

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: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

6 participants