Azure / autorest

OpenAPI (f.k.a Swagger) Specification code generator. Supports C#, PowerShell, Go, Java, Node.js, TypeScript, Python
MIT License
4.62k stars 739 forks source link

Code for multi-part form is generated for operation that consumes “application/x-www-form-urlencoded”. #2456

Closed CNBoland closed 7 years ago

CNBoland commented 7 years ago

Summary Generated code for an operation that consumes a single url-encoded form sends a multi-part form instead.

Using https://github.com/Azure/autorest/blob/27aaa35b6de55a7955a36e42e90be6cf9f35a28d/Samples/1a-code-generation-minimal/p%C3%A9tst%C3%B6re.json, the service definition contains the following segment that describes input for the UpdatePetWithForm operation:

. . .
        "operationId": "updatePetWithForm",
        "consumes": [
          "application/x-www-form-urlencoded"
        ],
. . .

Definition is transpiled with AutoRest 1.2.2 with the following command:

autorest --version=1.2.2 --namespace=PetStore --input-file=petstore.json --output-folder=PetStoreClient –csharp

The code generated for this operation (SwaggerPetstore.UpdatePetWithFormWithHttpMessagesAsync) contains the following:

        MultipartFormDataContent _multiPartContent = new MultipartFormDataContent();
        if (name != null)
        {
            StringContent _name = new StringContent(name, System.Text.Encoding.UTF8);
            _multiPartContent.Add(_name, "name");
       }
        if (status != null)
        {
            StringContent _status = new StringContent(status, System.Text.Encoding.UTF8);
            _multiPartContent.Add(_status, "status");
        }
        _httpRequest.Content = _multiPartContent;

I believe this to be incorrect. With content type application/x-www-form-urlencoded specified, the generated code should use FormUrlEncodedContent instead and look similar to this:

        List<KeyValuePair<string, string>> values = new List<KeyValuePair<string, string>>();
        if (name != null)
        {
            values.Add(new KeyValuePair<string, string>("name", name));
        }
        if (status != null)
        {
            values.Add(new KeyValuePair<string, string>("status", status));
        }
        FormUrlEncodedContent _formContent = new FormUrlEncodedContent(values);
        _httpRequest.Content = _formContent;
CNBoland commented 7 years ago

The post above is an example that replicates a problem I ran into while generating client code to call a token endpoint on a web application that uses OAuth Bearer Token authentication from the Microsoft.AspNet.Identity.Owin v2.2.1 package. The endpoint expects a single url-encoded form and does not work with the multi-part form code generated by AutoRest. I modified the generated code to use FormUrlEncodedContent as described above instead and it works correctly.

olydis commented 7 years ago

Thanks for filing this! Will investigate.

sboulema commented 7 years ago

Also affected by this issue :(

dsgouda commented 7 years ago

Looks like we support streams for multipartcontent. For now, won't extend that feature for simple formdata.

dsgouda commented 7 years ago

Addressed by the merged PR

Xample commented 5 years ago

The problems seem to come from the client, not the autorest generator. A solution is to provide to your generated service (here below GeneratedAPI) another client instead of the default XhrHttpClient.

private httpClient: XhrHttpClient = new AxiosHttpClient();
const service: GeneratedAPI = new GeneratedAPI({ httpClient });
dsgouda commented 5 years ago

@fearthecowboy could you take a look

CNBoland commented 5 years ago

@Xample, Returning to the original post above, I'd argue that AutoRest generating code to send multi-part form data to a service that states it consumes application/x-www-form-urlencoded isn't honoring the contract described in the service's Swagger definition.

Xample commented 5 years ago

I solved my problem using the AxiosHttpClient. I am not at work right now, but I can imagine the client to hold the responsibility of formatting properly the headers and payload content. If it was not the case, using another client would produce the same issue (i.e: My service in swagger is specifying application/x-www-form-urlencoded while the request sent was a multiform)

Xample commented 5 years ago

@CNBoland I double checked, actually…

My swagger:

yaml
      produces:
        - application/json
      consumes:
        - application/x-www-form-urlencoded

The header sent:

Provisional headers are shown
Accept: application/json, text/plain, */*
authorization: Basic BwZtSsdeIdMKSKmDMLSAKWDM=
Content-Type: multipart/form-data; boundary=----WebKitFormBoundaryfGpyXd5hUAp2INlB
Origin: http://localhost:8100
Referer: http://localhost:8100/
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.121 Safari/537.36

Meaning, the application/x-www-form-urlencoded is converted to multipart/form-data; boundary=----WebKitFormBoundaryfGpyXd5hUAp2INlB at some point.

here is the generated code

const createRequestOperationSpec: msRest.OperationSpec = {
  httpMethod: "POST",
  path: "api/request/",
  queryParameters: [
    Parameters.accessToken0
  ],
  formDataParameters: [
    Parameters.mobilisId1,
    Parameters.userSwisspassCkm1,
    Parameters.subject,
    Parameters.body,
    Parameters.contactReasonId,
    Parameters.line,
    Parameters.source
  ],
  contentType: "application/x-www-form-urlencoded",
  responses: {
    200: {
      bodyMapper: Mappers.Status
    },
    400: {},
    default: {}
  },
  serializer
};

Here is the header sent to Axios

authorization: {name: "authorization", value: "Basic BwZtSsdeIdMKSKmDMLSAKWDM="}
content-type: {name: "content-type", value: "application/x-www-form-urlencoded"}

[edit] Ok I think I finally got it… Xhr has the "good" idea to clear and let the browser manage the content-type, reason why after being deleted, the browser did then replace the x-www-form-urlencoded with multipart/form-data

    if (utils.isFormData(requestData)) {
      delete requestHeaders['Content-Type']; // Let the browser set it
    }
Xample commented 5 years ago

Here is the way to force Axios sending x-www-form-urlencoded requests. Meaning, the expected data content should be URLSearchParameters and not a key-value form data.

I fixed this adding the following snippet right after this line

      if (contentType && contentType.indexOf('application/x-www-form-urlencoded') !== -1)
      {
        if (httpRequest.body instanceof FormData) {
          const formParams: URLSearchParams = new URLSearchParams();
          const entries = Array.from(httpRequest.body.entries());
          entries.forEach(([name, value]) => formParams.append(name, value));
          httpRequest.body = formParams;
          }
      }

Note that URLSearchParams will need a shim on some browsers

Could someone from the team review and create a merge request for it? The last time a member of my team tried to fork the project he had problems with dependencies when building the lib.

[EDIT]: I modified the snipped to avoid possible issues with the loop on an iterable.

Xample commented 5 years ago

@CNBoland any feedback?

CNBoland commented 5 years ago

@Xample I'm using the CSharp generator and a fix was merged in response to this issue (see above). It looks like you are using a TypeScript client. I generated a TypeScript client to see what Autorest generates. As of v2.0.4283, it generates the snippet below. I haven't run it, but it appears to account fo application/x-www-form-urlencoded content.

const postOperationSpec: msRest.OperationSpec = {
  httpMethod: "POST",
  path: "token",
  formDataParameters: [
    Parameters.grantType
  ],
  contentType: "application/x-www-form-urlencoded",
  responses: {
    200: {
      bodyMapper: Mappers.TokenResponse
    },
    default: {}
  },
  serializer
};
Xample commented 5 years ago

Mmm… weird, the default XHR client used by autorest had also the problem, it was even worse as all the data was encoded for a multipart form. …and the Axios wrapper is still buggy without the fix I suggested. The reason to go to Axios was also that I had other constraints using the regular client.