stripe / openapi

An OpenAPI specification for the Stripe API.
MIT License
394 stars 123 forks source link

Add "format: int64" to all integer balance fields #50

Closed phayes closed 1 year ago

phayes commented 5 years ago

Some openAPI implementations interpret undecorated integer types as unsigned uints. (This is the case in the Rust Stripe library at https://github.com/wyyerd/stripe-rs).

Because balance related fields can be negative, this PR explicitly marks them as signed by denoting their format as int64.

This has been applied to account_balance, balance, starting_balance, and ending_balance

brandur-stripe commented 5 years ago

@phayes Thank you!

A problem with fixing this via your patch is that all of these specs are generated automatically, so even if we merged it, they'd just get blown away on the next update.

One thing we did in Go is simplify things (at the possible expense of some correctness) by just making all integers signed int64s. It'd be relatively easy for me to make that change broadly to the spec on our server side — would that cover the trouble that stripe-rs is having?

phayes commented 5 years ago

Hi there!

Digging into this a bit more, and looking at the spec (https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#data-types), it appears that any implementation that interprets integer as unsigned is in error.

From the spec: "Note that integer as a type is also supported and is defined as a JSON number without a fraction or exponent part.".

I read this to mean that unformatted integer types have to be signed, which means that the OpenAPI implementation used by stripe-rs is in error and this problem is better dealt with there.

Instead, integer types that are unsigned should be marked with "minimum": 0

CLAassistant commented 4 years ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

pakrym-stripe commented 1 year ago

Closing, the usage of integer seems to be inline with OpenAPI spec.