diff --git a/lib/StripeResource.js b/lib/StripeResource.js index cf80b28426..f56bb9bc72 100644 --- a/lib/StripeResource.js +++ b/lib/StripeResource.js @@ -69,18 +69,34 @@ StripeResource.prototype = { validateRequest: null, createFullPath(commandPath, urlData) { - return this._joinUrlParts([ - this.basePath(urlData), - this.path(urlData), - typeof commandPath == 'function' ? commandPath(urlData) : commandPath, - ]); + const urlParts = [this.basePath(urlData), this.path(urlData)]; + + if (typeof commandPath === 'function') { + const computedCommandPath = commandPath(urlData); + // If we have no actual command path, we just omit it to avoid adding a + // trailing slash. This is important for top-level listing requests, which + // do not have a command path. + if (computedCommandPath) { + urlParts.push(computedCommandPath); + } + } else { + urlParts.push(commandPath); + } + + return this._joinUrlParts(urlParts); }, // Creates a relative resource path with symbols left in (unlike // createFullPath which takes some data to replace them with). For example it // might produce: /invoices/{id} createResourcePathWithSymbols(pathWithSymbols) { - return `/${this._joinUrlParts([this.resourcePath, pathWithSymbols || ''])}`; + // If there is no path beyond the resource path, we want to produce just + // / rather than //. + if (pathWithSymbols) { + return `/${this._joinUrlParts([this.resourcePath, pathWithSymbols])}`; + } else { + return `/${this.resourcePath}`; + } }, _joinUrlParts(parts) { @@ -88,11 +104,7 @@ StripeResource.prototype = { // path.join, which would do this as well. Unfortunately we need to do this // as the functions for creating paths are technically part of the public // interface and so we need to preserve backwards compatibility. - const path = parts.join('/').replace(/\/{2,}/g, '/'); - - // If the path ends with a /, we preserve the behavior of path.join and - // strip off the trailing / (eg. /v1/customers/ -> /v1/customers). - return path.endsWith('/') ? path.slice(0, -1) : path; + return parts.join('/').replace(/\/{2,}/g, '/'); }, // DEPRECATED: Here for backcompat in case users relied on this. diff --git a/lib/makeRequest.js b/lib/makeRequest.js index f7844e6a2b..7ea0737b73 100644 --- a/lib/makeRequest.js +++ b/lib/makeRequest.js @@ -52,6 +52,7 @@ function getRequestOpts(self, requestArgs, spec, overrideData) { const requestPath = isUsingFullPath ? commandPath(urlData) : self.createFullPath(commandPath, urlData); + const headers = Object.assign(options.headers, spec.headers); if (spec.validator) { diff --git a/test/StripeResource.spec.js b/test/StripeResource.spec.js index 483f293ac7..78b999aedb 100644 --- a/test/StripeResource.spec.js +++ b/test/StripeResource.spec.js @@ -22,12 +22,19 @@ const { describe('StripeResource', () => { describe('createResourcePathWithSymbols', () => { - it('Generates a path', () => { + it('Generates a path when there is a symbol', () => { stripe.invoices.create({}); const path = stripe.invoices.createResourcePathWithSymbols('{id}'); expect(path).to.equal('/invoices/{id}'); }); + it('Generates a path when there is nothing beyond the resource path', () => { + stripe.invoices.create({}); + const path = stripe.invoices.createResourcePathWithSymbols(''); + // This explicitly shouldn't have a trailing slash. + expect(path).to.equal('/invoices'); + }); + it('Handles accidental double slashes', () => { stripe.invoices.create({}); const path = stripe.invoices.createResourcePathWithSymbols('/{id}'); @@ -36,9 +43,9 @@ describe('StripeResource', () => { }); describe('_joinUrlParts', () => { - it('handles trailing empty values correctly', () => { + it('includes trailing empty values', () => { const path = stripe.invoices._joinUrlParts(['a', '']); - expect(path).to.equal('a'); + expect(path).to.equal('a/'); }); it('joins parts', () => { @@ -132,6 +139,72 @@ describe('StripeResource', () => { }); }); + it('handles .. as a query param', (done) => { + const scope = nock(`https://${stripe.getConstant('DEFAULT_HOST')}`) + .get('/v1/customers/..', '') + .reply(200, '{}'); + + realStripe.customers.retrieve('..', (err, response) => { + done(err); + scope.done(); + }); + }); + + it('handles empty string as a query param', (done) => { + const scope = nock(`https://${stripe.getConstant('DEFAULT_HOST')}`) + // Note this should always have a trailing space to avoid calling the + // top level list endpoint (/v1/customers) and returning all customers. + .get('/v1/customers/', '') + .reply(200, '{}'); + + realStripe.customers.retrieve('', (err, response) => { + done(err); + scope.done(); + }); + }); + + it('handles empty string as a query param for namespaced resources', (done) => { + const scope = nock(`https://${stripe.getConstant('DEFAULT_HOST')}`) + // Note this should always have a trailing space to avoid calling the + // top level list endpoint (/v1/customers) and returning all customers. + .get('/v1/checkout/sessions/', '') + .reply(200, '{}'); + + realStripe.checkout.sessions.retrieve('', (err, response) => { + done(err); + scope.done(); + }); + }); + + it('handles empty string as a query param for nested resources', (done) => { + const scope = nock(`https://${stripe.getConstant('DEFAULT_HOST')}`) + // Note this should always have a trailing space to avoid calling the + // top level list endpoint (/v1/customers) and returning all customers. + .get('/v1/customers/cus_123/balance_transactions/', '') + .reply(200, '{}'); + + realStripe.customers.retrieveBalanceTransaction( + 'cus_123', + '', + (err, response) => { + done(err); + scope.done(); + } + ); + }); + + it('does not include trailing slash for endpoints without query parameters', (done) => { + const scope = nock(`https://${stripe.getConstant('DEFAULT_HOST')}`) + // Note that no trailing slash is present. + .get('/v1/customers', '') + .reply(200, '{}'); + + realStripe.customers.list((err, response) => { + done(err); + scope.done(); + }); + }); + it('works correctly with undefined optional arguments', (done) => { const scope = nock(`https://${stripe.getConstant('DEFAULT_HOST')}`) .get('/v1/accounts/acct_123')