Encode undefined parameters as the empty string

[exocode]

Describe the bug

If - for any reason - the node / js app having a undefined customer parameter, all subscriptions are leaked to the user.

I totally understand that
stripe.subscriptions.list() is providing a complete list, but
stripe.subscriptions.list({ customer: undefined }) or null or similar (or anything else being not a valid ID)

should not expose the complete subscriptionsList to the customer.

Sure, the app should check that too, but you'll never know, somehow it can happen, that an invalid customer string was provided (a typo,, but espacially providing a {customer: 'whatEverYouWant'} parameter should not return a complete list.

It should return an error message like: "No customer with that ID found". Or "This is a non-existing customer" or "invalid customer ID given".

But it definitely should NOT exposing ALL SUBSCRIPTIONS of a specific Stripe account

To Reproduce

import Stripe from 'stripe';

        const stripe = new Stripe(process.env.STRIPE_API_SECRET!, { apiVersion: '2020-08-27' });
        const stripeSubscriptions = await stripe.subscriptions.list({ customer: "typeWhatEverYouWant" }).then(subscriptions => {
            console.log(subscriptions) // <<<---- this will return ALL SUBSCRIPTIONS of a Stripe account.
        }).catch(err => {
            return { error: err.message }
        }

If session.stripe_id is undefined then Stripe Api is not complaining, it just will return all subscriptions of any customer

Expected behavior

stripe.subscriptions.list({ customer: "undefined_or_non_existing_customer_id"})

• should return an error
• or empty list
• BUT should not expose the whole list of subscription when accidential no customer id provided.

Code snippets

No response

OS

macOS

Node version

v14.17.0

Library version

react-stripe-js": "^1.7.1", stripe-js": "^1.32.0"

API version

2020-08-27

Additional context

No response

[richardm-stripe]

Hello @exocode, thank you for writing in and bringing this to our attention. I'm sorry that this happened and I agree that the design of stripe.subscriptions.list could have done more to prevent it.

I don't think this is something we can or should fix in stripe-node itself. Right now, the way the library works is that stripe-node will transmit any parameters that you provide without validation, i.e. in this case it sends GET /v1/subscriptions?customer=. All validation happens on the server. Our answer for SDK-level validation is the Typescript definitions. In this case the Typescript definitions could have caught the error, but only if strictNullChecks is enabled in the Typescript config, which unfortunately it isn't by default.

It would also be possible in theory to attempt to prevent this situation at the API level, rather than on the SDK. The API could have been designed so that stripe.subscriptions.listByCustomer and stripe.subscriptions.listAll were different methods entirely. Or perhaps the server could produce an error in the case of ?customer=. Unfortunately, that represents a breaking API change, which Stripe heavily tries to avoid because of the potential for disruption.

The github issues here are for tracking bugs in stripe-node, and so I'm going to close this ticket as I think it mostly reflects a shortcoming of the Stripe API and not something that we can fix in stripe-node given the current parameters of the library. I am going to surface your feedback internally to the teams who maintain this API, but I recommend that you also log your issue with Stripe Support which is the official place to report issues with the API.

Feel free to reopen if you disagree and think the fix should live in the SDK. Possibly we can reimagine this ticket as a more general feature request for SDK validation beyond Typescript.

[richardm-stripe]

Hi @exocode,

It turns out I diagnosed this incorrectly:

stripe.subscriptions.list({ customer: null }) will send ?customers= to the API and the API will 400 in this case and trigger an error, but stripe.subscriptions.list({ customer: undefined }) won't do this. This is something we could conceivably change, so I'm going to leave this issue open as a feature request to start distinguishing between undefined and truly missing properties, and encoding undefined as the empty string.

This would be a breaking and potentially highly dangerous change and would disrupt any users whose integrations unknowingly pass undefined and encounter/expect this to have no effect, so I don't think this is anything we could tackle even in a major version, but we might be able to phase it in somehow for net new interfaces we added to this library. We did something similar when we added a new paradigm to stripe-php.

[pakrym-stripe]

I do not think we should change this behavior. Undefines in JavaScript are confusing enough.

Having {} and {prop: undefined} have a different meaning will increase confusion even more.