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

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

Open
srihakum opened this issue Apr 20, 2017 · 10 comments

Comments

@srihakum
Copy link

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 #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);
});
@webron
Copy link
Contributor

webron commented Apr 20, 2017

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.

@srihakum
Copy link
Author

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

@kwv
Copy link

kwv commented May 10, 2017

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.

@webron
Copy link
Contributor

webron commented May 15, 2017

@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?

@kwv
Copy link

kwv commented May 16, 2017

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.

@webron
Copy link
Contributor

webron commented May 16, 2017

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.

@marjan-georgiev
Copy link

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.

@shockey
Copy link
Contributor

shockey commented Jul 14, 2017

@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?

@char0n
Copy link
Member

char0n commented Apr 29, 2020

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:

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

with

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

@char0n char0n self-assigned this Jul 2, 2020
@char0n char0n added this to the M5 milestone Jul 2, 2020
@frantuma frantuma modified the milestones: M5, M7 Aug 14, 2020
@frantuma frantuma modified the milestones: M7, M9 Sep 30, 2020
@frantuma frantuma modified the milestones: M9, M11 Oct 21, 2020
@char0n char0n removed their assignment Dec 14, 2021
@cjenkscybercom
Copy link

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.

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

8 participants