search

spring-cloud / spring-cloud-openfeign

Support for using OpenFeign in Spring Cloud apps
Apache License 2.0
1.21k stars 784 forks source link

SpringEncoder issue #628

Closed prasadpr1 closed 1 year ago

prasadpr1 commented 2 years ago

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 commented 2 years ago

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

prasadpr1 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Still not working.

prasadpr1 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

OlgaMaciaszek commented 1 year ago

Yes, seems like an upstream issue. Closing in favour of https://github.com/OpenFeign/feign-form/issues/109.