From 50e44ef9d625d68f71c2062f590c071293976304 Mon Sep 17 00:00:00 2001 From: Ayush Srivastava Date: Sun, 22 Mar 2026 00:11:51 +0530 Subject: [PATCH 1/2] Add querystring parameter builtin validations --- .../Validator/Validation+Builtins.swift | 118 ++++++++++++++- Sources/OpenAPIKit/Validator/Validator.swift | 3 + .../Validator/BuiltinValidationTests.swift | 140 +++++++++++++++++- 3 files changed, 257 insertions(+), 4 deletions(-) diff --git a/Sources/OpenAPIKit/Validator/Validation+Builtins.swift b/Sources/OpenAPIKit/Validator/Validation+Builtins.swift index 327f8991a..8b3567d09 100644 --- a/Sources/OpenAPIKit/Validator/Validation+Builtins.swift +++ b/Sources/OpenAPIKit/Validator/Validation+Builtins.swift @@ -377,6 +377,97 @@ extension Validation { ) } + /// Validate that `querystring` parameters are unique and do not coexist + /// with `query` parameters within a Path Item's effective operation + /// parameters. + /// + /// OpenAPI 3.2.0 requires that a `querystring` parameter + /// [must not appear more than once and must not appear in the same operation + /// as any `query` parameters](https://spec.openapis.org/oas/v3.2.0.html#parameter-locations). + /// + /// - Important: This is included in validation by default. + public static var querystringParametersAreCompatible: Validation { + .init( + description: "Querystring parameters are unique and do not coexist with query parameters", + check: { context in + let pathParameters = resolvedParameters(context.subject.parameters, components: context.document.components) + let pathSummary = parameterLocationSummary(pathParameters) + let pathParametersPath = context.codingPath + [Validator.CodingKey.init(stringValue: "parameters")] + var errors = [ValidationError]() + + if pathSummary.querystringCount > 1 { + errors.append( + ValidationError( + reason: "Path Item parameters must not contain more than one `querystring` parameter", + at: pathParametersPath + ) + ) + } + + if pathSummary.querystringCount > 0 && pathSummary.queryCount > 0 { + errors.append( + ValidationError( + reason: "Path Item parameters must not mix `querystring` and `query` parameter locations", + at: pathParametersPath + ) + ) + } + + for endpoint in context.subject.endpoints { + let operationParameters = resolvedParameters(endpoint.operation.parameters, components: context.document.components) + let operationSummary = parameterLocationSummary(operationParameters) + let operationParametersPath = context.codingPath + [ + Validator.CodingKey.init(stringValue: codingPathKey(for: endpoint.method)), + Validator.CodingKey.init(stringValue: "parameters") + ] + + if operationSummary.querystringCount > 1 { + errors.append( + ValidationError( + reason: "Operation parameters must not contain more than one `querystring` parameter", + at: operationParametersPath + ) + ) + } + + if operationSummary.querystringCount > 0 && operationSummary.queryCount > 0 { + errors.append( + ValidationError( + reason: "Operation parameters must not mix `querystring` and `query` parameter locations", + at: operationParametersPath + ) + ) + } + + if pathSummary.querystringCount <= 1 && + operationSummary.querystringCount <= 1 && + pathSummary.querystringCount + operationSummary.querystringCount > 1 { + errors.append( + ValidationError( + reason: "Operation parameters must not contain more than one `querystring` parameter, including inherited Path Item parameters", + at: operationParametersPath + ) + ) + } + + if !(pathSummary.querystringCount > 0 && pathSummary.queryCount > 0) && + !(operationSummary.querystringCount > 0 && operationSummary.queryCount > 0) && + pathSummary.querystringCount + operationSummary.querystringCount > 0 && + pathSummary.queryCount + operationSummary.queryCount > 0 { + errors.append( + ValidationError( + reason: "Operation parameters must not mix `querystring` and `query` parameter locations, including inherited Path Item parameters", + at: operationParametersPath + ) + ) + } + } + + return errors + } + ) + } + /// Validate that all OpenAPI Operation Ids are unique across the whole Document. /// /// The OpenAPI Specification requires that Operation Ids [are unique](https://spec.openapis.org/oas/v3.2.0.html#operation-object). @@ -575,9 +666,34 @@ extension Validation { /// Used by both the Path Item parameter check and the /// Operation parameter check in the default validations. fileprivate func parametersAreUnique(_ parameters: OpenAPI.Parameter.Array, components: OpenAPI.Components) -> Bool { - let foundParameters = parameters.compactMap { try? components.lookup($0) } + let foundParameters = resolvedParameters(parameters, components: components) let identities = foundParameters.map { OpenAPI.Parameter.ParameterIdentity(name: $0.name, location: $0.location) } return Set(identities).count == foundParameters.count } + +fileprivate func resolvedParameters(_ parameters: OpenAPI.Parameter.Array, components: OpenAPI.Components) -> [OpenAPI.Parameter] { + parameters.compactMap { try? components.lookup($0) } +} + +fileprivate struct ParameterLocationSummary { + let queryCount: Int + let querystringCount: Int +} + +fileprivate func parameterLocationSummary(_ parameters: [OpenAPI.Parameter]) -> ParameterLocationSummary { + .init( + queryCount: parameters.filter { $0.location == .query }.count, + querystringCount: parameters.filter { $0.location == .querystring }.count + ) +} + +fileprivate func codingPathKey(for method: OpenAPI.HttpMethod) -> String { + switch method { + case .builtin(let builtin): + return builtin.rawValue.lowercased() + case .other(let other): + return other + } +} diff --git a/Sources/OpenAPIKit/Validator/Validator.swift b/Sources/OpenAPIKit/Validator/Validator.swift index 4b8c5fade..73b4df3bd 100644 --- a/Sources/OpenAPIKit/Validator/Validator.swift +++ b/Sources/OpenAPIKit/Validator/Validator.swift @@ -77,6 +77,7 @@ extension OpenAPI.Document { /// - Document-level tag names are unique. /// - Parameters are unique within each Path Item. /// - Parameters are unique within each Operation. +/// - Querystring parameters are unique and do not coexist with query parameters. /// - Operation Ids are unique across the whole Document. /// - All OpenAPI.References that refer to components in this /// document can be found in the components dictionary. @@ -158,6 +159,7 @@ public final class Validator { .init(.documentTagNamesAreUnique), .init(.pathItemParametersAreUnique), .init(.operationParametersAreUnique), + .init(.querystringParametersAreCompatible), .init(.operationIdsAreUnique), .init(.serverVariableEnumIsValid), .init(.serverVariableDefaultExistsInEnum), @@ -202,6 +204,7 @@ public final class Validator { /// - Document-level tag names are unique. /// - Parameters are unique within each Path Item. /// - Parameters are unique within each Operation. + /// - Querystring parameters are unique and do not coexist with query parameters. /// - Operation Ids are unique across the whole Document. /// - All OpenAPI.References that refer to components in this document can /// be found in the components dictionary. diff --git a/Tests/OpenAPIKitTests/Validator/BuiltinValidationTests.swift b/Tests/OpenAPIKitTests/Validator/BuiltinValidationTests.swift index f8ffa15b1..8e0c37b7d 100644 --- a/Tests/OpenAPIKitTests/Validator/BuiltinValidationTests.swift +++ b/Tests/OpenAPIKitTests/Validator/BuiltinValidationTests.swift @@ -24,11 +24,12 @@ final class BuiltinValidationTests: XCTestCase { ]) let withoutReferenceValidations = Validator().skippingReferenceValidations() - XCTAssertEqual(withoutReferenceValidations.validationDescriptions.count, 7) + XCTAssertEqual(withoutReferenceValidations.validationDescriptions.count, 8) XCTAssertEqual(withoutReferenceValidations.validationDescriptions, [ "The names of Tags in the Document are unique", "Path Item parameters are unique (identity is defined by the \'name\' and \'location\')", "Operation parameters are unique (identity is defined by the \'name\' and \'location\')", + "Querystring parameters are unique and do not coexist with query parameters", "All Operation Ids in Document are unique", "Server Variable\'s enum is either not defined or is non-empty (if defined).", "Server Variable\'s default must exist in enum, if enum is defined.", @@ -36,11 +37,12 @@ final class BuiltinValidationTests: XCTestCase { ]) let defaultValidations = Validator() - XCTAssertEqual(defaultValidations.validationDescriptions.count, 17) + XCTAssertEqual(defaultValidations.validationDescriptions.count, 18) XCTAssertEqual(defaultValidations.validationDescriptions, [ "The names of Tags in the Document are unique", "Path Item parameters are unique (identity is defined by the \'name\' and \'location\')", "Operation parameters are unique (identity is defined by the \'name\' and \'location\')", + "Querystring parameters are unique and do not coexist with query parameters", "All Operation Ids in Document are unique", "Server Variable\'s enum is either not defined or is non-empty (if defined).", "Server Variable\'s default must exist in enum, if enum is defined.", @@ -58,11 +60,12 @@ final class BuiltinValidationTests: XCTestCase { ]) let stricterReferenceValidations = Validator().validatingAllReferencesFoundInComponents() - XCTAssertEqual(stricterReferenceValidations.validationDescriptions.count, 17) + XCTAssertEqual(stricterReferenceValidations.validationDescriptions.count, 18) XCTAssertEqual(stricterReferenceValidations.validationDescriptions, [ "The names of Tags in the Document are unique", "Path Item parameters are unique (identity is defined by the \'name\' and \'location\')", "Operation parameters are unique (identity is defined by the \'name\' and \'location\')", + "Querystring parameters are unique and do not coexist with query parameters", "All Operation Ids in Document are unique", "Server Variable\'s enum is either not defined or is non-empty (if defined).", "Server Variable\'s default must exist in enum, if enum is defined.", @@ -1411,4 +1414,135 @@ final class BuiltinValidationTests: XCTestCase { XCTAssertEqual(errorCollection?.values.first?.codingPath.stringValue, ".paths[\'/hello\'].get.parameters[0]") } } + + func test_duplicateQuerystringParametersOnPathItem_fails() throws { + let document = OpenAPI.Document( + info: .init(title: "test", version: "1.0"), + servers: [], + paths: [ + "/hello": .init( + parameters: [ + .parameter(OpenAPI.Parameter.querystring(name: "first", content: [:])), + .parameter(OpenAPI.Parameter.querystring(name: "second", content: [:])) + ], + get: .init(responses: [:]) + ) + ], + components: .noComponents + ) + + let validator = Validator.blank.validating(.querystringParametersAreCompatible) + + XCTAssertThrowsError(try document.validate(using: validator)) { error in + let errorCollection = error as? ValidationErrorCollection + XCTAssertEqual(errorCollection?.values.first?.reason, "Path Item parameters must not contain more than one `querystring` parameter") + XCTAssertEqual(errorCollection?.values.first?.codingPath.stringValue, ".paths[\'/hello\'].parameters") + } + } + + func test_duplicateQuerystringParametersAcrossPathItemAndOperation_fails() throws { + let document = OpenAPI.Document( + info: .init(title: "test", version: "1.0"), + servers: [], + paths: [ + "/hello": .init( + parameters: [ + .parameter(OpenAPI.Parameter.querystring(name: "first", content: [:])) + ], + get: .init( + parameters: [ + .parameter(OpenAPI.Parameter.querystring(name: "second", content: [:])) + ], + responses: [:] + ) + ) + ], + components: .noComponents + ) + + let validator = Validator.blank.validating(.querystringParametersAreCompatible) + + XCTAssertThrowsError(try document.validate(using: validator)) { error in + let errorCollection = error as? ValidationErrorCollection + XCTAssertEqual(errorCollection?.values.first?.reason, "Operation parameters must not contain more than one `querystring` parameter, including inherited Path Item parameters") + XCTAssertEqual(errorCollection?.values.first?.codingPath.stringValue, ".paths[\'/hello\'].get.parameters") + } + } + + func test_querystringAndQueryParametersOnOperation_fails() throws { + let document = OpenAPI.Document( + info: .init(title: "test", version: "1.0"), + servers: [], + paths: [ + "/hello": .init( + get: .init( + parameters: [ + .parameter(OpenAPI.Parameter.query(name: "query", schema: .string)), + .parameter(OpenAPI.Parameter.querystring(name: "querystring", content: [:])) + ], + responses: [:] + ) + ) + ], + components: .noComponents + ) + + let validator = Validator.blank.validating(.querystringParametersAreCompatible) + + XCTAssertThrowsError(try document.validate(using: validator)) { error in + let errorCollection = error as? ValidationErrorCollection + XCTAssertEqual(errorCollection?.values.first?.reason, "Operation parameters must not mix `querystring` and `query` parameter locations") + XCTAssertEqual(errorCollection?.values.first?.codingPath.stringValue, ".paths[\'/hello\'].get.parameters") + } + } + + func test_querystringAndQueryParametersAcrossPathItemAndOperation_fails() throws { + let document = OpenAPI.Document( + info: .init(title: "test", version: "1.0"), + servers: [], + paths: [ + "/hello": .init( + parameters: [ + .parameter(OpenAPI.Parameter.query(name: "query", schema: .string)) + ], + get: .init( + parameters: [ + .parameter(OpenAPI.Parameter.querystring(name: "querystring", content: [:])) + ], + responses: [:] + ) + ) + ], + components: .noComponents + ) + + let validator = Validator.blank.validating(.querystringParametersAreCompatible) + + XCTAssertThrowsError(try document.validate(using: validator)) { error in + let errorCollection = error as? ValidationErrorCollection + XCTAssertEqual(errorCollection?.values.first?.reason, "Operation parameters must not mix `querystring` and `query` parameter locations, including inherited Path Item parameters") + XCTAssertEqual(errorCollection?.values.first?.codingPath.stringValue, ".paths[\'/hello\'].get.parameters") + } + } + + func test_singleQuerystringParameter_succeeds() throws { + let document = OpenAPI.Document( + info: .init(title: "test", version: "1.0"), + servers: [], + paths: [ + "/hello": .init( + get: .init( + parameters: [ + .parameter(OpenAPI.Parameter.querystring(name: "querystring", content: [:])) + ], + responses: [:] + ) + ) + ], + components: .noComponents + ) + + let validator = Validator.blank.validating(.querystringParametersAreCompatible) + try document.validate(using: validator) + } } From 16d88fd1c3cb8f0eafe6689c024d4e3869ff6aac Mon Sep 17 00:00:00 2001 From: Ayush Srivastava Date: Sat, 21 Mar 2026 23:58:22 +0530 Subject: [PATCH 2/2] Avoid async let crashes in path item dereferencing --- .../Path Item/DereferencedPathItem.swift | 121 +++++++++--------- .../Path Item/DereferencedPathItem.swift | 102 ++++++++------- 2 files changed, 118 insertions(+), 105 deletions(-) diff --git a/Sources/OpenAPIKit/Path Item/DereferencedPathItem.swift b/Sources/OpenAPIKit/Path Item/DereferencedPathItem.swift index 8bddb1ab7..3fb043847 100644 --- a/Sources/OpenAPIKit/Path Item/DereferencedPathItem.swift +++ b/Sources/OpenAPIKit/Path Item/DereferencedPathItem.swift @@ -167,66 +167,73 @@ extension OpenAPI.PathItem: ExternallyDereferenceable { let oldAdditionalOperations = additionalOperations - async let (newParameters, c1, m1) = oldParameters.externallyDereferenced(with: loader) -// async let (newServers, c2, m2) = oldServers.externallyDereferenced(with: loader) - async let (newGet, c3, m3) = oldGet.externallyDereferenced(with: loader) - async let (newPut, c4, m4) = oldPut.externallyDereferenced(with: loader) - async let (newPost, c5, m5) = oldPost.externallyDereferenced(with: loader) - async let (newDelete, c6, m6) = oldDelete.externallyDereferenced(with: loader) - async let (newOptions, c7, m7) = oldOptions.externallyDereferenced(with: loader) - async let (newHead, c8, m8) = oldHead.externallyDereferenced(with: loader) - async let (newPatch, c9, m9) = oldPatch.externallyDereferenced(with: loader) - async let (newTrace, c10, m10) = oldTrace.externallyDereferenced(with: loader) - async let (newQuery, c11, m11) = oldQuery.externallyDereferenced(with: loader) - - async let (newAdditionalOperations, c12, m12) = oldAdditionalOperations.externallyDereferenced(with: loader) - var pathItem = self - var newComponents = try await c1 - var newMessages = try await m1 - - // ideally we would async let all of the props above and then set them here, - // but for now since there seems to be some sort of compiler bug we will do - // newServers in an if let below - pathItem.parameters = try await newParameters - pathItem.get = try await newGet - pathItem.put = try await newPut - pathItem.post = try await newPost - pathItem.delete = try await newDelete - pathItem.options = try await newOptions - pathItem.head = try await newHead - pathItem.patch = try await newPatch - pathItem.trace = try await newTrace - pathItem.query = try await newQuery - pathItem.additionalOperations = try await newAdditionalOperations - - try await newComponents.merge(c3) - try await newComponents.merge(c4) - try await newComponents.merge(c5) - try await newComponents.merge(c6) - try await newComponents.merge(c7) - try await newComponents.merge(c8) - try await newComponents.merge(c9) - try await newComponents.merge(c10) - try await newComponents.merge(c11) - try await newComponents.merge(c12) - - try await newMessages += m3 - try await newMessages += m4 - try await newMessages += m5 - try await newMessages += m6 - try await newMessages += m7 - try await newMessages += m8 - try await newMessages += m9 - try await newMessages += m10 - try await newMessages += m11 - try await newMessages += m12 + + // Older Swift 6.0/6.1 Linux runtimes have been observed crashing when this + // dereference path fans out into a large batch of `async let` child tasks. + // The sequential form keeps behavior the same while staying compatible + // across the full test matrix. + let (newParameters, c1, m1) = try await oldParameters.externallyDereferenced(with: loader) + pathItem.parameters = newParameters + + var newComponents = c1 + var newMessages = m1 + + let (newGet, c3, m3) = try await oldGet.externallyDereferenced(with: loader) + pathItem.get = newGet + try newComponents.merge(c3) + newMessages += m3 + + let (newPut, c4, m4) = try await oldPut.externallyDereferenced(with: loader) + pathItem.put = newPut + try newComponents.merge(c4) + newMessages += m4 + + let (newPost, c5, m5) = try await oldPost.externallyDereferenced(with: loader) + pathItem.post = newPost + try newComponents.merge(c5) + newMessages += m5 + + let (newDelete, c6, m6) = try await oldDelete.externallyDereferenced(with: loader) + pathItem.delete = newDelete + try newComponents.merge(c6) + newMessages += m6 + + let (newOptions, c7, m7) = try await oldOptions.externallyDereferenced(with: loader) + pathItem.options = newOptions + try newComponents.merge(c7) + newMessages += m7 + + let (newHead, c8, m8) = try await oldHead.externallyDereferenced(with: loader) + pathItem.head = newHead + try newComponents.merge(c8) + newMessages += m8 + + let (newPatch, c9, m9) = try await oldPatch.externallyDereferenced(with: loader) + pathItem.patch = newPatch + try newComponents.merge(c9) + newMessages += m9 + + let (newTrace, c10, m10) = try await oldTrace.externallyDereferenced(with: loader) + pathItem.trace = newTrace + try newComponents.merge(c10) + newMessages += m10 + + let (newQuery, c11, m11) = try await oldQuery.externallyDereferenced(with: loader) + pathItem.query = newQuery + try newComponents.merge(c11) + newMessages += m11 + + let (newAdditionalOperations, c12, m12) = try await oldAdditionalOperations.externallyDereferenced(with: loader) + pathItem.additionalOperations = newAdditionalOperations + try newComponents.merge(c12) + newMessages += m12 if let oldServers { - async let (newServers, c2, m2) = oldServers.externallyDereferenced(with: loader) - pathItem.servers = try await newServers - try await newComponents.merge(c2) - try await newMessages += m2 + let (newServers, c2, m2) = try await oldServers.externallyDereferenced(with: loader) + pathItem.servers = newServers + try newComponents.merge(c2) + newMessages += m2 } return (pathItem, newComponents, newMessages) diff --git a/Sources/OpenAPIKit30/Path Item/DereferencedPathItem.swift b/Sources/OpenAPIKit30/Path Item/DereferencedPathItem.swift index ae9d6e3be..fdd27f1b6 100644 --- a/Sources/OpenAPIKit30/Path Item/DereferencedPathItem.swift +++ b/Sources/OpenAPIKit30/Path Item/DereferencedPathItem.swift @@ -153,57 +153,63 @@ extension OpenAPI.PathItem: ExternallyDereferenceable { let oldPatch = patch let oldTrace = trace - async let (newParameters, c1, m1) = oldParameters.externallyDereferenced(with: loader) -// async let (newServers, c2, m2) = oldServers.externallyDereferenced(with: loader) - async let (newGet, c3, m3) = oldGet.externallyDereferenced(with: loader) - async let (newPut, c4, m4) = oldPut.externallyDereferenced(with: loader) - async let (newPost, c5, m5) = oldPost.externallyDereferenced(with: loader) - async let (newDelete, c6, m6) = oldDelete.externallyDereferenced(with: loader) - async let (newOptions, c7, m7) = oldOptions.externallyDereferenced(with: loader) - async let (newHead, c8, m8) = oldHead.externallyDereferenced(with: loader) - async let (newPatch, c9, m9) = oldPatch.externallyDereferenced(with: loader) - async let (newTrace, c10, m10) = oldTrace.externallyDereferenced(with: loader) - var pathItem = self - var newComponents = try await c1 - var newMessages = try await m1 - - // ideally we would async let all of the props above and then set them here, - // but for now since there seems to be some sort of compiler bug we will do - // newServers in an if let below - pathItem.parameters = try await newParameters - pathItem.get = try await newGet - pathItem.put = try await newPut - pathItem.post = try await newPost - pathItem.delete = try await newDelete - pathItem.options = try await newOptions - pathItem.head = try await newHead - pathItem.patch = try await newPatch - pathItem.trace = try await newTrace - - try await newComponents.merge(c3) - try await newComponents.merge(c4) - try await newComponents.merge(c5) - try await newComponents.merge(c6) - try await newComponents.merge(c7) - try await newComponents.merge(c8) - try await newComponents.merge(c9) - try await newComponents.merge(c10) - - try await newMessages += m3 - try await newMessages += m4 - try await newMessages += m5 - try await newMessages += m6 - try await newMessages += m7 - try await newMessages += m8 - try await newMessages += m9 - try await newMessages += m10 + + // Older Swift 6.0/6.1 Linux runtimes have been observed crashing when this + // dereference path fans out into a large batch of `async let` child tasks. + // The sequential form keeps behavior the same while staying compatible + // across the full test matrix. + let (newParameters, c1, m1) = try await oldParameters.externallyDereferenced(with: loader) + pathItem.parameters = newParameters + + var newComponents = c1 + var newMessages = m1 + + let (newGet, c3, m3) = try await oldGet.externallyDereferenced(with: loader) + pathItem.get = newGet + try newComponents.merge(c3) + newMessages += m3 + + let (newPut, c4, m4) = try await oldPut.externallyDereferenced(with: loader) + pathItem.put = newPut + try newComponents.merge(c4) + newMessages += m4 + + let (newPost, c5, m5) = try await oldPost.externallyDereferenced(with: loader) + pathItem.post = newPost + try newComponents.merge(c5) + newMessages += m5 + + let (newDelete, c6, m6) = try await oldDelete.externallyDereferenced(with: loader) + pathItem.delete = newDelete + try newComponents.merge(c6) + newMessages += m6 + + let (newOptions, c7, m7) = try await oldOptions.externallyDereferenced(with: loader) + pathItem.options = newOptions + try newComponents.merge(c7) + newMessages += m7 + + let (newHead, c8, m8) = try await oldHead.externallyDereferenced(with: loader) + pathItem.head = newHead + try newComponents.merge(c8) + newMessages += m8 + + let (newPatch, c9, m9) = try await oldPatch.externallyDereferenced(with: loader) + pathItem.patch = newPatch + try newComponents.merge(c9) + newMessages += m9 + + let (newTrace, c10, m10) = try await oldTrace.externallyDereferenced(with: loader) + pathItem.trace = newTrace + try newComponents.merge(c10) + newMessages += m10 if let oldServers { - async let (newServers, c2, m2) = oldServers.externallyDereferenced(with: loader) - pathItem.servers = try await newServers - try await newComponents.merge(c2) - try await newMessages += m2 + let (newServers, c2, m2) = try await oldServers.externallyDereferenced(with: loader) + pathItem.servers = newServers + try newComponents.merge(c2) + newMessages += m2 } return (pathItem, newComponents, newMessages)