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

SpringEncoder issue #628

Closed
prasadpr1 opened this issue Nov 14, 2021 · 15 comments
Closed

SpringEncoder issue #628

prasadpr1 opened this issue Nov 14, 2021 · 15 comments

Comments

@prasadpr1
Copy link

prasadpr1 commented Nov 14, 2021

Hi Team,

I am facing issue with springencoder in the new version of open feing. It used work fine with earlier versions of spring cloud.
When I debuuged the code, I found this additional code has been added in the new version of SpringEncoder

if (Objects.equals(requestContentType, MediaType.MULTIPART_FORM_DATA)) {
				this.springFormEncoder.encode(requestBody, bodyType, request);
				return;
			}

Earlier my code was using AllEncompassingFormHttpMessageConverter(SpringCloud 2.2.1) when i submitted multivalue map(MULTIFORM_DATA) but now it breaks with new version.

@OlgaMaciaszek
Copy link
Collaborator

Hello @prasadpr1, please provide a minimal, complete, verifiable example that reproduces the issue.

@prasadpr1
Copy link
Author

Please compare the code of SpringEncoder with previous versions. In the encode method, encoding implementation for multi-form data has been changed.. Version 2.2.1 and 2.5.6

@OlgaMaciaszek
Copy link
Collaborator

OlgaMaciaszek commented Nov 23, 2021

Yes, it has changed - it was fixed to handle multipart with dedicated encoding. If you have a use case where this causes an issue, please provide a minimal, complete, verifiable example that reproduces the issue (preferably as a GH repo link containing a small demo app) and we will verify it.

@prasadpr1
Copy link
Author

prasadpr1 commented Nov 23, 2021

We were using one of the apis provided by camunda.

https://docs.camunda.org/manual/7.8/reference/rest/process-instance/variables/post-variable-binary/

With earlier versions of spring it was using AllEncompassingFormHttpMessageConverter and the api was working fine with Multiform data.

Now with new versions the encoding has been changed and api no more works.
Below is the error log

[TestCLient#addVariable] Content-Length: 17

  • 2021.11.23 17:13:47.946 [http-nio-7071-exec-1] DEBUG TestCLientt - [TestCLient#addVariable] Content-Type: multipart/form-data; charset=UTF-8; boundary=17d4c9bf0de
  • 2021.11.23 17:13:47.948 [http-nio-7071-exec-1] DEBUG TestCLientt - [TestCLient#addVariable]
  • 2021.11.23 17:13:47.949 [http-nio-7071-exec-1] DEBUG TestCLientt - [TestCLient#addVariable] Binary data
  • 2021.11.23 17:13:47.951 [http-nio-7071-exec-1] DEBUG TestCLientt - [TestCLient#addVariable] ---> END HTTP (17-byte body)
  • 2021.11.23 17:13:47.975 [http-nio-7071-exec-1] DEBUG TestCLientt - [TestCLient#addVariable] <--- HTTP/1.1 500 Internal Server Error (22ms)
  • 2021.11.23 17:13:47.976 [http-nio-7071-exec-1] DEBUG TestCLientt - [TestCLient#addVariable] content-length: 46
  • 2021.11.23 17:13:47.978 [http-nio-7071-exec-1] DEBUG TestCLientt - [TestCLient#addVariable] content-type: application/json
  • 2021.11.23 17:13:47.979 [http-nio-7071-exec-1] DEBUG TestCLientt - [TestCLient#addVariable] date: Tue, 23 Nov 2021 11:43:48 GMT
  • 2021.11.23 17:13:47.981 [http-nio-7071-exec-1] DEBUG TestCLientt - [TestCLient#addVariable] vary: Access-Control-Request-Headers
  • 2021.11.23 17:13:47.982 [http-nio-7071-exec-1] DEBUG TestCLientt - [TestCLient#addVariable] vary: Access-Control-Request-Method
  • 2021.11.23 17:13:47.984 [http-nio-7071-exec-1] DEBUG TestCLientt - [TestCLient#addVariable] vary: Origin

Debug logs with old version :

Writing [{data=[MultipartFile resource [file]]}] as "multipart/form-data" using [org.springframework.http.converter.support.AllEncompassingFormHttpMessageConverter@5fc1e4fb]
TestCLient 2021.11.23 17:27:03.090 [http-nio-7071-exec-1] DEBUG TestCLient - [TestCLient#addVariable] ---> POST http://localhost:9090/rest/process-instance/18327102-4c31-11ec-8f35-eeee0afff8bb/variables/userdetails.json/data HTTP/1.1
TestCLient 2021.11.23 17:27:03.219 [http-nio-7071-exec-1] DEBUG TestCLient - [TestCLient#addVariable] <--- HTTP/1.1 204 No Content (125ms)

@OlgaMaciaszek
Copy link
Collaborator

@prasadpr1 Please provide the actual client setup code (as a runnable maven/ gradle demo app in a GitHub repo). Also, please note that the only supported release trains currently are 2020.0.x (Spring Cloud OpenFeign 3.0.x) and 2021.0.x (Spring Cloud OpenFeign 3.1.x). Please upgrade to a supported version first and verify if the issue still exists.

@prasadpr1
Copy link
Author

prasadpr1 commented Dec 1, 2021

I am currently using OpenFeign-3.0.5 and Cloud 2020.0.4.
Issue happens only with MutlivalueMap not with other types of data as encoding mechanism has been changed post release.
Can you explain why this encoding mechanism changed when it was working fine with AllEncompassingFormHttpMessageConverter.

@OlgaMaciaszek
Copy link
Collaborator

It was a fix - was supposed to improve the functioning and not cause bugs. If you have a scenario where this causes issues, we'll verify it and improve if required. For us to be able to do it, please provide a minimal, complete, verifiable example that reproduces the issue.

@spring-cloud-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-cloud-issues
Copy link

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@prasadpr1
Copy link
Author

Still not working.

@prasadpr1
Copy link
Author

prasadpr1 commented Dec 29, 2021

This part of code is causing issues. Can you please check why this has been changed when compared with earlier versions. Earlier version was using AllEncompassingFormHttpMessageConverter to encode form related data but with latest versions this has been changed. This sudden change in the code is causing issues with working code with library upgarde from 2.2 to 2.6.

	if (isFormRelatedContentType(requestContentType)) {
				springFormEncoder.encode(requestBody, bodyType, request);
				return;
			}

@ReneBentsen
Copy link

ReneBentsen commented Jan 4, 2022

Hi
I am facing the same issue. I have a body with 2 values.
Using requestContentType = application/x-www-form-urlencoded
And my requestbody is a LinkMultiValueMap
image

In the old version of the encode method this would result in the encode method ending up in the
encodeWithMessageConverter(requestBody, bodyType, request, requestContentType);
part of the code.

@Override
public void encode(Object requestBody, Type bodyType, RequestTemplate request) throws EncodeException {
	// template.body(conversionService.convert(object, String.class));
	if (requestBody != null) {
		Collection<String> contentTypes = request.headers().get(HttpEncoding.CONTENT_TYPE);

		MediaType requestContentType = null;
		if (contentTypes != null && !contentTypes.isEmpty()) {
			String type = contentTypes.iterator().next();
			requestContentType = MediaType.valueOf(type);
		}

		if (isMultipartType(requestContentType)) {
			this.springFormEncoder.encode(requestBody, bodyType, request);
			return;
		}
		else {
			if (bodyType == MultipartFile.class) {
				log.warn("For MultipartFile to be handled correctly, the 'consumes' parameter of @RequestMapping "
						+ "should be specified as MediaType.MULTIPART_FORM_DATA_VALUE");
			}
		}
		encodeWithMessageConverter(requestBody, bodyType, request, requestContentType);
	}
}

In the new version I end up the same place as prasadpr1 mentioned.

if (isFormRelatedContentType(requestContentType)) {
springFormEncoder.encode(requestBody, bodyType, request);
return;
}

And this springFormEncoder are not so happy about the LinkMultiValueMap ;-)

Ok, seems like if changing the multilvaluemap<> to a regular map<> then it at least works for me.
But again not sure if this was the intension with the change. ? ;-)

@tai6651
Copy link

tai6651 commented Jun 2, 2022

Hi, Sorry for digging this issue up again. I also faced that exact same issue once I upgrade spring boot from version 2.1.x to 2.6.x
I already did an investigation and found out that actual root cause is from FormEncoding class in OpenFeign not by SpringBoot it self.
refer to one section of FormEncoder.java code https://github.com/OpenFeign/feign-form/blob/master/feign-form/src/main/java/feign/form/FormEncoder.java

    Map<String, Object> data;
    if (MAP_STRING_WILDCARD.equals(bodyType)) {
      data = (Map<String, Object>) object;
    } else if (isUserPojo(bodyType)) {
      data = toMap(object);
    } else {
      delegate.encode(object, bodyType, template);
      return;
    }

    val charset = getCharset(contentTypeValue);
    processors.get(contentType).process(template, charset, data);
  

The way FormEncoder check for actual type of "bodyType" seems wrong, they are using MAP_STRING_WILDCARD.equals(bodyType)
This is exact comparison , it suppose to be compare using isAssignableFor technique (which is a bit complex when dealing with Generics Type) . If you pass MultiValueMap<String, Object> even it should be part of Map<String, Object> , That equal still return "false" causing it fall into data = toMap(object); section instead of simple data = (Map<String, Object>) object;

@DidierLoiseau
Copy link
Contributor

Following @tai6651, I found OpenFeign/feign-form/issues/109 which appears to be the cause of this issue.

@OlgaMaciaszek OlgaMaciaszek reopened this Nov 7, 2022
@OlgaMaciaszek
Copy link
Collaborator

Yes, seems like an upstream issue. Closing in favour of OpenFeign/feign-form#109.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants