swagger-api / swagger-js

Javascript library to connect to swagger-enabled APIs via browser or nodejs
http://swagger.io
Apache License 2.0
2.6k stars 755 forks source link

fix(execute): parse stringified objects nested in arrays #3466

Closed glowcloud closed 3 months ago

glowcloud commented 3 months ago

Refs https://github.com/swagger-api/swagger-ui/issues/9521

The URL for this example:

Screenshot 2024-04-10 at 14 36 22

should now be encoded as:

http://localhost:3200/endpoint?param=%5B%7B%22a%22%3A%7B%22c%22%3A%22string%22%7D%2C%22b%22%3A0%7D%5D

which should decode to:

http://localhost:3200/endpoint?param=[{"a":{"c":"string"},"b":0}]
glowcloud commented 3 months ago

We're also doing a similar thing for parameters with schema and type object before going into builders: https://github.com/swagger-api/swagger-js/blob/f72651a517268d5f21952053f1ab6ab023d92f68/src/execute/index.js#L253-L264

I was thinking of moving the parsing for arrays there as well, and I think it would have to look something like this:

if (
  specIsOAS3 &&
  parameter.content &&
  parameter.content['application/json'] &&
  parameter.content['application/json'].schema?.type === 'array' &&
  Array.isArray(value)
) {
  value = value.map((v) => {
    try {
      return JSON.parse(v);
    } catch (e) {
      return v;
    }
  });
}

so not sure if having this would actually be better.

glowcloud commented 3 months ago

I see that we can also refactor this line https://github.com/swagger-api/swagger-js/blob/5c60b7e08df2b5170d3ef72fd7c10bea4b968eb9/src/execute/oas3/content-serializer.js#L26 to

return String(value)
char0n commented 3 months ago

return String(value)

Yeah sure, it's small enough change to smuggle it with the bugfix.

char0n commented 3 months ago

I was thinking of moving the parsing for arrays there as well, and I think it would have to look something like this:

I was thinking the other way around, cannot we move the code from https://github.com/swagger-api/swagger-js/blob/f72651a517268d5f21952053f1ab6ab023d92f68/src/execute/index.js#L253-L264 to content-serializer to contain the logic there? If it's not possible, then placing your new code in https://github.com/swagger-api/swagger-js/blob/f72651a517268d5f21952053f1ab6ab023d92f68/src/execute/index.js#L253-L264 should be just fine.

glowcloud commented 3 months ago

I was thinking of moving the parsing for arrays there as well, and I think it would have to look something like this:

I was thinking the other way around, cannot we move the code from

https://github.com/swagger-api/swagger-js/blob/f72651a517268d5f21952053f1ab6ab023d92f68/src/execute/index.js#L253-L264

to content-serializer to contain the logic there? If it's not possible, then placing your new code in https://github.com/swagger-api/swagger-js/blob/f72651a517268d5f21952053f1ab6ab023d92f68/src/execute/index.js#L253-L264

should be just fine.

We can't move this to content-serializer as that's only for parameters with content, while this one is for parameters with schema. And I think that changing this to combine both logics might also create unnecessary parsing for arrays with objects in schema.

swagger-bot commented 3 months ago

:tada: This PR is included in version 3.26.8 :tada:

The release is available on:

Your semantic-release bot :package::rocket: