swagger-js icon indicating copy to clipboard operation
swagger-js copied to clipboard

Reopen 1010: Default request Content-Type header and parameter in body as object

Open srihakum opened this issue 7 years ago • 10 comments

I do not have permission to reopen 1010 hence new issue. Sorry for duplication. With the latest fix it seems the consumes is being used to fill up Content-Type, However if the spec does not have consumes it still remains the same. In 2.x it was getting defaulted to application/json. The body serialisations issue also still exists.

This issue is in further reference to https://github.com/swagger-api/swagger-js/issues/991

With version 3.0.5 a swagger client instance tries to make an API request the following issues are observed

  1. Content-Type header is not added in the request.
  2. If a parameter used in HTTP body is added to the request the client expects it to be in the string format.

However while using 2.0.32 there is no need to set these explicitly. So is there a way we can get back the old behaviour with latest version, like adding "application/json" as default content type and the adding body parameter as JSON object rather than string.

Code example:

var Swagger = require('swagger-client');
new Swagger({
    "url":"https://staging.cloud-elements.com/elements/api-v2/elements/44/docs"
})
    .then(function (client) {
        client.apis.zohocrm.createAccount({"Authorization":"...","body":{"Account Name":"XYZ1"}})
            .then(function(response){
                console.log(response);
            })
            .catch(function (error) {
                console.log(error);
            });
    }).catch(function (error) {
    console.log(error);
});

srihakum avatar Apr 20 '17 11:04 srihakum

Thanks, @srihakum. For future reference, if you feel a ticket was wrongly closed, just add a comment to it and we'll reopen it.

#1010 does mention that there are two separate tickets covering it, which are not included in 3.0.5. In fact, the one related to the headers was resolved only yesterday, so it's not published yet.

If you'd like to try it, clone the repository, build it locally, and use it in your app. We have a release slated for tomorrow, if you'd rather wait, but if you can spare the time to test it and validate it before we publish it, we'd appreciate it.

webron avatar Apr 20 '17 11:04 webron

@webron I just tried today with 3.0.0.7 but it this issue is not fixed.

srihakum avatar Apr 25 '17 09:04 srihakum

I'm running into the same issue. Given the spec consumes application/json I would expect an empty post to honor that.

When calling using v3 of swagger-js client.apis.user.remoteConnectUserDeviceBySerial( { 'serial': '1234'] }) it maps the request to: /user/devices/{serial}/remoteConnect

and fails:

response:
   { ok: false,
     url: 'http://localhost:7100/api/v1/user/devices/1234/remoteConnect',
     status: 400,
     statusText: 'Bad Request',
     headers:
      { 'x-powered-by': 'Express',
        'content-type': 'application/json',
        date: 'Wed, 10 May 2017 15:34:17 GMT',
        connection: 'close',
        'content-length': '184' },
     text: '{"message":"Validation errors","errors":[{"code":"INVALID_CONTENT_TYPE","message":"Invalid Content-Type (application/octet-stream).  These are supported: application/json","path":[]}]}',
     data: '{"message":"Validation errors","errors":[{"code":"INVALID_CONTENT_TYPE","message":"Invalid Content-Type (application/octet-stream).  These are supported: application/json","path":[]}]}',
     body: { message: 'Validation errors', errors: [Object] },
     obj: { message: 'Validation errors', errors: [Object] } } }

I presume that the change is near this line. If there is no (req.body || req.form) shouldn't content type still align with what spec.consumes ?

I haven't figured out how to override the content-type on a request.

kwv avatar May 10 '17 15:05 kwv

@kwv just looked the spec and issue you described - you don't even have a body parameter? In that case, regardless of the http verb, there's really no reason to send the Content-Type header. Am I missing something?

webron avatar May 15 '17 18:05 webron

Digging into this more, the server response is expected given the 3.0.9 implementation. Per S.O. on RFC-7231 when there is no content-type provided application/octet-stream may be inferred.

That swagger-js serializes the POST data as a URL parameter is an implementation detail.

Swagger-js end users can't specify that data should be passed a body parameter. Nor can they explicitly set content-type. That is the convenience of a HTTP framework like Swagger, it merely accepts the request data and handles the details in accordance to the spec.

The consumes attribute of the spec explicitly sets a global default. Per http://swagger.io/specification/

A list of MIME types the APIs can consume. This is global to all APIs but can be overridden on specific API calls.)

I wouldn't disagree that the specific operation should have a more precise consumes statement. However if a global content-type is specified, swagger-js should use that by default.

From the example code here, 2.x of swagger-js behaved differently.

kwv avatar May 16 '17 13:05 kwv

Thanks for the reply, but I still don't understand why you would expect or need to send the Content-Type when there's no payload.

As you said, you don't need to specify what's a body parameter and what's not, because swagger-js knows it from the spec. From the sound of what you describe, the current implementation around this is correct. Like you said, it accepts the request data, and handles the details in accordance to the spec. And if there's no payload, there's no need to send the header.

webron avatar May 16 '17 17:05 webron

I'm not sure if I'm doing anything wrong, but since I upgraded to version 3 all of my POST requests default to content-type:text/plain;charset=UTF-8, instead of application/json. The ability to override content-type would be very useful in this case.

marjan-georgiev avatar Jul 12 '17 08:07 marjan-georgiev

@marjan-georgiev, from what you describe, I think overriding the content-type would be more of a workaround than a fix of the root problem. Can you share your spec so we can reproduce the issue you're having?

shockey avatar Jul 14 '17 20:07 shockey

This problem is already partially addressed by requestContentType and attachContentTypeForEmptyPayload options. The TryItOut Exeuctor already supports what has been requested in this issue, when called as a static method from SwaggerClient class.

SwaggerClient.execute({ ...});
SwaggerClient.buildRequest({ ... });

When working with Tags Interface, this functionality doesn't work, because appropriate options are not properly passed.

In order to make this work, we have to replace this line: https://github.com/swagger-api/swagger-js/blob/7cec49c53db1974d07a7a8a18b89644ce034f4a1/src/interfaces.js#L21

with

...pick(swaggerJs, 'requestInterceptor', 'attachContentTypeForEmptyPayload', 'responseInterceptor', 'userFetch'),

char0n avatar Apr 29 '20 15:04 char0n

Tags Interface

Can confirm we also use the Tags feature and the attachContentTypeForEmptyPayload option doesn't seem to work so we need to override / workaround the problem with request interceptors for any POST requests having an empty body.

cjenkscybercom avatar Sep 25 '23 13:09 cjenkscybercom