From 78a287fc8cd8f52335169b326f59574916d6730d Mon Sep 17 00:00:00 2001 From: Nadeem Patwekar Date: Tue, 25 Nov 2025 16:56:59 +0530 Subject: [PATCH 1/3] feat: enhance URL handling in request module to support absolute URLs and improve query parameter serialization --- src/lib/request.ts | 16 +++- test/request.spec.ts | 213 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 197 insertions(+), 32 deletions(-) diff --git a/src/lib/request.ts b/src/lib/request.ts index 13e35df..a40ccfb 100644 --- a/src/lib/request.ts +++ b/src/lib/request.ts @@ -8,6 +8,7 @@ import { APIError } from './api-error'; */ function serializeParams(params: any): string { if (!params) return ''; + return serialize(params); } @@ -15,14 +16,23 @@ function serializeParams(params: any): string { * Builds the full URL with query parameters */ function buildFullUrl(baseURL: string | undefined, url: string, queryString: string): string { + if (url.startsWith('http://') || url.startsWith('https://')) { + return `${url}?${queryString}`; + } const base = baseURL || ''; + return `${base}${url}?${queryString}`; } /** * Makes the HTTP request with proper URL handling */ -async function makeRequest(instance: AxiosInstance, url: string, requestConfig: any, actualFullUrl: string): Promise { +async function makeRequest( + instance: AxiosInstance, + url: string, + requestConfig: any, + actualFullUrl: string +): Promise { // If URL is too long, use direct axios request with full URL if (actualFullUrl.length > 2000) { return await instance.request({ @@ -69,11 +79,11 @@ export async function getData(instance: AxiosInstance, url: string, data?: any) } } } - + const requestConfig = { ...data, maxContentLength: Infinity, - maxBodyLength: Infinity + maxBodyLength: Infinity, }; const queryString = serializeParams(requestConfig.params); const actualFullUrl = buildFullUrl(instance.defaults.baseURL, url, queryString); diff --git a/test/request.spec.ts b/test/request.spec.ts index 1214497..fb5f0f4 100644 --- a/test/request.spec.ts +++ b/test/request.spec.ts @@ -43,7 +43,7 @@ describe('Request tests', () => { const mock = new MockAdapter(client as any); const url = '/your-api-endpoint'; const mockResponse = { data: 'mocked' }; - + client.stackConfig = { live_preview: { enable: true, @@ -51,13 +51,13 @@ describe('Request tests', () => { live_preview: 'init', }, }; - + mock.onGet(url).reply(200, mockResponse); - + const result = await getData(client, url, {}); expect(result).toEqual(mockResponse); }); - + it('should set baseURL correctly when host is provided without https://', async () => { const client = httpClient({ defaultHostname: 'example.com', @@ -66,7 +66,7 @@ describe('Request tests', () => { const url = '/your-api-endpoint'; const livePreviewURL = 'https://rest-preview.com' + url; const mockResponse = { data: 'mocked' }; - + client.stackConfig = { live_preview: { enable: true, @@ -75,16 +75,16 @@ describe('Request tests', () => { host: 'rest-preview.com', }, }; - + mock.onGet(livePreviewURL).reply(200, mockResponse); - + const result = await getData(client, url, {}); expect(client.defaults.baseURL).toBe('https://example.com:443/v3'); expect(client.stackConfig.live_preview.host).toBe('rest-preview.com'); expect(mock.history.get[0].url).toBe(livePreviewURL); expect(result).toEqual(mockResponse); }); - + it('should not modify baseURL when host is already prefixed with https://', async () => { const client = httpClient({ defaultHostname: 'example.com', @@ -93,7 +93,7 @@ describe('Request tests', () => { const url = '/your-api-endpoint'; const livePreviewURL = 'https://rest-preview.com' + url; const mockResponse = { data: 'mocked' }; - + client.stackConfig = { live_preview: { enable: true, @@ -102,9 +102,9 @@ describe('Request tests', () => { host: 'https://rest-preview.com', }, }; - + mock.onGet(livePreviewURL).reply(200, mockResponse); - + const result = await getData(client, url, {}); expect(client.defaults.baseURL).toBe('https://example.com:443/v3'); expect(client.stackConfig.live_preview.host).toBe('https://rest-preview.com'); @@ -120,7 +120,7 @@ describe('Request tests', () => { // Mock response that returns undefined/empty data mock.onGet(url).reply(() => [200, undefined, {}]); - await expect(getData(client, url)).rejects.toBeDefined(); + await expect(getData(client, url)).rejects.toBeDefined(); }); it('should throw error when response is null', async () => { @@ -131,7 +131,7 @@ describe('Request tests', () => { // Mock response that returns null mock.onGet(url).reply(() => [200, null]); - await expect(getData(client, url)).rejects.toBeDefined(); + await expect(getData(client, url)).rejects.toBeDefined(); }); it('should handle live_preview when enable is false', async () => { @@ -152,7 +152,7 @@ describe('Request tests', () => { mock.onGet(url).reply(200, mockResponse); const result = await getData(client, url, {}); - + // Should not modify URL when live preview is disabled expect(mock.history.get[0].url).toBe(url); expect(result).toEqual(mockResponse); @@ -208,7 +208,7 @@ describe('Request tests', () => { const data: any = {}; const result = await getData(client, url, data); - + // Should set live_preview to 'init' expect(data.live_preview).toBe('init'); expect(result).toEqual(mockResponse); @@ -231,7 +231,7 @@ describe('Request tests', () => { mock.onGet(url).reply(200, mockResponse); const result = await getData(client, url, {}); - + // Should set headers expect(client.defaults.headers.preview_token).toBe('test-preview-token'); expect(client.defaults.headers.live_preview).toBe('init'); @@ -256,7 +256,7 @@ describe('Request tests', () => { const data: any = {}; const result = await getData(client, url, data); - + // Should still set live_preview in data expect(data.live_preview).toBe('init'); expect(result).toEqual(mockResponse); @@ -286,7 +286,7 @@ describe('Request tests', () => { }); // When error has message property, it uses the message - await expect(getData(client, url)).rejects.toBeDefined(); + await expect(getData(client, url)).rejects.toBeDefined(); }); it('should handle non-Error objects as errors when they have no message property', async () => { @@ -300,7 +300,7 @@ describe('Request tests', () => { }); // When error has no message property, it stringifies the object - await expect(getData(client, url)).rejects.toBeDefined(); + await expect(getData(client, url)).rejects.toBeDefined(); }); it('should pass data parameter to axios get request', async () => { @@ -313,6 +313,7 @@ describe('Request tests', () => { mock.onGet(url).reply((config) => { // Verify that data was passed correctly expect(config.params).toEqual(requestData.params); + return [200, mockResponse]; }); @@ -325,17 +326,17 @@ describe('Request tests', () => { const mock = new MockAdapter(client as any); const url = '/your-api-endpoint'; const mockResponse = { data: 'mocked' }; - const requestData = { - params: { - limit: 10, - skip: 0, + const requestData = { + params: { + limit: 10, + skip: 0, include: ['field1', 'field2'], query: { title: 'test' }, nullValue: null, undefinedValue: undefined, emptyString: '', - specialChars: 'hello world & more' - } + specialChars: 'hello world & more', + }, }; mock.onGet(url).reply(200, mockResponse); @@ -349,18 +350,18 @@ describe('Request tests', () => { const client = httpClient({ defaultHostname: 'example.com' }); const url = '/your-api-endpoint'; const mockResponse = { data: 'mocked' }; - + // Create a very long query parameter that will exceed 2000 characters when combined with baseURL // baseURL is typically like 'https://example.com:443/v3' (~30 chars), url is '/your-api-endpoint' (~20 chars) // So we need params that serialize to >1950 chars to exceed 2000 total const longParam = 'x'.repeat(2000); const requestData = { params: { longParam, param2: 'y'.repeat(500) } }; - + // Mock instance.request since that's what gets called for long URLs const requestSpy = jest.spyOn(client, 'request').mockResolvedValue({ data: mockResponse } as any); const result = await getData(client, url, requestData); - + expect(result).toEqual(mockResponse); // Verify that request was called (not get) with the full URL expect(requestSpy).toHaveBeenCalledWith( @@ -371,7 +372,161 @@ describe('Request tests', () => { maxBodyLength: Infinity, }) ); - + requestSpy.mockRestore(); }); + + describe('Absolute URL handling', () => { + it('should not concatenate baseURL when absolute https:// URL is passed', async () => { + const client = httpClient({ + defaultHostname: 'example.com', + }); + const mock = new MockAdapter(client as any); + const absoluteUrl = 'https://external-api.com/api/endpoint'; + const mockResponse = { data: 'mocked' }; + + mock.onGet(absoluteUrl).reply(200, mockResponse); + + const result = await getData(client, absoluteUrl, {}); + + expect(result).toEqual(mockResponse); + // Verify that the absolute URL was used as-is, not concatenated with baseURL + expect(mock.history.get[0].url).toBe(absoluteUrl); + }); + + it('should not concatenate baseURL when absolute http:// URL is passed', async () => { + const client = httpClient({ + defaultHostname: 'example.com', + }); + const mock = new MockAdapter(client as any); + const absoluteUrl = 'http://external-api.com/api/endpoint'; + const mockResponse = { data: 'mocked' }; + + mock.onGet(absoluteUrl).reply(200, mockResponse); + + const result = await getData(client, absoluteUrl, {}); + + expect(result).toEqual(mockResponse); + // Verify that the absolute URL was used as-is + expect(mock.history.get[0].url).toBe(absoluteUrl); + }); + + it('should still concatenate baseURL when relative URL is passed', async () => { + const client = httpClient({ + defaultHostname: 'example.com', + }); + const mock = new MockAdapter(client as any); + const relativeUrl = '/api/endpoint'; + const mockResponse = { data: 'mocked' }; + + mock.onGet(relativeUrl).reply(200, mockResponse); + + const result = await getData(client, relativeUrl, {}); + + expect(result).toEqual(mockResponse); + // Verify that relative URL was used (Axios will combine with baseURL) + expect(mock.history.get[0].url).toBe(relativeUrl); + expect(client.defaults.baseURL).toBe('https://example.com:443/v3'); + }); + + it('should handle absolute URL with query parameters correctly', async () => { + const client = httpClient({ + defaultHostname: 'example.com', + }); + const mock = new MockAdapter(client as any); + const absoluteUrl = 'https://external-api.com/api/endpoint'; + const mockResponse = { data: 'mocked' }; + const requestData = { params: { limit: 10, skip: 0 } }; + + mock.onGet(absoluteUrl).reply((config) => { + expect(config.params).toEqual(requestData.params); + + return [200, mockResponse]; + }); + + const result = await getData(client, absoluteUrl, requestData); + + expect(result).toEqual(mockResponse); + expect(mock.history.get[0].url).toBe(absoluteUrl); + }); + + it('should handle live preview with absolute URL correctly (no double baseURL)', async () => { + const client = httpClient({ + defaultHostname: 'example.com', + }); + const mock = new MockAdapter(client as any); + const relativeUrl = '/your-api-endpoint'; + const livePreviewAbsoluteUrl = 'https://rest-preview.com/your-api-endpoint'; + const mockResponse = { data: 'mocked' }; + + client.stackConfig = { + live_preview: { + enable: true, + preview_token: 'someToken', + live_preview: 'someHash', + host: 'rest-preview.com', + }, + }; + + mock.onGet(livePreviewAbsoluteUrl).reply(200, mockResponse); + + const result = await getData(client, relativeUrl, {}); + + expect(result).toEqual(mockResponse); + // Verify that the live preview absolute URL was used, not concatenated with baseURL + expect(mock.history.get[0].url).toBe(livePreviewAbsoluteUrl); + expect(client.defaults.baseURL).toBe('https://example.com:443/v3'); + }); + + it('should handle custom endpoint with absolute URL correctly', async () => { + const client = httpClient({ + endpoint: 'https://custom-api.com/v2', + }); + const mock = new MockAdapter(client as any); + const absoluteUrl = 'https://external-api.com/api/endpoint'; + const mockResponse = { data: 'mocked' }; + + mock.onGet(absoluteUrl).reply(200, mockResponse); + + const result = await getData(client, absoluteUrl, {}); + + expect(result).toEqual(mockResponse); + // Verify that absolute URL was used as-is, ignoring the custom endpoint baseURL + expect(mock.history.get[0].url).toBe(absoluteUrl); + }); + + it('should handle absolute URL when actualFullUrl exceeds 2000 characters', async () => { + const client = httpClient({ + defaultHostname: 'example.com', + }); + const absoluteUrl = 'https://external-api.com/api/endpoint'; + const mockResponse = { data: 'mocked' }; + + // Create a very long query parameter that will exceed 2000 characters + const longParam = 'x'.repeat(2000); + const requestData = { params: { longParam, param2: 'y'.repeat(500) } }; + + // Mock instance.request since that's what gets called for long URLs + const requestSpy = jest.spyOn(client, 'request').mockResolvedValue({ data: mockResponse } as any); + + const result = await getData(client, absoluteUrl, requestData); + + expect(result).toEqual(mockResponse); + // Verify that request was called with the absolute URL (not concatenated with baseURL) + expect(requestSpy).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'get', + url: expect.stringContaining('https://external-api.com/api/endpoint'), + maxContentLength: Infinity, + maxBodyLength: Infinity, + }) + ); + // Verify the URL doesn't contain the baseURL + const callUrl = requestSpy.mock.calls[0][0].url; + expect(callUrl).not.toContain('example.com:443/v3'); + expect(callUrl).toContain('external-api.com'); + + requestSpy.mockRestore(); + }); + }); }); From a602d1ed7594bd670dd29d4292346ec1a4537a8a Mon Sep 17 00:00:00 2001 From: Nadeem Patwekar Date: Tue, 25 Nov 2025 16:57:25 +0530 Subject: [PATCH 2/3] chore: version bumo --- CHANGELOG.md | 4 ++++ package.json | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 646eaec..7092c7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## Change log +### Version: 1.3.4 +#### Date: Dec-19-2024 + - Fix: Prevent baseURL concatenation when absolute URLs (http:// or https://) are passed to getData() or created by live preview, preventing malformed URLs + ### Version: 1.3.3 #### Date: Nov-10-2025 - Fix: Added 'exports' field to package.json to fix ESM import error where '@contentstack/core' does not provide an export named 'getData' in modern ESM environments (e.g., Nuxt.js, Vite) diff --git a/package.json b/package.json index cfa3194..ff32136 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@contentstack/core", - "version": "1.3.3", + "version": "1.3.4", "type": "commonjs", "main": "./dist/cjs/src/index.js", "types": "./dist/cjs/src/index.d.ts", From fd099f1a25e024d99895ef8bb170aef2f30c5938 Mon Sep 17 00:00:00 2001 From: Nadeem Patwekar Date: Tue, 25 Nov 2025 17:15:30 +0530 Subject: [PATCH 3/3] changelog correction --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7092c7e..b974dfb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ ## Change log ### Version: 1.3.4 -#### Date: Dec-19-2024 +#### Date: Nov-26-2025 - Fix: Prevent baseURL concatenation when absolute URLs (http:// or https://) are passed to getData() or created by live preview, preventing malformed URLs ### Version: 1.3.3