From a8bcc75467075ed6039197fa803b583ceeffed2e Mon Sep 17 00:00:00 2001 From: James George <25279263+jamesgeorge007@users.noreply.github.com> Date: Wed, 7 Aug 2024 09:16:27 -0700 Subject: [PATCH] fix: ensure `Content-Type` header priority in the CLI (#4242) - Ensure the `Content-Type` header takes priority over the value set in the request body. - Introduces `HoppRESTRequest` schema `v6` with `text/xml` added under the supported content types. --- packages/hoppscotch-cli/package.json | 2 +- .../src/__tests__/e2e/commands/test.spec.ts | 10 + .../content-type-header-scenarios.json | 171 ++++++++++++++++++ .../unit/fixtures/workspace-access.mock.ts | 6 +- .../hoppscotch-cli/src/utils/pre-request.ts | 10 +- packages/hoppscotch-cli/src/utils/request.ts | 26 ++- .../src/helpers/utils/contenttypes.ts | 2 + .../persistence/__tests__/__mocks__/index.ts | 12 +- .../hoppscotch-data/src/rest/content-types.ts | 5 +- packages/hoppscotch-data/src/rest/index.ts | 29 +-- packages/hoppscotch-data/src/rest/v/6.ts | 49 +++++ 11 files changed, 290 insertions(+), 32 deletions(-) create mode 100644 packages/hoppscotch-cli/src/__tests__/e2e/fixtures/collections/content-type-header-scenarios.json create mode 100644 packages/hoppscotch-data/src/rest/v/6.ts diff --git a/packages/hoppscotch-cli/package.json b/packages/hoppscotch-cli/package.json index 1f82f83b7..8eb0b6a55 100644 --- a/packages/hoppscotch-cli/package.json +++ b/packages/hoppscotch-cli/package.json @@ -1,6 +1,6 @@ { "name": "@hoppscotch/cli", - "version": "0.10.1", + "version": "0.10.2", "description": "A CLI to run Hoppscotch test scripts in CI environments.", "homepage": "https://hoppscotch.io", "type": "module", diff --git a/packages/hoppscotch-cli/src/__tests__/e2e/commands/test.spec.ts b/packages/hoppscotch-cli/src/__tests__/e2e/commands/test.spec.ts index 90affa48c..0e6b4574b 100644 --- a/packages/hoppscotch-cli/src/__tests__/e2e/commands/test.spec.ts +++ b/packages/hoppscotch-cli/src/__tests__/e2e/commands/test.spec.ts @@ -149,6 +149,16 @@ describe("hopp test [options] ", () => { expect(error).toBeNull(); }); + + test("The `Content-Type` header takes priority over the value set at the request body", async () => { + const args = `test ${getTestJsonFilePath( + "content-type-header-scenarios.json", + "collection" + )}`; + const { error } = await runCLI(args); + + expect(error).toBeNull(); + }); }); describe("Test `hopp test --env ` command:", () => { diff --git a/packages/hoppscotch-cli/src/__tests__/e2e/fixtures/collections/content-type-header-scenarios.json b/packages/hoppscotch-cli/src/__tests__/e2e/fixtures/collections/content-type-header-scenarios.json new file mode 100644 index 000000000..9aea2c0f2 --- /dev/null +++ b/packages/hoppscotch-cli/src/__tests__/e2e/fixtures/collections/content-type-header-scenarios.json @@ -0,0 +1,171 @@ +{ + "v": 2, + "name": "content-type-header-scenarios", + "folders": [], + "requests": [ + { + "v": "6", + "auth": { + "authType": "inherit", + "authActive": true + }, + "body": { + "body": "\n\n \n 12345\n John Doe\n john.doe@example.com\n \n \n 98765\n Sample Product\n 2\n \n\n", + "contentType": "text/xml" + }, + "name": "content-type-header-assignment", + "method": "POST", + "params": [], + "headers": [], + "endpoint": "https://echo.hoppscotch.io", + "testScript": "pw.test(\"The `Content-Type` header is assigned the content type value set at the request body level\", ()=> {\n pw.expect(pw.response.body.headers[\"content-type\"]).toBe(\"text/xml\");\n});", + "preRequestScript": "", + "requestVariables": [] + }, + { + "v": "6", + "auth": { + "authType": "inherit", + "authActive": true + }, + "body": { + "body": "\n\n \n 12345\n John Doe\n john.doe@example.com\n \n \n 98765\n Sample Product\n 2\n \n\n", + "contentType": "application/json" + }, + "name": "content-type-header-override", + "method": "POST", + "params": [], + "headers": [ + { + "key": "Content-Type", + "value": "application/xml", + "active": true + } + ], + "endpoint": "https://echo.hoppscotch.io", + "testScript": "pw.test(\"The `Content-Type` header overrides the content type value set at the request body level\", ()=> {\n pw.expect(pw.response.body.headers[\"content-type\"]).toBe(\"application/xml\");\n});", + "preRequestScript": "", + "requestVariables": [] + }, + { + "v": "6", + "auth": { + "authType": "inherit", + "authActive": true + }, + "body": { + "body": "\n\n \n 12345\n John Doe\n john.doe@example.com\n \n \n 98765\n Sample Product\n 2\n \n\n", + "contentType": "application/json" + }, + "name": "multiple-content-type-headers", + "method": "POST", + "params": [], + "headers": [ + { + "key": "Content-Type", + "value": "text/xml", + "active": true + }, + { + "key": "Content-Type", + "value": "application/json", + "active": true + }, + { + "key": "Content-Type", + "value": "application/xml", + "active": true + } + ], + "endpoint": "https://echo.hoppscotch.io", + "testScript": "pw.test(\"The last occurrence will be considered among multiple `Content-Type` headers\", ()=> {\n pw.expect(pw.response.body.headers[\"content-type\"]).toBe(\"application/xml\");\n});", + "preRequestScript": "", + "requestVariables": [] + }, + { + "v": "6", + "auth": { + "authType": "inherit", + "authActive": true + }, + "body": { + "body": "\n\n \n 12345\n John Doe\n john.doe@example.com\n \n \n 98765\n Sample Product\n 2\n \n\n", + "contentType": null + }, + "name": "multiple-content-type-headers-different-casing", + "method": "POST", + "params": [], + "headers": [ + { + "key": "Content-Type", + "value": "text/xml", + "active": true + }, + { + "key": "content-Type", + "value": "application/json", + "active": true + }, + { + "key": "Content-type", + "value": "text/plain", + "active": true + }, + { + "key": "CONTENT-TYPE", + "value": "application/xml", + "active": true + } + ], + "endpoint": "https://echo.hoppscotch.io", + "testScript": "pw.test(\"The last occurrence will be considered among multiple `Content-Type` headers following different casing\", ()=> {\n pw.expect(pw.response.body.headers[\"content-type\"]).toBe(\"application/xml\");\n});", + "preRequestScript": "", + "requestVariables": [] + }, + { + "v": "6", + "auth": { + "authType": "inherit", + "authActive": true + }, + "body": { + "body": "\n\n \n 12345\n John Doe\n john.doe@example.com\n \n \n 98765\n Sample Product\n 2\n \n\n", + "contentType": null + }, + "name": "multiple-content-type-headers-different-casing-without-value-set-at-body", + "method": "POST", + "params": [], + "headers": [ + { + "key": "Content-Type", + "value": "text/xml", + "active": true + }, + { + "key": "content-Type", + "value": "application/json", + "active": true + }, + { + "key": "Content-type", + "value": "text/plain", + "active": true + }, + { + "key": "CONTENT-TYPE", + "value": "application/xml", + "active": true + } + ], + "endpoint": "https://echo.hoppscotch.io", + "testScript": "pw.test(\"The content type is inferred from the `Content-Type` header if not set at the request body\", ()=> {\n pw.expect(pw.response.body.headers[\"content-type\"]).toBe(\"application/xml\");\n});", + "preRequestScript": "", + "requestVariables": [] + } + ], + "auth": { + "authType": "inherit", + "authActive": true + }, + "headers": [] +} \ No newline at end of file diff --git a/packages/hoppscotch-cli/src/__tests__/unit/fixtures/workspace-access.mock.ts b/packages/hoppscotch-cli/src/__tests__/unit/fixtures/workspace-access.mock.ts index e2a7d6bff..272ca22b0 100644 --- a/packages/hoppscotch-cli/src/__tests__/unit/fixtures/workspace-access.mock.ts +++ b/packages/hoppscotch-cli/src/__tests__/unit/fixtures/workspace-access.mock.ts @@ -3,6 +3,7 @@ import { Environment, EnvironmentSchemaVersion, HoppCollection, + RESTReqSchemaVersion, } from "@hoppscotch/data"; import { @@ -78,8 +79,7 @@ export const WORKSPACE_DEEPLY_NESTED_COLLECTIONS_WITH_AUTH_HEADERS_MOCK: Workspa collectionID: "clx1ldkzs005t10f8rp5u60q7", teamID: "clws3hg58000011o8h07glsb1", title: "RequestA", - request: - '{"v":"5","id":"clpttpdq00003qp16kut6doqv","auth":{"authType":"inherit","authActive":true},"body":{"body":null,"contentType":null},"name":"RequestA","method":"GET","params":[],"headers":[],"endpoint":"https://echo.hoppscotch.io","testScript":"pw.test(\\"Correctly inherits auth and headers from the root collection\\", ()=> {\\n pw.expect(pw.response.body.headers[\\"x-test-header\\"]).toBe(\\"Set at root collection\\");\\n pw.expect(pw.response.body.headers[\\"authorization\\"]).toBe(\\"Bearer BearerToken\\");\\n});","preRequestScript":"","requestVariables":[]}', + request: `{"v":"${RESTReqSchemaVersion}","id":"clpttpdq00003qp16kut6doqv","auth":{"authType":"inherit","authActive":true},"body":{"body":null,"contentType":null},"name":"RequestA","method":"GET","params":[],"headers":[],"endpoint":"https://echo.hoppscotch.io","testScript":"pw.test(\\"Correctly inherits auth and headers from the root collection\\", ()=> {\\n pw.expect(pw.response.body.headers[\\"x-test-header\\"]).toBe(\\"Set at root collection\\");\\n pw.expect(pw.response.body.headers[\\"authorization\\"]).toBe(\\"Bearer BearerToken\\");\\n});","preRequestScript":"","requestVariables":[]}`, }, ], }, @@ -214,7 +214,7 @@ export const TRANSFORMED_DEEPLY_NESTED_COLLECTIONS_WITH_AUTH_HEADERS_MOCK: HoppC ], requests: [ { - v: "5", + v: RESTReqSchemaVersion, id: "clpttpdq00003qp16kut6doqv", auth: { authType: "inherit", diff --git a/packages/hoppscotch-cli/src/utils/pre-request.ts b/packages/hoppscotch-cli/src/utils/pre-request.ts index f39410644..5c830be18 100644 --- a/packages/hoppscotch-cli/src/utils/pre-request.ts +++ b/packages/hoppscotch-cli/src/utils/pre-request.ts @@ -162,12 +162,18 @@ export function getEffectiveRESTRequest( } const effectiveFinalBody = _effectiveFinalBody.right; - if (request.body.contentType) + if ( + request.body.contentType && + !effectiveFinalHeaders.some( + ({ key }) => key.toLowerCase() === "content-type" + ) + ) { effectiveFinalHeaders.push({ active: true, - key: "content-type", + key: "Content-Type", value: request.body.contentType, }); + } // Parsing final-endpoint with applied ENVs. const _effectiveFinalURL = parseTemplateStringE( diff --git a/packages/hoppscotch-cli/src/utils/request.ts b/packages/hoppscotch-cli/src/utils/request.ts index 16a02c519..cbe9a0070 100644 --- a/packages/hoppscotch-cli/src/utils/request.ts +++ b/packages/hoppscotch-cli/src/utils/request.ts @@ -1,5 +1,10 @@ -import { Environment, HoppCollection, HoppRESTRequest } from "@hoppscotch/data"; -import axios, { Method } from "axios"; +import { + Environment, + HoppCollection, + HoppRESTRequest, + RESTReqSchemaVersion, +} from "@hoppscotch/data"; +import axios, { AxiosResponse, Method } from "axios"; import * as A from "fp-ts/Array"; import * as E from "fp-ts/Either"; import * as T from "fp-ts/Task"; @@ -55,8 +60,8 @@ const processVariables = (variable: Environment["variables"][number]) => { const processEnvs = (envs: Partial) => { // This can take the shape `{ global: undefined, selected: undefined }` when no environment is supplied const processedEnvs = { - global: envs.global?.map(processVariables), - selected: envs.selected?.map(processVariables), + global: envs.global?.map(processVariables) ?? [], + selected: envs.selected?.map(processVariables) ?? [], }; return processedEnvs; @@ -92,9 +97,12 @@ export const createRequest = (req: EffectiveHoppRESTRequest): RequestConfig => { } } } - if (req.body.contentType) { - config.headers["Content-Type"] = req.body.contentType; - switch (req.body.contentType) { + + const resolvedContentType = + config.headers["Content-Type"] ?? req.body.contentType; + + if (resolvedContentType) { + switch (resolvedContentType) { case "multipart/form-data": { // TODO: Parse Multipart Form Data // !NOTE: Temporary `config.supported` check @@ -166,7 +174,7 @@ export const requestRunner = }; if (axios.isAxiosError(e)) { - runnerResponse.endpoint = e.config.url ?? ""; + runnerResponse.endpoint = e.config?.url ?? ""; if (e.response) { const { data, status, statusText, headers } = e.response; @@ -358,7 +366,7 @@ export const preProcessRequest = ( const { headers: parentHeaders, auth: parentAuth } = collection; if (!tempRequest.v) { - tempRequest.v = "1"; + tempRequest.v = RESTReqSchemaVersion; } if (!tempRequest.name) { tempRequest.name = "Untitled Request"; diff --git a/packages/hoppscotch-common/src/helpers/utils/contenttypes.ts b/packages/hoppscotch-common/src/helpers/utils/contenttypes.ts index 822b07a8a..a4610230f 100644 --- a/packages/hoppscotch-common/src/helpers/utils/contenttypes.ts +++ b/packages/hoppscotch-common/src/helpers/utils/contenttypes.ts @@ -12,6 +12,7 @@ export const knownContentTypes: Record = { "multipart/form-data": "multipart", "text/html": "html", "text/plain": "plain", + "text/xml": "xml", } type ContentTypeTitle = @@ -33,6 +34,7 @@ export const segmentedContentTypes: SegmentedContentType[] = [ "application/hal+json", "application/vnd.api+json", "application/xml", + "text/xml", ], }, { diff --git a/packages/hoppscotch-common/src/services/persistence/__tests__/__mocks__/index.ts b/packages/hoppscotch-common/src/services/persistence/__tests__/__mocks__/index.ts index 1beaae9d9..2183aebc2 100644 --- a/packages/hoppscotch-common/src/services/persistence/__tests__/__mocks__/index.ts +++ b/packages/hoppscotch-common/src/services/persistence/__tests__/__mocks__/index.ts @@ -1,4 +1,8 @@ -import { Environment, HoppCollection } from "@hoppscotch/data" +import { + Environment, + HoppCollection, + RESTReqSchemaVersion, +} from "@hoppscotch/data" import { HoppGQLDocument } from "~/helpers/graphql/document" import { HoppRESTDocument } from "~/helpers/rest/document" @@ -25,7 +29,7 @@ export const REST_COLLECTIONS_MOCK: HoppCollection[] = [ folders: [], requests: [ { - v: "5", + v: RESTReqSchemaVersion, endpoint: "https://echo.hoppscotch.io", name: "Echo test", params: [], @@ -138,7 +142,7 @@ export const REST_HISTORY_MOCK: RESTHistoryEntry[] = [ preRequestScript: "", testScript: "", requestVariables: [], - v: "5", + v: RESTReqSchemaVersion, }, responseMeta: { duration: 807, statusCode: 200 }, star: false, @@ -194,7 +198,7 @@ export const REST_TAB_STATE_MOCK: PersistableTabState = { tabID: "e6e8d800-caa8-44a2-a6a6-b4765a3167aa", doc: { request: { - v: "5", + v: RESTReqSchemaVersion, endpoint: "https://echo.hoppscotch.io", name: "Echo test", params: [], diff --git a/packages/hoppscotch-data/src/rest/content-types.ts b/packages/hoppscotch-data/src/rest/content-types.ts index b4132d2c5..d3d27cecc 100644 --- a/packages/hoppscotch-data/src/rest/content-types.ts +++ b/packages/hoppscotch-data/src/rest/content-types.ts @@ -4,6 +4,7 @@ export const knownContentTypes = { "application/hal+json": "json", "application/vnd.api+json": "json", "application/xml": "xml", + "text/xml": "xml", "application/x-www-form-urlencoded": "multipart", "multipart/form-data": "multipart", "text/html": "html", @@ -12,4 +13,6 @@ export const knownContentTypes = { export type ValidContentTypes = keyof typeof knownContentTypes -export const ValidContentTypesList = Object.keys(knownContentTypes) as ValidContentTypes[] +export const ValidContentTypesList = Object.keys( + knownContentTypes +) as ValidContentTypes[] diff --git a/packages/hoppscotch-data/src/rest/index.ts b/packages/hoppscotch-data/src/rest/index.ts index d849ae5e5..c4317df29 100644 --- a/packages/hoppscotch-data/src/rest/index.ts +++ b/packages/hoppscotch-data/src/rest/index.ts @@ -1,32 +1,34 @@ import * as Eq from "fp-ts/Eq" import * as S from "fp-ts/string" import cloneDeep from "lodash/cloneDeep" +import { createVersionedEntity, InferredEntity } from "verzod" +import { z } from "zod" + +import { lodashIsEqualEq, mapThenEq, undefinedEq } from "../utils/eq" + import V0_VERSION from "./v/0" import V1_VERSION from "./v/1" import V2_VERSION from "./v/2" import V3_VERSION from "./v/3" import V4_VERSION from "./v/4" import V5_VERSION from "./v/5" -import { createVersionedEntity, InferredEntity } from "verzod" -import { lodashIsEqualEq, mapThenEq, undefinedEq } from "../utils/eq" +import V6_VERSION, { HoppRESTReqBody } from "./v/6" -import { HoppRESTReqBody, HoppRESTHeaders, HoppRESTParams } from "./v/1" +import { HoppRESTHeaders, HoppRESTParams } from "./v/1" -import { HoppRESTAuth } from "./v/5" import { HoppRESTRequestVariables } from "./v/2" -import { z } from "zod" +import { HoppRESTAuth } from "./v/5" export * from "./content-types" export { FormDataKeyValue, - HoppRESTReqBodyFormData, HoppRESTAuthBasic, - HoppRESTAuthInherit, HoppRESTAuthBearer, + HoppRESTAuthInherit, HoppRESTAuthNone, - HoppRESTReqBody, HoppRESTHeaders, + HoppRESTReqBodyFormData, } from "./v/1" export { @@ -37,21 +39,23 @@ export { export { AuthCodeGrantTypeParams, - HoppRESTAuthOAuth2, HoppRESTAuth, + HoppRESTAuthOAuth2, } from "./v/5" export { HoppRESTAuthAPIKey } from "./v/4" export { HoppRESTRequestVariables } from "./v/2" +export { HoppRESTReqBody } from "./v/6" + const versionedObject = z.object({ // v is a stringified number v: z.string().regex(/^\d+$/).transform(Number), }) export const HoppRESTRequest = createVersionedEntity({ - latestVersion: 5, + latestVersion: 6, versionMap: { 0: V0_VERSION, 1: V1_VERSION, @@ -59,6 +63,7 @@ export const HoppRESTRequest = createVersionedEntity({ 3: V3_VERSION, 4: V4_VERSION, 5: V5_VERSION, + 6: V6_VERSION, }, getVersion(data) { // For V1 onwards we have the v string storing the number @@ -100,7 +105,7 @@ const HoppRESTRequestEq = Eq.struct({ ), }) -export const RESTReqSchemaVersion = "5" +export const RESTReqSchemaVersion = "6" export type HoppRESTParam = HoppRESTRequest["params"][number] export type HoppRESTHeader = HoppRESTRequest["headers"][number] @@ -195,7 +200,7 @@ export function makeRESTRequest( export function getDefaultRESTRequest(): HoppRESTRequest { return { - v: "5", + v: RESTReqSchemaVersion, endpoint: "https://echo.hoppscotch.io", name: "Untitled", params: [], diff --git a/packages/hoppscotch-data/src/rest/v/6.ts b/packages/hoppscotch-data/src/rest/v/6.ts new file mode 100644 index 000000000..7c534912f --- /dev/null +++ b/packages/hoppscotch-data/src/rest/v/6.ts @@ -0,0 +1,49 @@ +import { defineVersion } from "verzod" +import { z } from "zod" + +import { FormDataKeyValue } from "./1" +import { V5_SCHEMA } from "./5" + +export const HoppRESTReqBody = z.union([ + z.object({ + contentType: z.literal(null), + body: z.literal(null).catch(null), + }), + z.object({ + contentType: z.literal("multipart/form-data"), + body: z.array(FormDataKeyValue).catch([]), + }), + z.object({ + contentType: z.union([ + z.literal("application/json"), + z.literal("application/ld+json"), + z.literal("application/hal+json"), + z.literal("application/vnd.api+json"), + z.literal("application/xml"), + z.literal("text/xml"), + z.literal("application/x-www-form-urlencoded"), + z.literal("text/html"), + z.literal("text/plain"), + ]), + body: z.string().catch(""), + }), +]) + +export type HoppRESTReqBody = z.infer + +export const V6_SCHEMA = V5_SCHEMA.extend({ + v: z.literal("6"), + body: HoppRESTReqBody, +}) + +export default defineVersion({ + schema: V6_SCHEMA, + initial: false, + up(old: z.infer) { + // No migration, `text/xml` is added to the list of supported content types + return { + ...old, + v: "6" as const, + } + }, +})