From cf9aed7acc7c81457e71777dea2bbe8877c994db Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 11:00:57 +0100 Subject: [PATCH 01/13] Add OpenApiValidationPlugin to validate API traffic against local OpenAPI specs Adds a reporting plugin that validates intercepted requests and responses against local OpenAPI specs (specFiles and/or specsFolder) and reports undocumented paths/methods/status codes/content types, parameter issues and request/response body schema violations. Closes #779 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OpenApiValidationPluginTests.cs | 409 +++++++++ .../Reporting/OpenApiValidationPlugin.cs | 805 ++++++++++++++++++ .../OpenApiValidationPluginReport.cs | 154 ++++ .../Utils/OpenApiSchemaValidator.cs | 300 +++++++ .../openapivalidationplugin.schema.json | 40 + 5 files changed, 1708 insertions(+) create mode 100644 DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs create mode 100644 DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs create mode 100644 DevProxy.Plugins/Reporting/OpenApiValidationPluginReport.cs create mode 100644 DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs create mode 100644 schemas/v4.1.0/openapivalidationplugin.schema.json diff --git a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs new file mode 100644 index 00000000..254540b3 --- /dev/null +++ b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs @@ -0,0 +1,409 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Net; +using DevProxy.Abstractions.Proxy; +using DevProxy.Abstractions.Utils; +using DevProxy.Plugins.Reporting; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace DevProxy.Integration.Tests; + +/// +/// Coverage for , which validates intercepted +/// requests and responses against local OpenAPI specs. +/// +public sealed class OpenApiValidationPluginTests : IDisposable +{ + private const string Spec = """ + { + "openapi": "3.0.3", + "info": { "title": "Contoso", "version": "1.0" }, + "servers": [ { "url": "https://api.contoso.com/v1" } ], + "paths": { + "/users": { + "get": { + "parameters": [ + { "name": "top", "in": "query", "schema": { "type": "integer" } }, + { "name": "status", "in": "query", "schema": { "type": "string", "enum": [ "active", "inactive" ] } }, + { "name": "x-tenant", "in": "header", "required": true, "schema": { "type": "string" } } + ], + "responses": { + "200": { + "description": "ok", + "headers": { + "x-total-count": { "required": true, "schema": { "type": "integer" } } + }, + "content": { + "application/json": { + "schema": { "type": "array", "items": { "$ref": "#/components/schemas/User" } } + } + } + }, + "4XX": { + "description": "client error", + "content": { "application/json": { "schema": { "$ref": "#/components/schemas/Error" } } } + } + } + }, + "post": { + "requestBody": { + "required": true, + "content": { + "application/json": { "schema": { "$ref": "#/components/schemas/User" } } + } + }, + "responses": { + "201": { + "description": "created", + "content": { "application/json": { "schema": { "$ref": "#/components/schemas/User" } } } + } + } + } + }, + "/users/{id}": { + "parameters": [ { "name": "id", "in": "path", "required": true, "schema": { "type": "integer" } } ], + "get": { + "responses": { + "200": { + "description": "ok", + "content": { "application/json": { "schema": { "$ref": "#/components/schemas/User" } } } + } + } + } + }, + "/users/me": { + "get": { + "responses": { + "200": { + "description": "ok", + "content": { "application/json": { "schema": { "$ref": "#/components/schemas/User" } } } + } + } + } + }, + "/pets": { + "get": { + "responses": { + "default": { + "description": "pets", + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { + "oneOf": [ { "$ref": "#/components/schemas/Cat" }, { "$ref": "#/components/schemas/Dog" } ] + } + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "User": { + "type": "object", + "required": [ "id", "name" ], + "additionalProperties": false, + "properties": { + "id": { "type": "integer", "readOnly": true }, + "name": { "type": "string" }, + "nickname": { "type": "string", "nullable": true }, + "role": { "type": "string", "enum": [ "admin", "user" ] }, + "tags": { "type": "array", "items": { "type": "string" } } + } + }, + "Error": { + "type": "object", + "required": [ "message" ], + "properties": { "message": { "type": "string" } } + }, + "Cat": { + "type": "object", + "required": [ "meows" ], + "additionalProperties": false, + "properties": { "meows": { "type": "boolean" } } + }, + "Dog": { + "type": "object", + "required": [ "barks" ], + "additionalProperties": false, + "properties": { "barks": { "type": "boolean" } } + } + } + } + } + """; + + private readonly DirectoryInfo _dir = Directory.CreateTempSubdirectory("devproxy-oasvalidation-"); + + public void Dispose() => _dir.Delete(recursive: true); + + private static RecordingArgs Recording(IEnumerable logs) => + new(logs) + { + GlobalData = new() { [ProxyUtils.ReportsKey] = new Dictionary() }, + }; + + private async Task CreatePluginAsync(string configJson = """{ "specFiles": [ "contoso.json" ] }""") + { + await File.WriteAllTextAsync(Path.Combine(_dir.FullName, "contoso.json"), Spec); + + var proxyConfig = new TestProxyConfiguration + { + ConfigFile = Path.Combine(_dir.FullName, "devproxyrc.json"), + }; + var plugin = new OpenApiValidationPlugin( + TestDefaults.HttpClient, + NullLogger.Instance, + KestrelProxyHarness.BuildUrlsToWatch("api.contoso.com"), + proxyConfig, + PluginConfig.FromJson(configJson)); + + using var host = new PluginTestHost(proxyConfig); + await plugin.InitializeAsync(host.CreateInitArgs(), CancellationToken.None); + return plugin; + } + + private static TestExchange Get(string url, HttpStatusCode status, string? body, + IEnumerable<(string, string)>? requestHeaders = null, + IEnumerable<(string, string)>? responseHeaders = null) => + TestExchange + .Request("GET", url, headers: requestHeaders) + .WithResponse( + status, + headers: [("Content-Type", "application/json"), .. responseHeaders ?? []], + body: body); + + private static string[] Messages(OpenApiValidationResult? result) => + [.. Assert.IsType(result).Issues.Select(i => i.ToString())]; + + [Fact] + public async Task ValidRequestAndResponse_HasNoIssues() + { + var plugin = await CreatePluginAsync(); + var exchange = Get("https://api.contoso.com/v1/users?top=10&status=active", HttpStatusCode.OK, + """[ { "id": 1, "name": "Alex", "nickname": null, "role": "admin", "tags": [ "a" ] } ]""", + requestHeaders: [("x-tenant", "contoso")], + responseHeaders: [("x-total-count", "1")]); + + var result = plugin.Validate(exchange.Session); + + Assert.NotNull(result); + Assert.Equal("GET /users", result.Operation); + Assert.Empty(result.Issues); + } + + [Fact] + public async Task ReportsParameterIssues() + { + var plugin = await CreatePluginAsync(); + var exchange = Get("https://api.contoso.com/v1/users?top=abc&status=deleted&foo=bar", HttpStatusCode.OK, + "[]", responseHeaders: [("x-total-count", "0")]); + + var messages = Messages(plugin.Validate(exchange.Session)); + + Assert.Contains(messages, m => m.Contains("Missing required header parameter 'x-tenant'", StringComparison.Ordinal)); + Assert.Contains(messages, m => m.StartsWith("top:", StringComparison.Ordinal) && m.Contains("Expected integer", StringComparison.Ordinal)); + Assert.Contains(messages, m => m.StartsWith("status:", StringComparison.Ordinal) && m.Contains("allowed values", StringComparison.Ordinal)); + Assert.Contains(messages, m => m.Contains("Query parameter 'foo' isn't documented", StringComparison.Ordinal)); + Assert.Equal(4, messages.Length); + } + + [Fact] + public async Task ReportsInvalidPathParameter() + { + var plugin = await CreatePluginAsync(); + var exchange = Get("https://api.contoso.com/v1/users/abc", HttpStatusCode.OK, """{ "id": 1, "name": "Alex" }"""); + + var result = plugin.Validate(exchange.Session); + + Assert.Equal("GET /users/{id}", result?.Operation); + var issue = Assert.Single(result!.Issues); + Assert.Equal("id", issue.Path); + Assert.Contains("Expected integer", issue.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task PrefersLiteralPathOverTemplate() + { + var plugin = await CreatePluginAsync(); + var exchange = Get("https://api.contoso.com/v1/users/me", HttpStatusCode.OK, """{ "id": 1, "name": "Alex" }"""); + + var result = plugin.Validate(exchange.Session); + + Assert.Equal("GET /users/me", result?.Operation); + Assert.Empty(result!.Issues); + } + + [Fact] + public async Task ReportsUndocumentedPathAndMethod() + { + var plugin = await CreatePluginAsync(); + + var path = plugin.Validate(Get("https://api.contoso.com/v1/orders", HttpStatusCode.OK, "[]").Session); + var method = plugin.Validate(TestExchange.Request("DELETE", "https://api.contoso.com/v1/users").WithResponse(HttpStatusCode.NoContent).Session); + + Assert.Contains("Path '/orders' isn't documented", Assert.Single(Messages(path)), StringComparison.Ordinal); + Assert.Contains("Method DELETE isn't documented for path '/users'", Assert.Single(Messages(method)), StringComparison.Ordinal); + } + + [Fact] + public async Task ReportsRequestBodyIssues() + { + var plugin = await CreatePluginAsync(); + var exchange = TestExchange + .Request("POST", "https://api.contoso.com/v1/users", + headers: [("Content-Type", "application/json")], + body: """{ "name": 42, "role": "guest", "unknown": true }""") + .WithResponse(HttpStatusCode.Created, headers: [("Content-Type", "application/json")], + body: """{ "id": 1, "name": "Alex" }"""); + + var result = plugin.Validate(exchange.Session); + var messages = Messages(result); + + // id is readOnly so it's not required in the request + Assert.DoesNotContain(messages, m => m.Contains("'id'", StringComparison.Ordinal)); + Assert.Contains("$.name: Expected string but got integer", messages); + Assert.Contains(messages, m => m.StartsWith("$.role:", StringComparison.Ordinal)); + Assert.Contains("$.unknown: Property 'unknown' isn't defined in the schema", messages); + Assert.All(result!.Issues, i => Assert.Equal(OpenApiValidationIssueLocation.Request, i.Location)); + } + + [Fact] + public async Task ReportsMissingRequiredRequestBodyAndUndocumentedContentType() + { + var plugin = await CreatePluginAsync(); + + var missing = plugin.Validate(TestExchange.Request("POST", "https://api.contoso.com/v1/users") + .WithResponse(HttpStatusCode.Created).Session); + var xml = plugin.Validate(TestExchange.Request("POST", "https://api.contoso.com/v1/users", + headers: [("Content-Type", "application/xml")], body: "") + .WithResponse(HttpStatusCode.Created).Session); + + Assert.Equal("Missing required request body", Assert.Single(Messages(missing))); + Assert.Contains("Content type 'application/xml' isn't documented", Assert.Single(Messages(xml)), StringComparison.Ordinal); + } + + [Fact] + public async Task ReportsResponseIssues() + { + var plugin = await CreatePluginAsync(); + var headers = new[] { ("x-tenant", "contoso") }; + + var undocumentedStatus = plugin.Validate(Get("https://api.contoso.com/v1/users", HttpStatusCode.InternalServerError, "{}", headers).Session); + var badBody = plugin.Validate(Get("https://api.contoso.com/v1/users", HttpStatusCode.OK, """[ { "id": "1" } ]""", headers, + responseHeaders: [("x-total-count", "1")]).Session); + var missingHeader = plugin.Validate(Get("https://api.contoso.com/v1/users", HttpStatusCode.OK, "[]", headers).Session); + var range = plugin.Validate(Get("https://api.contoso.com/v1/users", HttpStatusCode.NotFound, """{ "message": "nope" }""", headers).Session); + + Assert.Contains("Status code 500 isn't documented", Assert.Single(Messages(undocumentedStatus)), StringComparison.Ordinal); + var badBodyMessages = Messages(badBody); + Assert.Contains("$[0].id: Expected integer but got string", badBodyMessages); + Assert.Contains("$[0]: Missing required property 'name'", badBodyMessages); + Assert.Contains("Missing required response header 'x-total-count'", Assert.Single(Messages(missingHeader)), StringComparison.Ordinal); + Assert.Empty(range!.Issues); + } + + [Fact] + public async Task ValidatesOneOf() + { + var plugin = await CreatePluginAsync(); + + var valid = plugin.Validate(Get("https://api.contoso.com/v1/pets", HttpStatusCode.OK, """[ { "meows": true }, { "barks": false } ]""").Session); + var invalid = plugin.Validate(Get("https://api.contoso.com/v1/pets", HttpStatusCode.OK, """[ { "quacks": true } ]""").Session); + + Assert.Empty(valid!.Issues); + Assert.Contains("$[0]: Value doesn't match any of the schemas in 'oneOf'", Messages(invalid)); + } + + [Fact] + public async Task IgnoresRequestsNotMatchingAnySpec() + { + var plugin = await CreatePluginAsync(); + + Assert.Null(plugin.Validate(Get("https://api.contoso.com/v2/users", HttpStatusCode.OK, "[]").Session)); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task StoresReportOnRecordingStop(bool reportUnmatchedRequests) + { + var plugin = await CreatePluginAsync($$"""{ "specsFolder": ".", "reportUnmatchedRequests": {{(reportUnmatchedRequests ? "true" : "false")}} }"""); + var args = Recording([ + Get("https://api.contoso.com/v1/users/1", HttpStatusCode.OK, """{ "id": 1, "name": "Alex" }""").AsRequestLog(MessageType.InterceptedResponse), + Get("https://api.contoso.com/v1/users/1", HttpStatusCode.OK, """{ "id": 1 }""").AsRequestLog(MessageType.InterceptedResponse), + Get("https://api.contoso.com/other", HttpStatusCode.OK, "{}").AsRequestLog(MessageType.InterceptedResponse), + ]); + + await plugin.AfterRecordingStopAsync(args, CancellationToken.None); + + var report = Assert.IsType( + ((Dictionary)args.GlobalData[ProxyUtils.ReportsKey])[plugin.Name]); + Assert.Equal(2, report.ValidatedRequests); + var result = Assert.Single(report.Results); + Assert.Equal("GET /users/{id}", result.Operation); + if (reportUnmatchedRequests) + { + Assert.Equal(["GET https://api.contoso.com/other"], report.UnmatchedRequests); + } + else + { + Assert.Null(report.UnmatchedRequests); + } + Assert.Contains("Missing required property 'name'", report.ToMarkdown(), StringComparison.Ordinal); + Assert.Contains("Missing required property 'name'", report.ToPlainText(), StringComparison.Ordinal); + } + + [Fact] + public async Task LoadsYamlOpenApi31SpecsFromFolder() + { + var specsDir = Directory.CreateDirectory(Path.Combine(_dir.FullName, "specs", "nested")); + await File.WriteAllTextAsync(Path.Combine(specsDir.FullName, "fabrikam.yaml"), """ + openapi: 3.1.0 + info: + title: Fabrikam + version: '1.0' + servers: + - url: https://{region}.fabrikam.com + variables: + region: + default: eu + paths: + /items: + get: + responses: + '200': + description: ok + content: + application/json: + schema: + type: object + properties: + name: + type: [string, 'null'] + """); + var plugin = await CreatePluginAsync("""{ "specsFolder": "specs" }"""); + + var valid = plugin.Validate(TestExchange.Request("GET", "https://eu.fabrikam.com/items") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json; charset=utf-8")], body: """{ "name": null }""").Session); + var invalid = plugin.Validate(TestExchange.Request("GET", "https://eu.fabrikam.com/items") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: """{ "name": 1 }""").Session); + + Assert.Equal("GET /items", valid?.Operation); + Assert.Empty(valid!.Issues); + Assert.Contains("$.name: Expected string or null but got integer", Messages(invalid)); + } + + [Fact] + public async Task ThrowsWhenNoSpecsConfigured() + { + _ = await Assert.ThrowsAsync(() => CreatePluginAsync("{}")); + } +} diff --git a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs new file mode 100644 index 00000000..6f6ead6d --- /dev/null +++ b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs @@ -0,0 +1,805 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using DevProxy.Abstractions.Plugins; +using DevProxy.Abstractions.Proxy; +using DevProxy.Abstractions.Proxy.Http; +using DevProxy.Abstractions.Utils; +using DevProxy.Plugins.Utils; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; +using Microsoft.OpenApi; +using Microsoft.OpenApi.Reader; +using System.Globalization; +using System.Runtime.CompilerServices; +using System.Text.Json; +using System.Text.RegularExpressions; + +namespace DevProxy.Plugins.Reporting; + +public sealed class OpenApiValidationPluginConfiguration +{ + public bool ReportUnmatchedRequests { get; set; } + public IEnumerable? SpecFiles { get; set; } + public string? SpecsFolder { get; set; } +} + +public sealed class OpenApiValidationPlugin( + HttpClient httpClient, + ILogger logger, + ISet urlsToWatch, + IProxyConfiguration proxyConfiguration, + IConfigurationSection pluginConfigurationSection) : + BaseReportingPlugin( + httpClient, + logger, + urlsToWatch, + proxyConfiguration, + pluginConfigurationSection) +{ + // headers that OpenAPI says to ignore when defined as header parameters + private static readonly string[] _ignoredHeaderParameters = ["Accept", "Content-Type", "Authorization"]; + + private readonly ConditionalWeakTable _outcomes = []; + private List _specs = []; + + public override string Name => nameof(OpenApiValidationPlugin); + + public override async Task InitializeAsync(InitArgs e, CancellationToken cancellationToken) + { + await base.InitializeAsync(e, cancellationToken); + + var specFiles = new List(); + + foreach (var specFile in Configuration.SpecFiles ?? []) + { + if (string.IsNullOrWhiteSpace(specFile)) + { + continue; + } + + var fullPath = ProxyUtils.GetFullPath(specFile, ProxyConfiguration.ConfigFile); + if (!File.Exists(fullPath)) + { + Enabled = false; + throw new InvalidOperationException($"Spec file '{fullPath}' does not exist."); + } + specFiles.Add(fullPath); + } + + if (!string.IsNullOrWhiteSpace(Configuration.SpecsFolder)) + { + var folderPath = ProxyUtils.GetFullPath(Configuration.SpecsFolder, ProxyConfiguration.ConfigFile); + if (!Directory.Exists(folderPath)) + { + Enabled = false; + throw new InvalidOperationException($"Specs folder '{folderPath}' does not exist."); + } + + specFiles.AddRange(Directory + .EnumerateFiles(folderPath, "*.*", SearchOption.AllDirectories) + .Where(IsSpecFile) + .Order(StringComparer.Ordinal)); + } + + if (Configuration.SpecFiles?.Any() != true && string.IsNullOrWhiteSpace(Configuration.SpecsFolder)) + { + Enabled = false; + throw new InvalidOperationException("Specify at least one OpenAPI spec using specFiles or specsFolder."); + } + + _specs = await LoadSpecsAsync(specFiles.Distinct(StringComparer.Ordinal), cancellationToken); + if (_specs.Count == 0) + { + Logger.LogWarning("No valid OpenAPI specs with absolute server URLs found. Requests won't be validated."); + } + } + + public override Task AfterResponseAsync(ProxyResponseArgs e, CancellationToken cancellationToken) + { + Logger.LogTrace("{Method} called", nameof(AfterResponseAsync)); + + ArgumentNullException.ThrowIfNull(e); + + var loggingContext = new LoggingContext(e.ProxySession); + if (!e.HasRequestUrlMatch(UrlsToWatch)) + { + Logger.LogRequest("URL not matched", MessageType.Skipped, loggingContext); + return Task.CompletedTask; + } + if (IsCorsPreflight(e.ProxySession.Request)) + { + Logger.LogRequest("Skipping CORS preflight request", MessageType.Skipped, loggingContext); + return Task.CompletedTask; + } + + var outcome = GetOrValidate(e.ProxySession); + if (outcome.Result is null) + { + Logger.LogRequest("No matching OpenAPI spec", MessageType.Skipped, loggingContext); + } + else if (outcome.Result.Issues.Any()) + { + foreach (var issue in outcome.Result.Issues) + { + Logger.LogRequest( + $"OpenAPI {issue.Location.ToString().ToLowerInvariant()} validation: {issue}", + MessageType.Warning, + loggingContext); + } + } + else + { + Logger.LogRequest($"Request and response match {outcome.Result.Operation}", MessageType.Processed, loggingContext); + } + + Logger.LogTrace("Left {Name}", nameof(AfterResponseAsync)); + return Task.CompletedTask; + } + + public override Task AfterRecordingStopAsync(RecordingArgs e, CancellationToken cancellationToken) + { + Logger.LogTrace("{Method} called", nameof(AfterRecordingStopAsync)); + + ArgumentNullException.ThrowIfNull(e); + + var sessions = e.RequestLogs + .Where(l => + l.MessageType == MessageType.InterceptedResponse && + l.Context?.Session is not null && + l.Context.Session.HasResponse && + ProxyUtils.MatchesUrlToWatch(UrlsToWatch, l.Context.Session.Request.RequestUri.AbsoluteUri) && + !IsCorsPreflight(l.Context.Session.Request)) + .Select(l => l.Context!.Session) + .Distinct(); + + if (!sessions.Any()) + { + Logger.LogRequest("No requests to process", MessageType.Skipped); + return Task.CompletedTask; + } + + var validated = 0; + var results = new List(); + var unmatched = new List(); + + foreach (var session in sessions) + { + cancellationToken.ThrowIfCancellationRequested(); + + var outcome = GetOrValidate(session); + if (outcome.Result is null) + { + unmatched.Add($"{session.Request.Method} {session.Request.Url}"); + continue; + } + + validated++; + if (outcome.Result.Issues.Any()) + { + results.Add(outcome.Result); + } + } + + if (Logger.IsEnabled(LogLevel.Information)) + { + Logger.LogInformation( + "Validated {Validated} request(s) against OpenAPI specs. {WithIssues} request(s) with issues.", + validated, + results.Count); + } + + var report = new OpenApiValidationPluginReport + { + ValidatedRequests = validated, + Results = results, + UnmatchedRequests = Configuration.ReportUnmatchedRequests ? [.. unmatched.Distinct()] : null + }; + StoreReport(report, e); + + Logger.LogTrace("Left {Name}", nameof(AfterRecordingStopAsync)); + return Task.CompletedTask; + } + + internal OpenApiValidationResult? Validate(IProxySession session) => GetOrValidate(session).Result; + + private ValidationOutcome GetOrValidate(IProxySession session) => + _outcomes.GetValue(session, s => new ValidationOutcome(ValidateSession(s))); + + private OpenApiValidationResult? ValidateSession(IProxySession session) + { + var request = session.Request; + var response = session.Response; + var requestText = $"{request.Method} {request.Url}"; + int? statusCode = response is null ? null : (int)response.StatusCode; + + var match = FindOperation(request); + if (match is null) + { + return null; + } + + var issues = new List(); + var specFile = match.Spec.FilePath; + + if (match.PathTemplate is null) + { + issues.Add(new() + { + Location = OpenApiValidationIssueLocation.Request, + Message = $"Path '{match.RelativePath}' isn't documented in the spec" + }); + return new() + { + Request = requestText, + StatusCode = statusCode, + SpecFile = specFile, + Issues = issues + }; + } + + if (match.Operation is null) + { + issues.Add(new() + { + Location = OpenApiValidationIssueLocation.Request, + Message = $"Method {request.Method} isn't documented for path '{match.PathTemplate}'" + }); + return new() + { + Request = requestText, + StatusCode = statusCode, + SpecFile = specFile, + Issues = issues + }; + } + + ValidateParameters(request, match, issues); + ValidateRequestBody(request, match.Operation, issues); + if (response is not null) + { + ValidateResponse(response, match.Operation, issues); + } + + return new() + { + Request = requestText, + StatusCode = statusCode, + Operation = $"{request.Method} {match.PathTemplate}", + SpecFile = specFile, + Issues = issues + }; + } + + private OperationMatch? FindOperation(IHttpRequest request) + { + var requestUrl = request.RequestUri.GetLeftPart(UriPartial.Path); + OperationMatch? firstServerMatch = null; + + foreach (var spec in _specs) + { + foreach (var serverRegex in spec.ServerRegexes) + { + var serverMatch = serverRegex.Match(requestUrl); + if (!serverMatch.Success) + { + continue; + } + + var relativePath = requestUrl[serverMatch.Length..]; + if (relativePath.Length > 0 && relativePath[0] != '/') + { + // server URL matched only part of a path segment + continue; + } + if (relativePath.Length == 0) + { + relativePath = "/"; + } + + var pathMatch = FindPath(spec, relativePath); + if (pathMatch is null) + { + firstServerMatch ??= new OperationMatch(spec, relativePath, null, null, null, []); + continue; + } + + var (template, pathItem, pathValues) = pathMatch.Value; + OpenApiOperation? operation = null; + _ = pathItem.Operations?.TryGetValue(new HttpMethod(request.Method), out operation); + return new OperationMatch(spec, relativePath, template, pathItem, operation, pathValues); + } + } + + return firstServerMatch; + } + + private static (string Template, IOpenApiPathItem PathItem, Dictionary PathValues)? FindPath(LoadedSpec spec, string relativePath) + { + // prefer literal paths over templated paths (e.g. /users/me over /users/{id}), + // and among templated paths, those with fewer parameters + foreach (var path in spec.Paths) + { + var m = path.Regex.Match(relativePath); + if (!m.Success) + { + continue; + } + + var values = new Dictionary(StringComparer.Ordinal); + for (var i = 0; i < path.ParameterNames.Count; i++) + { + values[path.ParameterNames[i]] = Uri.UnescapeDataString(m.Groups[i + 1].Value); + } + return (path.Template, path.PathItem, values); + } + + return null; + } + + private static void ValidateParameters(IHttpRequest request, OperationMatch match, List issues) + { + var parameters = GetEffectiveParameters(match.PathItem!, match.Operation!); + var query = ParseQueryString(request.RequestUri.Query); + + foreach (var parameter in parameters) + { + if (parameter.Name is null || parameter.In is null) + { + continue; + } + + List? values = null; + switch (parameter.In) + { + case ParameterLocation.Path: + if (match.PathValues.TryGetValue(parameter.Name, out var pathValue)) + { + values = [pathValue]; + } + break; + case ParameterLocation.Query: + if (query.TryGetValue(parameter.Name, out var queryValues)) + { + values = queryValues; + } + break; + case ParameterLocation.Header: + if (_ignoredHeaderParameters.Contains(parameter.Name, StringComparer.OrdinalIgnoreCase)) + { + continue; + } + var headerValues = request.Headers.GetAll(parameter.Name).Select(h => h.Value).ToList(); + if (headerValues.Count > 0) + { + values = headerValues; + } + break; + default: + // cookie and querystring parameters aren't validated + continue; + } + + var locationName = parameter.In.Value.ToString().ToLowerInvariant(); + if (values is null) + { + if (parameter.Required) + { + issues.Add(new() + { + Location = OpenApiValidationIssueLocation.Request, + Path = parameter.Name, + Message = $"Missing required {locationName} parameter '{parameter.Name}'" + }); + } + continue; + } + + ValidateParameterValue(parameter.Name, $"{locationName} parameter", values, parameter.Schema, + OpenApiValidationIssueLocation.Request, issues); + } + + // unknown query parameters + var hasQueryStringParameter = parameters.Any(p => p.In == ParameterLocation.QueryString); + if (hasQueryStringParameter) + { + return; + } + + var knownQueryParameters = parameters + .Where(p => p.In == ParameterLocation.Query && p.Name is not null) + .Select(p => p.Name!) + .ToArray(); + foreach (var name in query.Keys) + { + if (knownQueryParameters.Contains(name, StringComparer.Ordinal) || + // deepObject style, e.g. filter[name]=value + knownQueryParameters.Any(p => name.StartsWith($"{p}[", StringComparison.Ordinal))) + { + continue; + } + + issues.Add(new() + { + Location = OpenApiValidationIssueLocation.Request, + Path = name, + Message = $"Query parameter '{name}' isn't documented" + }); + } + } + + private static void ValidateParameterValue(string name, string kind, List values, IOpenApiSchema? schema, + OpenApiValidationIssueLocation location, List issues) + { + if (schema is null || schema.Type == JsonSchemaType.Object) + { + return; + } + + var value = OpenApiSchemaValidator.CoerceParameterValue(values, schema); + var direction = location == OpenApiValidationIssueLocation.Request + ? SchemaValidationDirection.Request + : SchemaValidationDirection.Response; + foreach (var error in OpenApiSchemaValidator.Validate(value, schema, direction, name)) + { + issues.Add(new() + { + Location = location, + Path = error.Path, + Message = $"Invalid {kind} value '{string.Join(",", values)}': {error.Message}" + }); + } + } + + private static void ValidateRequestBody(IHttpRequest request, OpenApiOperation operation, List issues) + { + var requestBody = operation.RequestBody; + var hasBody = request.Body.Length > 0; + + if (!hasBody) + { + if (requestBody?.Required == true && !request.HasBody) + { + issues.Add(new() + { + Location = OpenApiValidationIssueLocation.Request, + Message = "Missing required request body" + }); + } + return; + } + + if (requestBody is null) + { + issues.Add(new() + { + Location = OpenApiValidationIssueLocation.Request, + Message = "Request body isn't documented for this operation" + }); + return; + } + + ValidateBody(request, requestBody.Content, OpenApiValidationIssueLocation.Request, issues); + } + + private static void ValidateResponse(IHttpResponse response, OpenApiOperation operation, List issues) + { + var statusCode = ((int)response.StatusCode).ToString(CultureInfo.InvariantCulture); + var responses = operation.Responses; + IOpenApiResponse? specResponse = null; + if (responses is not null) + { + specResponse = + responses.FirstOrDefault(r => r.Key == statusCode).Value ?? + responses.FirstOrDefault(r => string.Equals(r.Key, $"{statusCode[0]}XX", StringComparison.OrdinalIgnoreCase)).Value ?? + responses.FirstOrDefault(r => string.Equals(r.Key, "default", StringComparison.OrdinalIgnoreCase)).Value; + } + + if (specResponse is null) + { + issues.Add(new() + { + Location = OpenApiValidationIssueLocation.Response, + Message = $"Status code {statusCode} isn't documented for this operation" + }); + return; + } + + foreach (var (headerName, header) in specResponse.Headers ?? new Dictionary()) + { + if (string.Equals(headerName, "Content-Type", StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + var values = response.Headers.GetAll(headerName).Select(h => h.Value).ToList(); + if (values.Count == 0) + { + if (header.Required) + { + issues.Add(new() + { + Location = OpenApiValidationIssueLocation.Response, + Path = headerName, + Message = $"Missing required response header '{headerName}'" + }); + } + continue; + } + + ValidateParameterValue(headerName, "response header", values, header.Schema, + OpenApiValidationIssueLocation.Response, issues); + } + + if (response.Body.Length == 0) + { + return; + } + + if (specResponse.Content is not { Count: > 0 }) + { + issues.Add(new() + { + Location = OpenApiValidationIssueLocation.Response, + Message = $"Response body isn't documented for status code {statusCode}" + }); + return; + } + + ValidateBody(response, specResponse.Content, OpenApiValidationIssueLocation.Response, issues); + } + + private static void ValidateBody(IHttpMessage message, IDictionary? content, + OpenApiValidationIssueLocation location, List issues) + { + if (content is not { Count: > 0 }) + { + return; + } + + var locationName = location.ToString().ToLowerInvariant(); + var contentType = GetMediaType(message.ContentType); + if (contentType is null) + { + issues.Add(new() + { + Location = location, + Message = $"The {locationName} has a body but no Content-Type header" + }); + return; + } + + var mediaType = FindMediaType(content, contentType); + if (mediaType is null) + { + issues.Add(new() + { + Location = location, + Message = $"Content type '{contentType}' isn't documented. Documented: {string.Join(", ", content.Keys)}" + }); + return; + } + + if (mediaType.Schema is null || !IsJsonMediaType(contentType)) + { + return; + } + + JsonDocument json; + try + { + json = JsonDocument.Parse(message.Body); + } + catch (JsonException ex) + { + issues.Add(new() + { + Location = location, + Message = $"The {locationName} body isn't valid JSON: {ex.Message}" + }); + return; + } + + using (json) + { + var direction = location == OpenApiValidationIssueLocation.Request + ? SchemaValidationDirection.Request + : SchemaValidationDirection.Response; + foreach (var error in OpenApiSchemaValidator.Validate(json.RootElement, mediaType.Schema, direction)) + { + issues.Add(new() + { + Location = location, + Path = error.Path, + Message = error.Message + }); + } + } + } + + private static IOpenApiMediaType? FindMediaType(IDictionary content, string contentType) + { + var entries = content + .Select(c => (MediaType: GetMediaType(c.Key) ?? c.Key, c.Value)) + .ToArray(); + + var exact = entries.FirstOrDefault(c => string.Equals(c.MediaType, contentType, StringComparison.OrdinalIgnoreCase)); + if (exact.Value is not null) + { + return exact.Value; + } + + var typeWildcard = $"{contentType.Split('/')[0]}/*"; + var wildcard = entries.FirstOrDefault(c => string.Equals(c.MediaType, typeWildcard, StringComparison.OrdinalIgnoreCase)); + if (wildcard.Value is not null) + { + return wildcard.Value; + } + + return entries.FirstOrDefault(c => c.MediaType == "*/*").Value; + } + + private static string? GetMediaType(string? contentType) + { + if (string.IsNullOrWhiteSpace(contentType)) + { + return null; + } + + return contentType.Split(';')[0].Trim().ToLowerInvariant(); + } + + private static bool IsJsonMediaType(string mediaType) => + mediaType == "application/json" || + mediaType.EndsWith("+json", StringComparison.Ordinal); + + private static List GetEffectiveParameters(IOpenApiPathItem pathItem, OpenApiOperation operation) + { + var parameters = new List(); + parameters.AddRange(pathItem.Parameters ?? []); + + // operation-level parameters override path-level parameters with the same name and location + foreach (var parameter in operation.Parameters ?? []) + { + _ = parameters.RemoveAll(p => p.Name == parameter.Name && p.In == parameter.In); + parameters.Add(parameter); + } + + return parameters; + } + + private static Dictionary> ParseQueryString(string query) + { + var result = new Dictionary>(StringComparer.Ordinal); + if (string.IsNullOrEmpty(query)) + { + return result; + } + + foreach (var pair in query.TrimStart('?').Split('&', StringSplitOptions.RemoveEmptyEntries)) + { + var separatorIndex = pair.IndexOf('=', StringComparison.Ordinal); + var name = Unescape(separatorIndex < 0 ? pair : pair[..separatorIndex]); + var value = separatorIndex < 0 ? string.Empty : Unescape(pair[(separatorIndex + 1)..]); + + if (!result.TryGetValue(name, out var values)) + { + values = []; + result[name] = values; + } + values.Add(value); + } + + return result; + } + + private static string Unescape(string value) => + Uri.UnescapeDataString(value.Replace('+', ' ')); + + private static bool IsCorsPreflight(IHttpRequest request) => + string.Equals(request.Method, "OPTIONS", StringComparison.OrdinalIgnoreCase) && + request.Headers.Contains("Access-Control-Request-Method"); + + private static bool IsSpecFile(string file) + { + var extension = Path.GetExtension(file); + return extension.Equals(".json", StringComparison.OrdinalIgnoreCase) || + extension.Equals(".yaml", StringComparison.OrdinalIgnoreCase) || + extension.Equals(".yml", StringComparison.OrdinalIgnoreCase); + } + + private async Task> LoadSpecsAsync(IEnumerable files, CancellationToken cancellationToken) + { + var specs = new List(); + + foreach (var file in files) + { + cancellationToken.ThrowIfCancellationRequested(); + + Logger.LogDebug("Loading OpenAPI spec from '{File}'...", file); + OpenApiDocument? document; + string? diagnostics; + try + { + var fileContents = await File.ReadAllTextAsync(file, cancellationToken); + fileContents = ProxyUtils.ReplaceVariables(fileContents, ProxyConfiguration.Env, v => $"{{{v}}}"); + + var settings = new OpenApiReaderSettings(); + settings.AddYamlReader(); + var readResult = OpenApiDocument.Parse(fileContents, null, settings); + document = readResult.Document; + diagnostics = readResult.Diagnostic?.Errors is { Count: > 0 } errors + ? string.Join("; ", errors.Select(e => e.ToString())) + : null; + } + catch (Exception ex) + { + Logger.LogError(ex, "Failed to load OpenAPI spec from '{File}'", file); + continue; + } + + if (document is null) + { + Logger.LogWarning("File '{File}' isn't a valid OpenAPI spec. Skipping. {Diagnostics}", file, diagnostics); + continue; + } + + var serverRegexes = new List(); + foreach (var server in document.Servers ?? []) + { + if (string.IsNullOrWhiteSpace(server.Url) || + !server.Url.Contains("://", StringComparison.Ordinal)) + { + Logger.LogDebug("Skipping server '{ServerUrl}' in '{File}' because it isn't an absolute URL", server.Url, file); + continue; + } + + var pattern = Regex.Replace(Regex.Escape(server.Url.TrimEnd('/')), @"\\\{[^}]+\}", "[^/]*"); + serverRegexes.Add(new Regex($"^{pattern}", RegexOptions.IgnoreCase | RegexOptions.CultureInvariant)); + } + + if (serverRegexes.Count == 0) + { + Logger.LogWarning("OpenAPI spec '{File}' doesn't define any absolute server URLs. Skipping", file); + continue; + } + + var paths = (document.Paths ?? []) + .Select(p => CompilePath(p.Key, p.Value)) + .OrderBy(p => p.ParameterNames.Count) + .ToList(); + + specs.Add(new LoadedSpec(file, document, serverRegexes, paths)); + Logger.LogInformation("Loaded OpenAPI spec '{File}'", file); + } + + return specs; + } + + private static CompiledPath CompilePath(string template, IOpenApiPathItem pathItem) + { + var parameterNames = new List(); + var pattern = Regex.Replace(Regex.Escape(template), @"\\\{([^}]+)\}", m => + { + parameterNames.Add(Regex.Unescape(m.Groups[1].Value)); + return "([^/]+)"; + }); + var regex = new Regex($"^{pattern}/?$", RegexOptions.IgnoreCase | RegexOptions.CultureInvariant); + return new CompiledPath(template, pathItem, regex, parameterNames); + } + + private sealed record CompiledPath(string Template, IOpenApiPathItem PathItem, Regex Regex, List ParameterNames); + + private sealed record LoadedSpec(string FilePath, OpenApiDocument Document, List ServerRegexes, List Paths); + + private sealed record OperationMatch( + LoadedSpec Spec, + string RelativePath, + string? PathTemplate, + IOpenApiPathItem? PathItem, + OpenApiOperation? Operation, + Dictionary PathValues); + + private sealed record ValidationOutcome(OpenApiValidationResult? Result); +} diff --git a/DevProxy.Plugins/Reporting/OpenApiValidationPluginReport.cs b/DevProxy.Plugins/Reporting/OpenApiValidationPluginReport.cs new file mode 100644 index 00000000..fda2af37 --- /dev/null +++ b/DevProxy.Plugins/Reporting/OpenApiValidationPluginReport.cs @@ -0,0 +1,154 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using DevProxy.Abstractions.Plugins; +using System.Globalization; +using System.Text; + +namespace DevProxy.Plugins.Reporting; + +public enum OpenApiValidationIssueLocation +{ + Request, + Response +} + +public sealed class OpenApiValidationIssue +{ + public required OpenApiValidationIssueLocation Location { get; init; } + public required string Message { get; init; } + /// + /// Location of the issue in the request or response, e.g. the parameter + /// name or a JSON path in the body. Empty when the issue applies to the + /// whole request or response. + /// + public string? Path { get; init; } + + public override string ToString() => + string.IsNullOrEmpty(Path) ? Message : $"{Path}: {Message}"; +} + +public sealed class OpenApiValidationResult +{ + public required string Request { get; init; } + public int? StatusCode { get; init; } + /// + /// Matched operation, e.g. GET /users/{id}. Empty when the request + /// isn't documented in the spec. + /// + public string? Operation { get; init; } + public string? SpecFile { get; init; } + public required IEnumerable Issues { get; init; } +} + +public sealed class OpenApiValidationPluginReport : IMarkdownReport, IPlainTextReport +{ + public required int ValidatedRequests { get; init; } + public required IEnumerable Results { get; init; } + public IEnumerable? UnmatchedRequests { get; init; } + + public string? ToMarkdown() + { + var sb = new StringBuilder(); + _ = sb.AppendLine("# OpenAPI validation report") + .AppendLine() + .AppendLine(CultureInfo.InvariantCulture, $"- Validated requests: {ValidatedRequests}") + .AppendLine(CultureInfo.InvariantCulture, $"- Requests with issues: {Results.Count()}"); + + if (!Results.Any()) + { + _ = sb.AppendLine() + .AppendLine("✅ All validated requests and responses match their OpenAPI specs."); + } + + foreach (var result in Results) + { + _ = sb.AppendLine() + .AppendLine(CultureInfo.InvariantCulture, $"## {result.Request}") + .AppendLine(); + + if (result.StatusCode is not null) + { + _ = sb.AppendLine(CultureInfo.InvariantCulture, $"- Status code: {result.StatusCode}"); + } + if (!string.IsNullOrEmpty(result.Operation)) + { + _ = sb.AppendLine(CultureInfo.InvariantCulture, $"- Operation: `{result.Operation}`"); + } + if (!string.IsNullOrEmpty(result.SpecFile)) + { + _ = sb.AppendLine(CultureInfo.InvariantCulture, $"- Spec: {result.SpecFile}"); + } + + _ = sb.AppendLine() + .AppendLine("|Location|Path|Issue|") + .AppendLine("|--------|----|-----|"); + foreach (var issue in result.Issues) + { + var issuePath = string.IsNullOrEmpty(issue.Path) ? "" : $"`{issue.Path}`"; + _ = sb.AppendLine(CultureInfo.InvariantCulture, + $"|{issue.Location.ToString().ToLowerInvariant()}|{issuePath}|{EscapeTableCell(issue.Message)}|"); + } + } + + if (UnmatchedRequests?.Any() == true) + { + _ = sb.AppendLine() + .AppendLine("## Requests not matching any OpenAPI spec") + .AppendLine() + .AppendJoin(Environment.NewLine, UnmatchedRequests.Select(r => $"- {r}")) + .AppendLine(); + } + + return sb.ToString(); + } + + public string? ToPlainText() + { + var sb = new StringBuilder(); + _ = sb.AppendLine("OpenAPI validation report") + .AppendLine() + .AppendLine(CultureInfo.InvariantCulture, $"Validated requests: {ValidatedRequests}") + .AppendLine(CultureInfo.InvariantCulture, $"Requests with issues: {Results.Count()}"); + + foreach (var result in Results) + { + _ = sb.AppendLine() + .AppendLine(result.Request); + + if (result.StatusCode is not null) + { + _ = sb.AppendLine(CultureInfo.InvariantCulture, $"Status code: {result.StatusCode}"); + } + if (!string.IsNullOrEmpty(result.Operation)) + { + _ = sb.AppendLine(CultureInfo.InvariantCulture, $"Operation: {result.Operation}"); + } + if (!string.IsNullOrEmpty(result.SpecFile)) + { + _ = sb.AppendLine(CultureInfo.InvariantCulture, $"Spec: {result.SpecFile}"); + } + + foreach (var issue in result.Issues) + { + _ = sb.AppendLine(CultureInfo.InvariantCulture, + $"- [{issue.Location.ToString().ToLowerInvariant()}] {issue}"); + } + } + + if (UnmatchedRequests?.Any() == true) + { + _ = sb.AppendLine() + .AppendLine("Requests not matching any OpenAPI spec:") + .AppendLine() + .AppendJoin(Environment.NewLine, UnmatchedRequests.Select(r => $"- {r}")) + .AppendLine(); + } + + return sb.ToString(); + } + + private static string EscapeTableCell(string value) => + value.Replace("|", "\\|", StringComparison.Ordinal); +} diff --git a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs new file mode 100644 index 00000000..744ecb0b --- /dev/null +++ b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs @@ -0,0 +1,300 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Microsoft.OpenApi; +using System.Globalization; +using System.Text.Json; +using System.Text.Json.Nodes; + +namespace DevProxy.Plugins.Utils; + +internal enum SchemaValidationDirection +{ + Request, + Response +} + +internal sealed record SchemaValidationError(string Path, string Message); + +/// +/// Lightweight validator of JSON values against OpenAPI schemas +/// (Microsoft.OpenApi object model). Supports: type (incl. nullable / type null), +/// required, properties, additionalProperties, items, enum, allOf, anyOf, oneOf +/// and $ref. Other keywords (format, min/max, pattern, etc.) are ignored. +/// +internal static class OpenApiSchemaValidator +{ + private const int MaxDepth = 64; + + public static List Validate( + JsonElement value, + IOpenApiSchema? schema, + SchemaValidationDirection direction, + string path = "$") + { + var errors = new List(); + ValidateCore(value, schema, direction, path, errors, 0); + return errors; + } + + public static List Validate( + JsonNode? value, + IOpenApiSchema? schema, + SchemaValidationDirection direction, + string path = "$") + { + using var doc = JsonDocument.Parse(value?.ToJsonString() ?? "null"); + return Validate(doc.RootElement.Clone(), schema, direction, path); + } + + /// + /// Converts a raw (string) parameter value into a JSON value, coercing it + /// to the primitive type declared by the schema when possible, so that it + /// can be validated with . + /// + public static JsonNode? CoerceParameterValue(IReadOnlyList values, IOpenApiSchema? schema) + { + var type = schema?.Type; + if (type is not null && type.Value.HasFlag(JsonSchemaType.Array)) + { + var items = values.Count == 1 + ? values[0].Split(',') + : [.. values]; + var array = new JsonArray(); + foreach (var item in items) + { + array.Add(CoercePrimitive(item, schema!.Items)); + } + return array; + } + + return CoercePrimitive(values.Count > 0 ? values[0] : string.Empty, schema); + } + + private static JsonValue? CoercePrimitive(string value, IOpenApiSchema? schema) + { + var type = schema?.Type; + if (type is null) + { + return JsonValue.Create(value); + } + + if (type.Value.HasFlag(JsonSchemaType.Integer) && + long.TryParse(value, NumberStyles.Integer, CultureInfo.InvariantCulture, out var l)) + { + return JsonValue.Create(l); + } + if (type.Value.HasFlag(JsonSchemaType.Number) && + double.TryParse(value, NumberStyles.Float, CultureInfo.InvariantCulture, out var d)) + { + return JsonValue.Create(d); + } + if (type.Value.HasFlag(JsonSchemaType.Boolean) && bool.TryParse(value, out var b)) + { + return JsonValue.Create(b); + } + if (type.Value.HasFlag(JsonSchemaType.Null) && value.Length == 0) + { + return null; + } + + return JsonValue.Create(value); + } + + private static void ValidateCore( + JsonElement value, + IOpenApiSchema? schema, + SchemaValidationDirection direction, + string path, + List errors, + int depth) + { + if (schema is null || depth > MaxDepth) + { + return; + } + + if (!ValidateType(value, schema, path, errors)) + { + // no point validating further if the type doesn't match + return; + } + + ValidateEnum(value, schema, path, errors); + + if (value.ValueKind == JsonValueKind.Object) + { + ValidateObject(value, schema, direction, path, errors, depth); + } + else if (value.ValueKind == JsonValueKind.Array && schema.Items is not null) + { + var i = 0; + foreach (var item in value.EnumerateArray()) + { + ValidateCore(item, schema.Items, direction, $"{path}[{i}]", errors, depth + 1); + i++; + } + } + + if (schema.AllOf is { Count: > 0 }) + { + foreach (var subSchema in schema.AllOf) + { + ValidateCore(value, subSchema, direction, path, errors, depth + 1); + } + } + + if (schema.AnyOf is { Count: > 0 } && + !schema.AnyOf.Any(s => IsValid(value, s, direction, path, depth))) + { + errors.Add(new(path, "Value doesn't match any of the schemas in 'anyOf'")); + } + + if (schema.OneOf is { Count: > 0 }) + { + var matches = schema.OneOf.Count(s => IsValid(value, s, direction, path, depth)); + if (matches == 0) + { + errors.Add(new(path, "Value doesn't match any of the schemas in 'oneOf'")); + } + else if (matches > 1) + { + errors.Add(new(path, $"Value matches {matches} schemas in 'oneOf' but must match exactly one")); + } + } + } + + private static bool IsValid(JsonElement value, IOpenApiSchema schema, SchemaValidationDirection direction, string path, int depth) + { + var subErrors = new List(); + ValidateCore(value, schema, direction, path, subErrors, depth + 1); + return subErrors.Count == 0; + } + + private static bool ValidateType(JsonElement value, IOpenApiSchema schema, string path, List errors) + { + if (schema.Type is not { } type || type == 0) + { + return true; + } + + var matches = value.ValueKind switch + { + JsonValueKind.Null => type.HasFlag(JsonSchemaType.Null), + JsonValueKind.True or JsonValueKind.False => type.HasFlag(JsonSchemaType.Boolean), + JsonValueKind.String => type.HasFlag(JsonSchemaType.String), + JsonValueKind.Object => type.HasFlag(JsonSchemaType.Object), + JsonValueKind.Array => type.HasFlag(JsonSchemaType.Array), + JsonValueKind.Number => type.HasFlag(JsonSchemaType.Number) || + (type.HasFlag(JsonSchemaType.Integer) && IsInteger(value)), + JsonValueKind.Undefined => true, + _ => true + }; + + if (!matches) + { + errors.Add(new(path, $"Expected {DescribeType(type)} but got {DescribeValueKind(value)}")); + } + + return matches; + } + + private static bool IsInteger(JsonElement value) => + value.TryGetInt64(out _) || + (value.TryGetDecimal(out var d) && decimal.Truncate(d) == d); + + private static void ValidateEnum(JsonElement value, IOpenApiSchema schema, string path, List errors) + { + if (schema.Enum is not { Count: > 0 }) + { + return; + } + + var node = JsonNode.Parse(value.GetRawText()); + if (schema.Enum.Any(e => JsonNode.DeepEquals(e, node))) + { + return; + } + + // OAS 3.0 nullable enums: null is allowed if the schema allows null + if (node is null && schema.Type is { } type && type.HasFlag(JsonSchemaType.Null)) + { + return; + } + + var allowed = string.Join(", ", schema.Enum.Select(e => e?.ToJsonString() ?? "null")); + errors.Add(new(path, $"Value {value.GetRawText()} isn't one of the allowed values: {allowed}")); + } + + private static void ValidateObject( + JsonElement value, + IOpenApiSchema schema, + SchemaValidationDirection direction, + string path, + List errors, + int depth) + { + if (schema.Required is { Count: > 0 }) + { + foreach (var requiredProperty in schema.Required) + { + if (value.TryGetProperty(requiredProperty, out _)) + { + continue; + } + + // readOnly properties aren't sent in requests, writeOnly + // properties aren't returned in responses + IOpenApiSchema? propertySchema = null; + _ = schema.Properties?.TryGetValue(requiredProperty, out propertySchema); + if (propertySchema is not null && + ((direction == SchemaValidationDirection.Request && propertySchema.ReadOnly) || + (direction == SchemaValidationDirection.Response && propertySchema.WriteOnly))) + { + continue; + } + + errors.Add(new(path, $"Missing required property '{requiredProperty}'")); + } + } + + foreach (var property in value.EnumerateObject()) + { + var propertyPath = $"{path}.{property.Name}"; + if (schema.Properties is not null && + schema.Properties.TryGetValue(property.Name, out var propertySchema)) + { + ValidateCore(property.Value, propertySchema, direction, propertyPath, errors, depth + 1); + } + else if (schema.AdditionalProperties is not null) + { + ValidateCore(property.Value, schema.AdditionalProperties, direction, propertyPath, errors, depth + 1); + } + else if (!schema.AdditionalPropertiesAllowed) + { + errors.Add(new(propertyPath, $"Property '{property.Name}' isn't defined in the schema")); + } + } + } + + private static string DescribeType(JsonSchemaType type) + { + var names = Enum.GetValues() + .Where(t => type.HasFlag(t)) + .OrderBy(t => t == JsonSchemaType.Null) + .Select(t => t.ToString().ToLowerInvariant()); + return string.Join(" or ", names); + } + + private static string DescribeValueKind(JsonElement value) => value.ValueKind switch + { + JsonValueKind.Null => "null", + JsonValueKind.True or JsonValueKind.False => "boolean", + JsonValueKind.String => "string", + JsonValueKind.Object => "object", + JsonValueKind.Array => "array", + JsonValueKind.Number => IsInteger(value) ? "integer" : "number", + _ => value.ValueKind.ToString().ToLowerInvariant() + }; +} diff --git a/schemas/v4.1.0/openapivalidationplugin.schema.json b/schemas/v4.1.0/openapivalidationplugin.schema.json new file mode 100644 index 00000000..9eca2386 --- /dev/null +++ b/schemas/v4.1.0/openapivalidationplugin.schema.json @@ -0,0 +1,40 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "title": "Dev Proxy OpenApiValidationPlugin config schema", + "type": "object", + "properties": { + "$schema": { + "type": "string", + "description": "The JSON schema reference for validation." + }, + "specFiles": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Relative or absolute paths to OpenAPI spec files (JSON or YAML) to validate requests and responses against." + }, + "specsFolder": { + "type": "string", + "description": "Relative or absolute path to a folder with OpenAPI spec files (JSON or YAML). The folder is scanned recursively." + }, + "reportUnmatchedRequests": { + "type": "boolean", + "default": false, + "description": "Set to true to list requests that don't match any of the OpenAPI specs in the report." + } + }, + "anyOf": [ + { + "required": [ + "specFiles" + ] + }, + { + "required": [ + "specsFolder" + ] + } + ], + "additionalProperties": false +} From 5f034ba90c9ab787e58ea2b5e382316208865eb3 Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 11:15:26 +0100 Subject: [PATCH 02/13] Address review feedback for OpenApiValidationPlugin - Report schemas nested deeper than the supported maximum instead of treating them as valid - Search all specs and matching paths for the requested method before reporting it as undocumented - Recognize required deepObject query parameters - Match operations against their effective (document-, path- or operation-level) servers - Match server base paths and request paths case-sensitively and strictly on trailing slashes - Decode array parameters according to their style and explode settings - Reject non-finite numbers when coercing parameter values Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OpenApiValidationPluginTests.cs | 157 +++++++++++++++ .../Reporting/OpenApiValidationPlugin.cs | 183 ++++++++++++++---- .../Utils/OpenApiSchemaValidator.cs | 73 ++++++- 3 files changed, 361 insertions(+), 52 deletions(-) diff --git a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs index 254540b3..ac683cdf 100644 --- a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs +++ b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs @@ -406,4 +406,161 @@ public async Task ThrowsWhenNoSpecsConfigured() { _ = await Assert.ThrowsAsync(() => CreatePluginAsync("{}")); } + + private async Task CreatePluginWithSpecsAsync(params (string Name, string Content)[] specs) + { + foreach (var (name, content) in specs) + { + await File.WriteAllTextAsync(Path.Combine(_dir.FullName, name), content); + } + var files = string.Join(", ", specs.Select(s => $"\"{s.Name}\"")); + return await CreatePluginAsync($$"""{ "specFiles": [ {{files}} ] }"""); + } + + private static TestExchange Call(string method, string url, IEnumerable<(string, string)>? requestHeaders = null) => + TestExchange.Request(method, url, headers: requestHeaders).WithResponse(HttpStatusCode.NoContent); + + [Fact] + public async Task FindsMethodAcrossSpecsSharingServerAndPath() + { + var plugin = await CreatePluginWithSpecsAsync( + ("read.yaml", """ + openapi: 3.0.3 + info: { title: Read, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /users: + get: + responses: { '204': { description: ok } } + """), + ("write.yaml", """ + openapi: 3.0.3 + info: { title: Write, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /users: + post: + responses: { '204': { description: ok } } + """)); + + var post = plugin.Validate(Call("POST", "https://api.fabrikam.com/users").Session); + var delete = plugin.Validate(Call("DELETE", "https://api.fabrikam.com/users").Session); + + Assert.Equal("POST /users", post?.Operation); + Assert.EndsWith("write.yaml", post!.SpecFile, StringComparison.Ordinal); + Assert.Empty(post.Issues); + Assert.Contains("Method DELETE isn't documented", Assert.Single(Messages(delete)), StringComparison.Ordinal); + } + + [Fact] + public async Task MatchesPathAndOperationLevelServers() + { + var plugin = await CreatePluginWithSpecsAsync(("servers.yaml", """ + openapi: 3.0.3 + info: { title: Servers, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /users: + get: + responses: { '204': { description: ok } } + post: + servers: + - url: https://write.fabrikam.com + responses: { '204': { description: ok } } + /files: + servers: + - url: https://files.fabrikam.com + get: + responses: { '204': { description: ok } } + """)); + + Assert.Empty(plugin.Validate(Call("GET", "https://api.fabrikam.com/users").Session)!.Issues); + Assert.Empty(plugin.Validate(Call("POST", "https://write.fabrikam.com/users").Session)!.Issues); + Assert.Empty(plugin.Validate(Call("GET", "https://files.fabrikam.com/files").Session)!.Issues); + Assert.Contains("Method POST isn't documented", Assert.Single(Messages(plugin.Validate(Call("POST", "https://api.fabrikam.com/users").Session))), StringComparison.Ordinal); + Assert.Contains("Path '/files' isn't documented", Assert.Single(Messages(plugin.Validate(Call("GET", "https://api.fabrikam.com/files").Session))), StringComparison.Ordinal); + } + + [Fact] + public async Task MatchesPathsCaseSensitivelyAndStrictlyOnTrailingSlash() + { + var plugin = await CreatePluginAsync(); + + Assert.Null(plugin.Validate(Get("https://API.contoso.com/V1/users/me", HttpStatusCode.OK, "{}").Session)); + Assert.Equal("GET /users/me", plugin.Validate(Get("https://API.contoso.com/v1/users/me", HttpStatusCode.OK, """{ "id": 1, "name": "Alex" }""").Session)?.Operation); + Assert.Contains("Path '/Users/me' isn't documented", Assert.Single(Messages(plugin.Validate(Get("https://api.contoso.com/v1/Users/me", HttpStatusCode.OK, "{}").Session))), StringComparison.Ordinal); + Assert.Contains("Path '/users/me/' isn't documented", Assert.Single(Messages(plugin.Validate(Get("https://api.contoso.com/v1/users/me/", HttpStatusCode.OK, "{}").Session))), StringComparison.Ordinal); + } + + [Fact] + public async Task DecodesParametersAccordingToStyleAndExplode() + { + var plugin = await CreatePluginWithSpecsAsync(("styles.yaml", """ + openapi: 3.0.3 + info: { title: Styles, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /items/{ids}: + get: + parameters: + - { name: ids, in: path, required: true, style: label, schema: { type: array, items: { type: integer } } } + - { name: form, in: query, schema: { type: array, items: { type: integer } } } + - { name: csv, in: query, explode: false, schema: { type: array, items: { type: integer } } } + - { name: pipes, in: query, style: pipeDelimited, explode: false, schema: { type: array, items: { type: integer } } } + - { name: x-ids, in: header, schema: { type: array, items: { type: integer } } } + - { name: filter, in: query, required: true, style: deepObject, schema: { type: object } } + - { name: score, in: query, schema: { type: number } } + responses: { '204': { description: ok } } + """)); + + var valid = plugin.Validate(Call("GET", + "https://api.fabrikam.com/items/.1,2?form=1&form=2&csv=1,2&pipes=1%7C2&filter[name]=Alex&score=1.5", + [("x-ids", "1, 2")]).Session); + var invalid = plugin.Validate(Call("GET", + "https://api.fabrikam.com/items/.1,2?form=1,2&filter[name]=Alex&score=NaN").Session); + + Assert.Empty(valid!.Issues); + var messages = Messages(invalid); + Assert.Contains(messages, m => m.StartsWith("form[0]:", StringComparison.Ordinal) && m.Contains("Expected integer", StringComparison.Ordinal)); + Assert.Contains(messages, m => m.StartsWith("score:", StringComparison.Ordinal) && m.Contains("Expected number", StringComparison.Ordinal)); + Assert.Equal(2, messages.Length); + } + + [Fact] + public async Task ReportsSchemasExceedingMaxDepth() + { + var schemas = string.Join(",\n", Enumerable.Range(0, 70).Select(i => + $$""" "S{{i}}": { "allOf": [ { "$ref": "#/components/schemas/S{{i + 1}}" } ] }""")); + var plugin = await CreatePluginWithSpecsAsync(("deep.json", $$""" + { + "openapi": "3.0.3", + "info": { "title": "Deep", "version": "1.0" }, + "servers": [ { "url": "https://api.fabrikam.com" } ], + "paths": { + "/deep": { + "get": { + "responses": { + "200": { "description": "ok", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/S0" } } } } + } + } + } + }, + "components": { + "schemas": { + {{schemas}}, + "S70": { "type": "string" } + } + } + } + """)); + + var result = plugin.Validate(TestExchange.Request("GET", "https://api.fabrikam.com/deep") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: "1").Session); + + Assert.Contains(Messages(result), m => m.Contains("supported maximum", StringComparison.Ordinal)); + } } diff --git a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs index 6f6ead6d..445b8ccf 100644 --- a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs +++ b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs @@ -275,13 +275,17 @@ private ValidationOutcome GetOrValidate(IProxySession session) => private OperationMatch? FindOperation(IHttpRequest request) { var requestUrl = request.RequestUri.GetLeftPart(UriPartial.Path); + var method = new HttpMethod(request.Method); + OperationMatch? firstPathMatch = null; OperationMatch? firstServerMatch = null; + // prefer a spec that documents the requested method; fall back to + // a matching path without the method, and then to a matching server foreach (var spec in _specs) { - foreach (var serverRegex in spec.ServerRegexes) + foreach (var server in spec.Servers) { - var serverMatch = serverRegex.Match(requestUrl); + var serverMatch = server.Regex.Match(requestUrl); if (!serverMatch.Success) { continue; @@ -298,28 +302,32 @@ private ValidationOutcome GetOrValidate(IProxySession session) => relativePath = "/"; } - var pathMatch = FindPath(spec, relativePath); - if (pathMatch is null) + var matchedAnyPath = false; + foreach (var (path, pathValues) in FindPaths(server, relativePath)) { - firstServerMatch ??= new OperationMatch(spec, relativePath, null, null, null, []); - continue; + matchedAnyPath = true; + if (path.Operations.TryGetValue(method, out var operation)) + { + return new OperationMatch(spec, relativePath, path.Template, path.PathItem, operation, pathValues); + } + firstPathMatch ??= new OperationMatch(spec, relativePath, path.Template, path.PathItem, null, pathValues); } - var (template, pathItem, pathValues) = pathMatch.Value; - OpenApiOperation? operation = null; - _ = pathItem.Operations?.TryGetValue(new HttpMethod(request.Method), out operation); - return new OperationMatch(spec, relativePath, template, pathItem, operation, pathValues); + if (!matchedAnyPath) + { + firstServerMatch ??= new OperationMatch(spec, relativePath, null, null, null, []); + } } } - return firstServerMatch; + return firstPathMatch ?? firstServerMatch; } - private static (string Template, IOpenApiPathItem PathItem, Dictionary PathValues)? FindPath(LoadedSpec spec, string relativePath) + private static IEnumerable<(CompiledPath Path, Dictionary PathValues)> FindPaths(ServerScope server, string relativePath) { - // prefer literal paths over templated paths (e.g. /users/me over /users/{id}), - // and among templated paths, those with fewer parameters - foreach (var path in spec.Paths) + // paths are ordered so that literal paths are preferred over templated paths + // (e.g. /users/me over /users/{id}), and among templated paths, those with fewer parameters + foreach (var path in server.Paths) { var m = path.Regex.Match(relativePath); if (!m.Success) @@ -332,10 +340,8 @@ private static (string Template, IOpenApiPathItem PathItem, Dictionary issues) @@ -364,6 +370,12 @@ private static void ValidateParameters(IHttpRequest request, OperationMatch matc { values = queryValues; } + else if (parameter.Style == ParameterStyle.DeepObject && + query.Keys.Any(k => k.StartsWith($"{parameter.Name}[", StringComparison.Ordinal))) + { + // deepObject values (e.g. filter[name]=value) aren't validated + continue; + } break; case ParameterLocation.Header: if (_ignoredHeaderParameters.Contains(parameter.Name, StringComparer.OrdinalIgnoreCase)) @@ -397,6 +409,7 @@ private static void ValidateParameters(IHttpRequest request, OperationMatch matc } ValidateParameterValue(parameter.Name, $"{locationName} parameter", values, parameter.Schema, + parameter.Style ?? GetDefaultStyle(parameter.In.Value), parameter.Explode, OpenApiValidationIssueLocation.Request, issues); } @@ -429,15 +442,21 @@ private static void ValidateParameters(IHttpRequest request, OperationMatch matc } } + private static ParameterStyle GetDefaultStyle(ParameterLocation location) => location switch + { + ParameterLocation.Query or ParameterLocation.Cookie => ParameterStyle.Form, + _ => ParameterStyle.Simple + }; + private static void ValidateParameterValue(string name, string kind, List values, IOpenApiSchema? schema, - OpenApiValidationIssueLocation location, List issues) + ParameterStyle style, bool explode, OpenApiValidationIssueLocation location, List issues) { if (schema is null || schema.Type == JsonSchemaType.Object) { return; } - var value = OpenApiSchemaValidator.CoerceParameterValue(values, schema); + var value = OpenApiSchemaValidator.CoerceParameterValue(values, schema, name, style, explode); var direction = location == OpenApiValidationIssueLocation.Request ? SchemaValidationDirection.Request : SchemaValidationDirection.Response; @@ -529,6 +548,7 @@ private static void ValidateResponse(IHttpResponse response, OpenApiOperation op } ValidateParameterValue(headerName, "response header", values, header.Schema, + header.Style ?? ParameterStyle.Simple, header.Explode, OpenApiValidationIssueLocation.Response, issues); } @@ -745,36 +765,108 @@ private async Task> LoadSpecsAsync(IEnumerable files, C continue; } - var serverRegexes = new List(); - foreach (var server in document.Servers ?? []) + var servers = BuildServerScopes(document, file); + if (servers.Count == 0) + { + Logger.LogWarning("OpenAPI spec '{File}' doesn't define any absolute server URLs. Skipping", file); + continue; + } + + specs.Add(new LoadedSpec(file, servers)); + Logger.LogInformation("Loaded OpenAPI spec '{File}'", file); + } + + return specs; + } + + private List BuildServerScopes(OpenApiDocument document, string file) + { + // each operation is served from its effective servers: operation-level + // servers override path-level servers, which override document-level servers + var scopes = new Dictionary Paths)>(StringComparer.Ordinal); + var skippedServers = new HashSet(StringComparer.Ordinal); + + (Regex Regex, Dictionary Paths)? GetScope(string? url) + { + if (string.IsNullOrWhiteSpace(url) || !url.Contains("://", StringComparison.Ordinal)) { - if (string.IsNullOrWhiteSpace(server.Url) || - !server.Url.Contains("://", StringComparison.Ordinal)) + if (skippedServers.Add(url ?? string.Empty)) { - Logger.LogDebug("Skipping server '{ServerUrl}' in '{File}' because it isn't an absolute URL", server.Url, file); - continue; + Logger.LogDebug("Skipping server '{ServerUrl}' in '{File}' because it isn't an absolute URL", url, file); } + return null; + } - var pattern = Regex.Replace(Regex.Escape(server.Url.TrimEnd('/')), @"\\\{[^}]+\}", "[^/]*"); - serverRegexes.Add(new Regex($"^{pattern}", RegexOptions.IgnoreCase | RegexOptions.CultureInvariant)); + var key = url.TrimEnd('/'); + if (!scopes.TryGetValue(key, out var scope)) + { + scope = (CompileServerUrl(key), new(StringComparer.Ordinal)); + scopes[key] = scope; } + return scope; + } + + foreach (var (template, pathItem) in document.Paths ?? []) + { + var pathServers = pathItem.Servers is { Count: > 0 } ? pathItem.Servers : document.Servers ?? []; - if (serverRegexes.Count == 0) + // register the path for its servers even if it has no operations + // there, so that requests with undocumented methods are reported + foreach (var server in pathServers) { - Logger.LogWarning("OpenAPI spec '{File}' doesn't define any absolute server URLs. Skipping", file); - continue; + if (GetScope(server.Url) is { } scope) + { + GetOrAddPath(scope.Paths, template, pathItem); + } } - var paths = (document.Paths ?? []) - .Select(p => CompilePath(p.Key, p.Value)) - .OrderBy(p => p.ParameterNames.Count) - .ToList(); + foreach (var (method, operation) in pathItem.Operations ?? new Dictionary()) + { + var operationServers = operation.Servers is { Count: > 0 } ? operation.Servers : pathServers; + foreach (var server in operationServers) + { + if (GetScope(server.Url) is { } scope) + { + GetOrAddPath(scope.Paths, template, pathItem).Operations[method] = operation; + } + } + } + } - specs.Add(new LoadedSpec(file, document, serverRegexes, paths)); - Logger.LogInformation("Loaded OpenAPI spec '{File}'", file); + // servers without paths still need to be matched so that requests + // to undocumented paths are reported + foreach (var server in document.Servers ?? []) + { + _ = GetScope(server.Url); } - return specs; + return [.. scopes.Values.Select(s => new ServerScope( + s.Regex, + [.. s.Paths.Values.OrderBy(p => p.ParameterNames.Count)]))]; + } + + private static CompiledPath GetOrAddPath(Dictionary paths, string template, IOpenApiPathItem pathItem) + { + if (!paths.TryGetValue(template, out var path)) + { + path = CompilePath(template, pathItem); + paths[template] = path; + } + return path; + } + + private static Regex CompileServerUrl(string url) + { + // scheme and host are case-insensitive, but the base path is case-sensitive + var authorityStart = url.IndexOf("://", StringComparison.Ordinal) + 3; + var pathStart = url.IndexOf('/', authorityStart); + var origin = pathStart < 0 ? url : url[..pathStart]; + var basePath = pathStart < 0 ? string.Empty : url[pathStart..]; + + static string ToPattern(string value) => + Regex.Replace(Regex.Escape(value), @"\\\{[^}]+\}", "[^/]*"); + + return new Regex($"^(?i:{ToPattern(origin)}){ToPattern(basePath)}", RegexOptions.CultureInvariant); } private static CompiledPath CompilePath(string template, IOpenApiPathItem pathItem) @@ -785,13 +877,20 @@ private static CompiledPath CompilePath(string template, IOpenApiPathItem pathIt parameterNames.Add(Regex.Unescape(m.Groups[1].Value)); return "([^/]+)"; }); - var regex = new Regex($"^{pattern}/?$", RegexOptions.IgnoreCase | RegexOptions.CultureInvariant); - return new CompiledPath(template, pathItem, regex, parameterNames); + var regex = new Regex($"^{pattern}$", RegexOptions.CultureInvariant); + return new CompiledPath(template, pathItem, regex, parameterNames, []); } - private sealed record CompiledPath(string Template, IOpenApiPathItem PathItem, Regex Regex, List ParameterNames); + private sealed record CompiledPath( + string Template, + IOpenApiPathItem PathItem, + Regex Regex, + List ParameterNames, + Dictionary Operations); + + private sealed record ServerScope(Regex Regex, List Paths); - private sealed record LoadedSpec(string FilePath, OpenApiDocument Document, List ServerRegexes, List Paths); + private sealed record LoadedSpec(string FilePath, List Servers); private sealed record OperationMatch( LoadedSpec Spec, diff --git a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs index 744ecb0b..4cf1563e 100644 --- a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs +++ b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs @@ -49,29 +49,76 @@ public static List Validate( } /// - /// Converts a raw (string) parameter value into a JSON value, coercing it - /// to the primitive type declared by the schema when possible, so that it + /// Converts raw (string) parameter values into a JSON value, decoding them + /// according to the parameter's serialization style and coercing them + /// to the primitive type declared by the schema when possible, so that they /// can be validated with . /// - public static JsonNode? CoerceParameterValue(IReadOnlyList values, IOpenApiSchema? schema) + public static JsonNode? CoerceParameterValue( + IReadOnlyList values, + IOpenApiSchema? schema, + string name, + ParameterStyle style, + bool explode) { var type = schema?.Type; if (type is not null && type.Value.HasFlag(JsonSchemaType.Array)) { - var items = values.Count == 1 - ? values[0].Split(',') - : [.. values]; var array = new JsonArray(); - foreach (var item in items) + foreach (var item in SplitArrayValues(values, name, style, explode)) { array.Add(CoercePrimitive(item, schema!.Items)); } return array; } - return CoercePrimitive(values.Count > 0 ? values[0] : string.Empty, schema); + var value = values.Count > 0 ? values[0] : string.Empty; + return CoercePrimitive(StripStylePrefix(value, name, style), schema); } + private static IEnumerable SplitArrayValues(IReadOnlyList values, string name, ParameterStyle style, bool explode) + { + switch (style) + { + case ParameterStyle.Form: + case ParameterStyle.SpaceDelimited: + case ParameterStyle.PipeDelimited: + // exploded arrays are sent as repeated parameters, e.g. ids=1&ids=2 + if (explode) + { + return values; + } + var delimiter = style switch + { + ParameterStyle.SpaceDelimited => ' ', + ParameterStyle.PipeDelimited => '|', + _ => ',' + }; + return values.SelectMany(v => v.Split(delimiter)); + case ParameterStyle.Label: + return values.SelectMany(v => StripStylePrefix(v, name, style).Split(explode ? '.' : ',')); + case ParameterStyle.Matrix: + if (explode) + { + // ;id=1;id=2 + var prefix = $";{name}="; + return values.SelectMany(v => v.Split(prefix, StringSplitOptions.RemoveEmptyEntries)); + } + return values.SelectMany(v => StripStylePrefix(v, name, style).Split(',')); + case ParameterStyle.Simple: + default: + return values.SelectMany(v => v.Split(',').Select(i => i.Trim())); + } + } + + private static string StripStylePrefix(string value, string name, ParameterStyle style) => style switch + { + ParameterStyle.Label when value.StartsWith('.') => value[1..], + ParameterStyle.Matrix when value.StartsWith($";{name}=", StringComparison.Ordinal) => value[(name.Length + 2)..], + ParameterStyle.Matrix when value == $";{name}" => string.Empty, + _ => value + }; + private static JsonValue? CoercePrimitive(string value, IOpenApiSchema? schema) { var type = schema?.Type; @@ -86,7 +133,8 @@ public static List Validate( return JsonValue.Create(l); } if (type.Value.HasFlag(JsonSchemaType.Number) && - double.TryParse(value, NumberStyles.Float, CultureInfo.InvariantCulture, out var d)) + double.TryParse(value, NumberStyles.Float, CultureInfo.InvariantCulture, out var d) && + double.IsFinite(d)) { return JsonValue.Create(d); } @@ -110,8 +158,13 @@ private static void ValidateCore( List errors, int depth) { - if (schema is null || depth > MaxDepth) + if (schema is null) + { + return; + } + if (depth > MaxDepth) { + errors.Add(new(path, $"Schema is nested deeper than the supported maximum of {MaxDepth} levels, so the value couldn't be validated")); return; } From 4e6c3548671ae9662375400ca987eea9f3006ee6 Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 11:26:53 +0100 Subject: [PATCH 03/13] Bound schema validation work in OpenApiValidationPlugin Memoize composed (allOf/anyOf/oneOf) subschema results per schema and value, dedupe repeated allOf errors, and cap total evaluations as a backstop so repeated references can't cause exponential validation work. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OpenApiValidationPluginTests.cs | 37 +++++++++++ .../Utils/OpenApiSchemaValidator.cs | 65 +++++++++++++++---- 2 files changed, 90 insertions(+), 12 deletions(-) diff --git a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs index ac683cdf..ac5a977d 100644 --- a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs +++ b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs @@ -563,4 +563,41 @@ public async Task ReportsSchemasExceedingMaxDepth() Assert.Contains(Messages(result), m => m.Contains("supported maximum", StringComparison.Ordinal)); } + + [Fact] + public async Task ValidatesRepeatedSchemaReferencesEfficiently() + { + var schemas = string.Join(",\n", Enumerable.Range(0, 30).Select(i => + $$""" "S{{i}}": { "allOf": [ { "$ref": "#/components/schemas/S{{i + 1}}" }, { "$ref": "#/components/schemas/S{{i + 1}}" } ] }""")); + var plugin = await CreatePluginWithSpecsAsync(("wide.json", $$""" + { + "openapi": "3.0.3", + "info": { "title": "Wide", "version": "1.0" }, + "servers": [ { "url": "https://api.fabrikam.com" } ], + "paths": { + "/wide": { + "get": { + "responses": { + "200": { "description": "ok", "content": { "application/json": { "schema": { "$ref": "#/components/schemas/S0" } } } } + } + } + } + }, + "components": { + "schemas": { + {{schemas}}, + "S30": { "type": "string" } + } + } + } + """)); + + var valid = plugin.Validate(TestExchange.Request("GET", "https://api.fabrikam.com/wide") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: "\"ok\"").Session); + var invalid = plugin.Validate(TestExchange.Request("GET", "https://api.fabrikam.com/wide") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: "1").Session); + + Assert.Empty(valid!.Issues); + Assert.Equal("$: Expected string but got integer", Assert.Single(Messages(invalid))); + } } diff --git a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs index 4cf1563e..1b701045 100644 --- a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs +++ b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs @@ -26,6 +26,15 @@ internal sealed record SchemaValidationError(string Path, string Message); internal static class OpenApiSchemaValidator { private const int MaxDepth = 64; + // bounds total work, e.g. for schemas that reference the same subschema repeatedly + private const int MaxEvaluations = 1_000_000; + + private sealed class ValidationState + { + public int Evaluations { get; set; } + public bool BudgetExceeded { get; set; } + public Dictionary<(IOpenApiSchema Schema, string Path), List> Memo { get; } = []; + } public static List Validate( JsonElement value, @@ -34,7 +43,12 @@ public static List Validate( string path = "$") { var errors = new List(); - ValidateCore(value, schema, direction, path, errors, 0); + var state = new ValidationState(); + ValidateCore(value, schema, direction, path, errors, state, 0); + if (state.BudgetExceeded) + { + errors.Add(new(path, $"Validation stopped after {MaxEvaluations} schema evaluations, so the value couldn't be fully validated")); + } return errors; } @@ -156,10 +170,16 @@ private static void ValidateCore( SchemaValidationDirection direction, string path, List errors, + ValidationState state, int depth) { - if (schema is null) + if (schema is null || state.BudgetExceeded) + { + return; + } + if (++state.Evaluations > MaxEvaluations) { + state.BudgetExceeded = true; return; } if (depth > MaxDepth) @@ -178,14 +198,14 @@ private static void ValidateCore( if (value.ValueKind == JsonValueKind.Object) { - ValidateObject(value, schema, direction, path, errors, depth); + ValidateObject(value, schema, direction, path, errors, state, depth); } else if (value.ValueKind == JsonValueKind.Array && schema.Items is not null) { var i = 0; foreach (var item in value.EnumerateArray()) { - ValidateCore(item, schema.Items, direction, $"{path}[{i}]", errors, depth + 1); + ValidateCore(item, schema.Items, direction, $"{path}[{i}]", errors, state, depth + 1); i++; } } @@ -194,19 +214,26 @@ private static void ValidateCore( { foreach (var subSchema in schema.AllOf) { - ValidateCore(value, subSchema, direction, path, errors, depth + 1); + foreach (var error in ValidateComposed(value, subSchema, direction, path, state, depth + 1)) + { + // subschemas can report the same error, e.g. when they reference the same schema + if (!errors.Contains(error)) + { + errors.Add(error); + } + } } } if (schema.AnyOf is { Count: > 0 } && - !schema.AnyOf.Any(s => IsValid(value, s, direction, path, depth))) + !schema.AnyOf.Any(s => IsValid(value, s, direction, path, state, depth))) { errors.Add(new(path, "Value doesn't match any of the schemas in 'anyOf'")); } if (schema.OneOf is { Count: > 0 }) { - var matches = schema.OneOf.Count(s => IsValid(value, s, direction, path, depth)); + var matches = schema.OneOf.Count(s => IsValid(value, s, direction, path, state, depth)); if (matches == 0) { errors.Add(new(path, "Value doesn't match any of the schemas in 'oneOf'")); @@ -218,11 +245,24 @@ private static void ValidateCore( } } - private static bool IsValid(JsonElement value, IOpenApiSchema schema, SchemaValidationDirection direction, string path, int depth) + private static bool IsValid(JsonElement value, IOpenApiSchema schema, SchemaValidationDirection direction, string path, ValidationState state, int depth) => + ValidateComposed(value, schema, direction, path, state, depth + 1).Count == 0; + + // Composition keywords can reference the same subschema many times for the + // same value. Within a single validation, a path identifies a value, so + // results are memoized per (resolved schema, path) to avoid exponential work. + private static List ValidateComposed(JsonElement value, IOpenApiSchema schema, SchemaValidationDirection direction, string path, ValidationState state, int depth) { + var target = schema is OpenApiSchemaReference reference ? (IOpenApiSchema?)reference.Target ?? schema : schema; + if (state.Memo.TryGetValue((target, path), out var cached)) + { + return cached; + } + var subErrors = new List(); - ValidateCore(value, schema, direction, path, subErrors, depth + 1); - return subErrors.Count == 0; + ValidateCore(value, schema, direction, path, subErrors, state, depth); + state.Memo[(target, path)] = subErrors; + return subErrors; } private static bool ValidateType(JsonElement value, IOpenApiSchema schema, string path, List errors) @@ -286,6 +326,7 @@ private static void ValidateObject( SchemaValidationDirection direction, string path, List errors, + ValidationState state, int depth) { if (schema.Required is { Count: > 0 }) @@ -318,11 +359,11 @@ private static void ValidateObject( if (schema.Properties is not null && schema.Properties.TryGetValue(property.Name, out var propertySchema)) { - ValidateCore(property.Value, propertySchema, direction, propertyPath, errors, depth + 1); + ValidateCore(property.Value, propertySchema, direction, propertyPath, errors, state, depth + 1); } else if (schema.AdditionalProperties is not null) { - ValidateCore(property.Value, schema.AdditionalProperties, direction, propertyPath, errors, depth + 1); + ValidateCore(property.Value, schema.AdditionalProperties, direction, propertyPath, errors, state, depth + 1); } else if (!schema.AdditionalPropertiesAllowed) { From 7bc435029df0d1665ec501c008a65f7459b00415 Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 11:43:23 +0100 Subject: [PATCH 04/13] Address round 3 review feedback for OpenApiValidationPlugin - Use unambiguous property paths so memoized results don't collide - Recognize exploded form-object query parameters - Exempt bracketed query keys only for deepObject parameters - Report malformed JSON even when the media type has no schema - Match server URLs with explicit default ports - Coerce parameter values using types declared via composition Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OpenApiValidationPluginTests.cs | 128 ++++++++++++++++++ .../Reporting/OpenApiValidationPlugin.cs | 55 ++++++-- .../Utils/OpenApiSchemaValidator.cs | 73 +++++++++- 3 files changed, 242 insertions(+), 14 deletions(-) diff --git a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs index ac5a977d..89b88ee0 100644 --- a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs +++ b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs @@ -530,6 +530,134 @@ public async Task DecodesParametersAccordingToStyleAndExplode() Assert.Equal(2, messages.Length); } + [Fact] + public async Task HandlesObjectQueryParameters() + { + var plugin = await CreatePluginWithSpecsAsync(("objects.yaml", """ + openapi: 3.0.3 + info: { title: Objects, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /items: + get: + parameters: + - name: filter + in: query + required: true + schema: { type: object, properties: { status: { type: string }, owner: { type: string } } } + - { name: top, in: query, schema: { type: integer } } + responses: { '204': { description: ok } } + """)); + + var exploded = plugin.Validate(Call("GET", "https://api.fabrikam.com/items?status=active&owner=me&top=5").Session); + var bracketed = plugin.Validate(Call("GET", "https://api.fabrikam.com/items?status=active&top[foo]=bar").Session); + var missing = plugin.Validate(Call("GET", "https://api.fabrikam.com/items?top=5").Session); + + Assert.Empty(exploded!.Issues); + Assert.Equal("top[foo]: Query parameter 'top[foo]' isn't documented", Assert.Single(Messages(bracketed))); + Assert.Equal("filter: Missing required query parameter 'filter'", Assert.Single(Messages(missing))); + } + + [Fact] + public async Task CoercesParametersUsingComposedSchemaTypes() + { + var plugin = await CreatePluginWithSpecsAsync(("composed.yaml", """ + openapi: 3.0.3 + info: { title: Composed, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /items: + get: + parameters: + - { name: count, in: query, schema: { allOf: [ { type: integer } ] } } + - { name: ids, in: query, schema: { allOf: [ { type: array, items: { allOf: [ { type: integer } ] } } ] } } + responses: { '204': { description: ok } } + """)); + + var valid = plugin.Validate(Call("GET", "https://api.fabrikam.com/items?count=42&ids=1&ids=2").Session); + var invalid = plugin.Validate(Call("GET", "https://api.fabrikam.com/items?count=abc").Session); + + Assert.Empty(valid!.Issues); + Assert.Contains("Expected integer", Assert.Single(Messages(invalid)), StringComparison.Ordinal); + } + + [Fact] + public async Task ReportsInvalidJsonForMediaTypesWithoutSchema() + { + var plugin = await CreatePluginWithSpecsAsync(("noschema.yaml", """ + openapi: 3.0.3 + info: { title: No schema, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /items: + get: + responses: + '200': { description: ok, content: { application/json: {} } } + """)); + + var valid = plugin.Validate(TestExchange.Request("GET", "https://api.fabrikam.com/items") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: "{}").Session); + var invalid = plugin.Validate(TestExchange.Request("GET", "https://api.fabrikam.com/items") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: "{").Session); + + Assert.Empty(valid!.Issues); + Assert.Contains("isn't valid JSON", Assert.Single(Messages(invalid)), StringComparison.Ordinal); + } + + [Fact] + public async Task MatchesServersWithExplicitDefaultPorts() + { + var plugin = await CreatePluginWithSpecsAsync(("ports.yaml", """ + openapi: 3.0.3 + info: { title: Ports, version: '1.0' } + servers: + - url: https://api.contoso.com:443/v1 + - url: http://api.contoso.com:80/v1 + paths: + /items: + get: + responses: { '204': { description: ok } } + """)); + + Assert.NotNull(plugin.Validate(Call("GET", "https://api.contoso.com/v1/items").Session)); + Assert.NotNull(plugin.Validate(Call("GET", "http://api.contoso.com/v1/items").Session)); + } + + [Fact] + public async Task DistinguishesDottedPropertyNamesFromNestedProperties() + { + var plugin = await CreatePluginWithSpecsAsync(("dotted.yaml", """ + openapi: 3.0.3 + info: { title: Dotted, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /items: + get: + responses: + '200': + description: ok + content: + application/json: + schema: + type: object + properties: + a: { type: object, properties: { b: { allOf: [ { $ref: '#/components/schemas/Str' } ] } } } + a.b: { allOf: [ { $ref: '#/components/schemas/Str' } ] } + components: + schemas: + Str: { type: string } + """)); + + var result = plugin.Validate(TestExchange.Request("GET", "https://api.fabrikam.com/items") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: """{"a":{"b":"ok"},"a.b":123}""").Session); + + Assert.Equal("$[\"a.b\"]: Expected string but got integer", Assert.Single(Messages(result))); + } + [Fact] public async Task ReportsSchemasExceedingMaxDepth() { diff --git a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs index 445b8ccf..53941f31 100644 --- a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs +++ b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs @@ -370,10 +370,10 @@ private static void ValidateParameters(IHttpRequest request, OperationMatch matc { values = queryValues; } - else if (parameter.Style == ParameterStyle.DeepObject && - query.Keys.Any(k => k.StartsWith($"{parameter.Name}[", StringComparison.Ordinal))) + else if (GetObjectQueryKeys(parameter).Any(k => query.Keys.Any(k.IsMatch))) { - // deepObject values (e.g. filter[name]=value) aren't validated + // values of deepObject (e.g. filter[name]=value) and exploded + // form object (e.g. name=value) parameters aren't validated continue; } break; @@ -420,15 +420,16 @@ private static void ValidateParameters(IHttpRequest request, OperationMatch matc return; } - var knownQueryParameters = parameters + var queryParameters = parameters .Where(p => p.In == ParameterLocation.Query && p.Name is not null) - .Select(p => p.Name!) + .ToArray(); + var knownQueryKeys = queryParameters + .Select(p => new ObjectQueryKey(p.Name!, false)) + .Concat(queryParameters.SelectMany(GetObjectQueryKeys)) .ToArray(); foreach (var name in query.Keys) { - if (knownQueryParameters.Contains(name, StringComparer.Ordinal) || - // deepObject style, e.g. filter[name]=value - knownQueryParameters.Any(p => name.StartsWith($"{p}[", StringComparison.Ordinal))) + if (knownQueryKeys.Any(k => k.IsMatch(name))) { continue; } @@ -448,10 +449,41 @@ private static void ValidateParameters(IHttpRequest request, OperationMatch matc _ => ParameterStyle.Simple }; + private readonly record struct ObjectQueryKey(string Name, bool IsPrefix) + { + public bool IsMatch(string key) => IsPrefix + ? key.StartsWith(Name, StringComparison.Ordinal) + : string.Equals(key, Name, StringComparison.Ordinal); + } + + // Query keys used to serialize object parameters: deepObject parameters + // are sent as filter[name]=value and exploded form objects as name=value + private static IEnumerable GetObjectQueryKeys(IOpenApiParameter parameter) + { + if (parameter.Name is null) + { + return []; + } + + if (parameter.Style == ParameterStyle.DeepObject) + { + return [new($"{parameter.Name}[", true)]; + } + + var type = OpenApiSchemaValidator.GetEffectiveType(parameter.Schema); + if ((parameter.Style ?? ParameterStyle.Form) == ParameterStyle.Form && parameter.Explode && + type is not null && type.Value.HasFlag(JsonSchemaType.Object)) + { + return OpenApiSchemaValidator.GetPropertyNames(parameter.Schema).Select(p => new ObjectQueryKey(p, false)); + } + + return []; + } + private static void ValidateParameterValue(string name, string kind, List values, IOpenApiSchema? schema, ParameterStyle style, bool explode, OpenApiValidationIssueLocation location, List issues) { - if (schema is null || schema.Type == JsonSchemaType.Object) + if (schema is null || OpenApiSchemaValidator.GetEffectiveType(schema) == JsonSchemaType.Object) { return; } @@ -601,7 +633,7 @@ private static void ValidateBody(IHttpMessage message, IDictionary https://api.contoso.com + origin = Regex.Replace(origin, "^(?:(?https://[^/]*):443|(?http://[^/]*):80)$", "${o}", + RegexOptions.IgnoreCase | RegexOptions.CultureInvariant); var basePath = pathStart < 0 ? string.Empty : url[pathStart..]; static string ToPattern(string value) => diff --git a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs index 1b701045..c767f49d 100644 --- a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs +++ b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs @@ -75,13 +75,14 @@ public static List Validate( ParameterStyle style, bool explode) { - var type = schema?.Type; + var type = GetEffectiveType(schema); if (type is not null && type.Value.HasFlag(JsonSchemaType.Array)) { + var itemsSchema = GetItems(schema); var array = new JsonArray(); foreach (var item in SplitArrayValues(values, name, style, explode)) { - array.Add(CoercePrimitive(item, schema!.Items)); + array.Add(CoercePrimitive(item, itemsSchema)); } return array; } @@ -133,9 +134,66 @@ ParameterStyle.Label when value.StartsWith('.') => value[1..], _ => value }; + /// + /// Gets the type declared by the schema, or when the schema doesn't declare + /// a type, the types declared by its allOf/anyOf/oneOf subschemas. + /// + public static JsonSchemaType? GetEffectiveType(IOpenApiSchema? schema) => GetEffectiveType(schema, 0); + + private static JsonSchemaType? GetEffectiveType(IOpenApiSchema? schema, int depth) + { + if (schema is null || depth > MaxDepth) + { + return null; + } + if (schema.Type is not null) + { + return schema.Type; + } + + JsonSchemaType? type = null; + foreach (var subSchema in GetComposedSchemas(schema)) + { + var subType = GetEffectiveType(subSchema, depth + 1); + if (subType is not null) + { + type = (type ?? 0) | subType.Value; + } + } + return type; + } + + private static IOpenApiSchema? GetItems(IOpenApiSchema? schema, int depth = 0) + { + if (schema is null || depth > MaxDepth) + { + return null; + } + return schema.Items ?? GetComposedSchemas(schema) + .Select(s => GetItems(s, depth + 1)) + .FirstOrDefault(i => i is not null); + } + + /// + /// Gets the names of the properties declared by the schema and its allOf/anyOf/oneOf subschemas. + /// + public static IEnumerable GetPropertyNames(IOpenApiSchema? schema, int depth = 0) + { + if (schema is null || depth > MaxDepth) + { + return []; + } + return (schema.Properties?.Keys ?? Enumerable.Empty()) + .Concat(GetComposedSchemas(schema).SelectMany(s => GetPropertyNames(s, depth + 1))) + .Distinct(StringComparer.Ordinal); + } + + private static IEnumerable GetComposedSchemas(IOpenApiSchema schema) => + (schema.AllOf ?? []).Concat(schema.AnyOf ?? []).Concat(schema.OneOf ?? []); + private static JsonValue? CoercePrimitive(string value, IOpenApiSchema? schema) { - var type = schema?.Type; + var type = GetEffectiveType(schema); if (type is null) { return JsonValue.Create(value); @@ -355,7 +413,7 @@ private static void ValidateObject( foreach (var property in value.EnumerateObject()) { - var propertyPath = $"{path}.{property.Name}"; + var propertyPath = GetPropertyPath(path, property.Name); if (schema.Properties is not null && schema.Properties.TryGetValue(property.Name, out var propertySchema)) { @@ -372,6 +430,13 @@ private static void ValidateObject( } } + // Use bracket notation for property names that would make the path + // ambiguous (e.g. 'a.b' vs. 'a' -> 'b'), because paths are also used as memo keys + private static string GetPropertyPath(string path, string propertyName) => + propertyName.Length == 0 || propertyName.IndexOfAny(['.', '[', ']', '"']) >= 0 + ? $"{path}[{JsonSerializer.Serialize(propertyName)}]" + : $"{path}.{propertyName}"; + private static string DescribeType(JsonSchemaType type) { var names = Enum.GetValues() From f3abdaaebef3e11362e271961e604cfe3cb7fa7e Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 11:52:25 +0100 Subject: [PATCH 05/13] Visit each composed schema once when inspecting parameter schemas Type inference, item lookup and property discovery now traverse allOf/anyOf/oneOf with a visited set, so repeated references and cycles can't cause exponential work. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OpenApiValidationPluginTests.cs | 36 +++++++++ .../Utils/OpenApiSchemaValidator.cs | 76 +++++++++++-------- 2 files changed, 79 insertions(+), 33 deletions(-) diff --git a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs index 89b88ee0..bda45726 100644 --- a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs +++ b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs @@ -583,6 +583,42 @@ public async Task CoercesParametersUsingComposedSchemaTypes() Assert.Contains("Expected integer", Assert.Single(Messages(invalid)), StringComparison.Ordinal); } + [Fact] + public async Task InspectsRepeatedParameterSchemaReferencesEfficiently() + { + var schemas = string.Join(",\n", Enumerable.Range(0, 30).Select(i => + $$""" "S{{i}}": { "allOf": [ { "$ref": "#/components/schemas/S{{i + 1}}" }, { "$ref": "#/components/schemas/S{{i + 1}}" } ] }""")); + var plugin = await CreatePluginWithSpecsAsync(("wideparams.json", $$""" + { + "openapi": "3.1.0", + "info": { "title": "Wide", "version": "1.0" }, + "servers": [ { "url": "https://api.fabrikam.com" } ], + "paths": { + "/wide": { + "get": { + "parameters": [ + { "name": "count", "in": "query", "schema": { "$ref": "#/components/schemas/S0" } }, + { "name": "ids", "in": "query", "schema": { "type": "array", "allOf": [ { "$ref": "#/components/schemas/S0" } ] } }, + { "name": "filter", "in": "query", "schema": { "type": "object", "allOf": [ { "$ref": "#/components/schemas/S0" } ] } } + ], + "responses": { "204": { "description": "ok" } } + } + } + }, + "components": { + "schemas": { + {{schemas}}, + "S30": { "properties": { "status": { "type": "string" } } } + } + } + } + """)); + + var result = plugin.Validate(Call("GET", "https://api.fabrikam.com/wide?count=1&ids=1&status=active").Session); + + Assert.Empty(result!.Issues); + } + [Fact] public async Task ReportsInvalidJsonForMediaTypesWithoutSchema() { diff --git a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs index c767f49d..d19a7080 100644 --- a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs +++ b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs @@ -138,56 +138,66 @@ ParameterStyle.Label when value.StartsWith('.') => value[1..], /// Gets the type declared by the schema, or when the schema doesn't declare /// a type, the types declared by its allOf/anyOf/oneOf subschemas. /// - public static JsonSchemaType? GetEffectiveType(IOpenApiSchema? schema) => GetEffectiveType(schema, 0); - - private static JsonSchemaType? GetEffectiveType(IOpenApiSchema? schema, int depth) + public static JsonSchemaType? GetEffectiveType(IOpenApiSchema? schema) { - if (schema is null || depth > MaxDepth) - { - return null; - } - if (schema.Type is not null) - { - return schema.Type; - } - JsonSchemaType? type = null; - foreach (var subSchema in GetComposedSchemas(schema)) + foreach (var s in EnumerateComposition(schema, s => s.Type is null)) { - var subType = GetEffectiveType(subSchema, depth + 1); - if (subType is not null) + if (s.Type is not null) { - type = (type ?? 0) | subType.Value; + type = (type ?? 0) | s.Type.Value; } } return type; } - private static IOpenApiSchema? GetItems(IOpenApiSchema? schema, int depth = 0) - { - if (schema is null || depth > MaxDepth) - { - return null; - } - return schema.Items ?? GetComposedSchemas(schema) - .Select(s => GetItems(s, depth + 1)) - .FirstOrDefault(i => i is not null); - } + private static IOpenApiSchema? GetItems(IOpenApiSchema? schema) => + EnumerateComposition(schema, s => s.Items is null).FirstOrDefault(s => s.Items is not null)?.Items; /// /// Gets the names of the properties declared by the schema and its allOf/anyOf/oneOf subschemas. /// - public static IEnumerable GetPropertyNames(IOpenApiSchema? schema, int depth = 0) + public static IEnumerable GetPropertyNames(IOpenApiSchema? schema) => + EnumerateComposition(schema, _ => true) + .SelectMany(s => s.Properties?.Keys ?? Enumerable.Empty()) + .Distinct(StringComparer.Ordinal); + + // Enumerates the schema and its allOf/anyOf/oneOf subschemas depth-first, + // visiting each (resolved) schema once so that repeated references and + // cycles don't cause exponential work. + private static IEnumerable EnumerateComposition(IOpenApiSchema? schema, Func descend) { - if (schema is null || depth > MaxDepth) + if (schema is null) { - return []; + yield break; + } + + var visited = new HashSet(ReferenceEqualityComparer.Instance); + var stack = new Stack<(IOpenApiSchema Schema, int Depth)>(); + stack.Push((schema, 0)); + while (stack.Count > 0) + { + var (current, depth) = stack.Pop(); + if (!visited.Add(ResolveTarget(current))) + { + continue; + } + + yield return current; + + if (depth < MaxDepth && descend(current)) + { + foreach (var subSchema in GetComposedSchemas(current).Reverse()) + { + stack.Push((subSchema, depth + 1)); + } + } } - return (schema.Properties?.Keys ?? Enumerable.Empty()) - .Concat(GetComposedSchemas(schema).SelectMany(s => GetPropertyNames(s, depth + 1))) - .Distinct(StringComparer.Ordinal); } + private static IOpenApiSchema ResolveTarget(IOpenApiSchema schema) => + schema is OpenApiSchemaReference reference ? (IOpenApiSchema?)reference.Target ?? schema : schema; + private static IEnumerable GetComposedSchemas(IOpenApiSchema schema) => (schema.AllOf ?? []).Concat(schema.AnyOf ?? []).Concat(schema.OneOf ?? []); @@ -311,7 +321,7 @@ private static bool IsValid(JsonElement value, IOpenApiSchema schema, SchemaVali // results are memoized per (resolved schema, path) to avoid exponential work. private static List ValidateComposed(JsonElement value, IOpenApiSchema schema, SchemaValidationDirection direction, string path, ValidationState state, int depth) { - var target = schema is OpenApiSchemaReference reference ? (IOpenApiSchema?)reference.Target ?? schema : schema; + var target = ResolveTarget(schema); if (state.Memo.TryGetValue((target, path), out var cached)) { return cached; From fb8bfb5b29c3329853c46ce5f810e85beba13f9e Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 14:14:59 +0100 Subject: [PATCH 06/13] Hint at per-subschema allOf rules and skip validating large bodies - When an allOf subschema with additionalProperties: false rejects a property declared in a sibling subschema, explain why in the message - Skip validating JSON bodies larger than 5 MB to avoid stalling the proxy, and log that validation was skipped Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OpenApiValidationPluginTests.cs | 58 +++++++++++++++++++ .../Reporting/OpenApiValidationPlugin.cs | 41 +++++++++---- .../Utils/OpenApiSchemaValidator.cs | 28 +++++++-- 3 files changed, 113 insertions(+), 14 deletions(-) diff --git a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs index bda45726..ea759ac3 100644 --- a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs +++ b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs @@ -694,6 +694,64 @@ public async Task DistinguishesDottedPropertyNamesFromNestedProperties() Assert.Equal("$[\"a.b\"]: Expected string but got integer", Assert.Single(Messages(result))); } + [Fact] + public async Task HintsWhenAllOfBranchRejectsPropertyDeclaredInSibling() + { + var plugin = await CreatePluginWithSpecsAsync(("allof.yaml", """ + openapi: 3.0.3 + info: { title: AllOf, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /items: + get: + responses: + '200': + description: ok + content: + application/json: + schema: + allOf: + - { type: object, properties: { id: { type: integer } }, additionalProperties: false } + - { type: object, properties: { name: { type: string } } } + """)); + + var result = plugin.Validate(TestExchange.Request("GET", "https://api.fabrikam.com/items") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: """{"id":1,"name":"Alex","other":true}""").Session); + + var messages = Messages(result); + Assert.Equal(2, messages.Length); + Assert.Contains("$.name: Property 'name' isn't defined in the schema. The property is declared in a sibling allOf schema; additionalProperties: false applies per subschema", messages); + Assert.Contains("$.other: Property 'other' isn't defined in the schema", messages); + } + + [Fact] + public async Task SkipsValidatingLargeBodies() + { + var plugin = await CreatePluginWithSpecsAsync(("large.yaml", """ + openapi: 3.0.3 + info: { title: Large, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /items: + get: + responses: + '200': { description: ok, content: { application/json: { schema: { type: integer } } } } + """)); + + var large = TestExchange.Request("GET", "https://api.fabrikam.com/items") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], + body: $"\"{new string('a', 5 * 1024 * 1024)}\"").Session; + var small = TestExchange.Request("GET", "https://api.fabrikam.com/items") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: "\"a\"").Session; + + Assert.Empty(plugin.Validate(large)!.Issues); + Assert.Contains("exceeds the maximum of 5,242,880 bytes", Assert.Single(plugin.GetNotes(large)), StringComparison.Ordinal); + Assert.Contains("Expected integer", Assert.Single(Messages(plugin.Validate(small))), StringComparison.Ordinal); + Assert.Empty(plugin.GetNotes(small)); + } + [Fact] public async Task ReportsSchemasExceedingMaxDepth() { diff --git a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs index 53941f31..7ec3711e 100644 --- a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs +++ b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs @@ -41,6 +41,8 @@ public sealed class OpenApiValidationPlugin( // headers that OpenAPI says to ignore when defined as header parameters private static readonly string[] _ignoredHeaderParameters = ["Accept", "Content-Type", "Authorization"]; + private const int MaxBodySize = 5 * 1024 * 1024; + private readonly ConditionalWeakTable _outcomes = []; private List _specs = []; @@ -115,6 +117,10 @@ public override Task AfterResponseAsync(ProxyResponseArgs e, CancellationToken c } var outcome = GetOrValidate(e.ProxySession); + foreach (var note in outcome.Notes) + { + Logger.LogRequest(note, MessageType.Normal, loggingContext); + } if (outcome.Result is null) { Logger.LogRequest("No matching OpenAPI spec", MessageType.Skipped, loggingContext); @@ -204,10 +210,16 @@ public override Task AfterRecordingStopAsync(RecordingArgs e, CancellationToken internal OpenApiValidationResult? Validate(IProxySession session) => GetOrValidate(session).Result; + internal IReadOnlyList GetNotes(IProxySession session) => GetOrValidate(session).Notes; + private ValidationOutcome GetOrValidate(IProxySession session) => - _outcomes.GetValue(session, s => new ValidationOutcome(ValidateSession(s))); + _outcomes.GetValue(session, s => + { + var notes = new List(); + return new ValidationOutcome(ValidateSession(s, notes), notes); + }); - private OpenApiValidationResult? ValidateSession(IProxySession session) + private OpenApiValidationResult? ValidateSession(IProxySession session, List notes) { var request = session.Request; var response = session.Response; @@ -256,10 +268,10 @@ private ValidationOutcome GetOrValidate(IProxySession session) => } ValidateParameters(request, match, issues); - ValidateRequestBody(request, match.Operation, issues); + ValidateRequestBody(request, match.Operation, issues, notes); if (response is not null) { - ValidateResponse(response, match.Operation, issues); + ValidateResponse(response, match.Operation, issues, notes); } return new() @@ -503,7 +515,7 @@ private static void ValidateParameterValue(string name, string kind, List issues) + private static void ValidateRequestBody(IHttpRequest request, OpenApiOperation operation, List issues, List notes) { var requestBody = operation.RequestBody; var hasBody = request.Body.Length > 0; @@ -531,10 +543,10 @@ private static void ValidateRequestBody(IHttpRequest request, OpenApiOperation o return; } - ValidateBody(request, requestBody.Content, OpenApiValidationIssueLocation.Request, issues); + ValidateBody(request, requestBody.Content, OpenApiValidationIssueLocation.Request, issues, notes); } - private static void ValidateResponse(IHttpResponse response, OpenApiOperation operation, List issues) + private static void ValidateResponse(IHttpResponse response, OpenApiOperation operation, List issues, List notes) { var statusCode = ((int)response.StatusCode).ToString(CultureInfo.InvariantCulture); var responses = operation.Responses; @@ -599,11 +611,11 @@ private static void ValidateResponse(IHttpResponse response, OpenApiOperation op return; } - ValidateBody(response, specResponse.Content, OpenApiValidationIssueLocation.Response, issues); + ValidateBody(response, specResponse.Content, OpenApiValidationIssueLocation.Response, issues, notes); } private static void ValidateBody(IHttpMessage message, IDictionary? content, - OpenApiValidationIssueLocation location, List issues) + OpenApiValidationIssueLocation location, List issues, List notes) { if (content is not { Count: > 0 }) { @@ -638,6 +650,15 @@ private static void ValidateBody(IHttpMessage message, IDictionary MaxBodySize) + { + notes.Add(string.Create(CultureInfo.InvariantCulture, + $"Skipped validating the {locationName} body because it's {message.Body.Length:N0} bytes, which exceeds the maximum of {MaxBodySize:N0} bytes")); + return; + } + JsonDocument json; try { @@ -935,5 +956,5 @@ private sealed record OperationMatch( OpenApiOperation? Operation, Dictionary PathValues); - private sealed record ValidationOutcome(OpenApiValidationResult? Result); + private sealed record ValidationOutcome(OpenApiValidationResult? Result, IReadOnlyList Notes); } diff --git a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs index d19a7080..b775faf7 100644 --- a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs +++ b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs @@ -15,7 +15,9 @@ internal enum SchemaValidationDirection Response } -internal sealed record SchemaValidationError(string Path, string Message); +/// Name of the property when the error is +/// about a property that isn't allowed by the schema. +internal sealed record SchemaValidationError(string Path, string Message, string? UndefinedProperty = null); /// /// Lightweight validator of JSON values against OpenAPI schemas @@ -198,6 +200,10 @@ private static IEnumerable EnumerateComposition(IOpenApiSchema? private static IOpenApiSchema ResolveTarget(IOpenApiSchema schema) => schema is OpenApiSchemaReference reference ? (IOpenApiSchema?)reference.Target ?? schema : schema; + private static bool IsDeclaredInSibling(IList allOf, int index, string propertyName) => + allOf.Where((_, i) => i != index) + .Any(s => GetPropertyNames(s).Contains(propertyName, StringComparer.Ordinal)); + private static IEnumerable GetComposedSchemas(IOpenApiSchema schema) => (schema.AllOf ?? []).Concat(schema.AnyOf ?? []).Concat(schema.OneOf ?? []); @@ -280,10 +286,24 @@ private static void ValidateCore( if (schema.AllOf is { Count: > 0 }) { - foreach (var subSchema in schema.AllOf) + for (var i = 0; i < schema.AllOf.Count; i++) { - foreach (var error in ValidateComposed(value, subSchema, direction, path, state, depth + 1)) + foreach (var subError in ValidateComposed(value, schema.AllOf[i], direction, path, state, depth + 1)) { + var error = subError; + if (error.UndefinedProperty is not null && + error.Path == GetPropertyPath(path, error.UndefinedProperty) && + IsDeclaredInSibling(schema.AllOf, i, error.UndefinedProperty)) + { + // additionalProperties: false applies to each subschema on its own, + // which is spec-compliant but often unexpected + error = error with + { + Message = $"{error.Message}. The property is declared in a sibling allOf schema; additionalProperties: false applies per subschema", + UndefinedProperty = null + }; + } + // subschemas can report the same error, e.g. when they reference the same schema if (!errors.Contains(error)) { @@ -435,7 +455,7 @@ private static void ValidateObject( } else if (!schema.AdditionalPropertiesAllowed) { - errors.Add(new(propertyPath, $"Property '{property.Name}' isn't defined in the schema")); + errors.Add(new(propertyPath, $"Property '{property.Name}' isn't defined in the schema", property.Name)); } } } From eb45b94ec53c7c5990182b6295c9db4146096cbd Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 14:24:56 +0100 Subject: [PATCH 07/13] Include skipped validations in the OpenAPI validation report Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OpenApiValidationPluginTests.cs | 23 +++++++++++++ .../Reporting/OpenApiValidationPlugin.cs | 5 ++- .../OpenApiValidationPluginReport.cs | 33 ++++++++++++++++++- 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs index ea759ac3..9fa26b45 100644 --- a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs +++ b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs @@ -361,6 +361,29 @@ public async Task StoresReportOnRecordingStop(bool reportUnmatchedRequests) Assert.Contains("Missing required property 'name'", report.ToPlainText(), StringComparison.Ordinal); } + [Fact] + public async Task ReportsSkippedValidations() + { + var plugin = await CreatePluginAsync(); + var args = Recording([ + Get("https://api.contoso.com/v1/users/1", HttpStatusCode.OK, """{ "id": 1, "name": "Alex" }""").AsRequestLog(MessageType.InterceptedResponse), + Get("https://api.contoso.com/v1/users/2", HttpStatusCode.OK, $$"""{ "id": 2, "name": "{{new string('a', 5 * 1024 * 1024)}}" }""").AsRequestLog(MessageType.InterceptedResponse), + ]); + + await plugin.AfterRecordingStopAsync(args, CancellationToken.None); + + var report = Assert.IsType( + ((Dictionary)args.GlobalData[ProxyUtils.ReportsKey])[plugin.Name]); + Assert.Equal(2, report.ValidatedRequests); + Assert.Empty(report.Results); + var skipped = Assert.Single(report.SkippedValidations!); + Assert.StartsWith("GET https://api.contoso.com/v1/users/2: Skipped validating the response body", skipped, StringComparison.Ordinal); + var markdown = report.ToMarkdown()!; + Assert.DoesNotContain("All validated requests", markdown, StringComparison.Ordinal); + Assert.Contains("## Skipped validations", markdown, StringComparison.Ordinal); + Assert.Contains(skipped, report.ToPlainText(), StringComparison.Ordinal); + } + [Fact] public async Task LoadsYamlOpenApi31SpecsFromFolder() { diff --git a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs index 7ec3711e..275bbbcd 100644 --- a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs +++ b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs @@ -169,6 +169,7 @@ public override Task AfterRecordingStopAsync(RecordingArgs e, CancellationToken var validated = 0; var results = new List(); var unmatched = new List(); + var skipped = new List(); foreach (var session in sessions) { @@ -182,6 +183,7 @@ public override Task AfterRecordingStopAsync(RecordingArgs e, CancellationToken } validated++; + skipped.AddRange(outcome.Notes.Select(n => $"{session.Request.Method} {session.Request.Url}: {n}")); if (outcome.Result.Issues.Any()) { results.Add(outcome.Result); @@ -200,7 +202,8 @@ public override Task AfterRecordingStopAsync(RecordingArgs e, CancellationToken { ValidatedRequests = validated, Results = results, - UnmatchedRequests = Configuration.ReportUnmatchedRequests ? [.. unmatched.Distinct()] : null + UnmatchedRequests = Configuration.ReportUnmatchedRequests ? [.. unmatched.Distinct()] : null, + SkippedValidations = skipped.Count > 0 ? skipped : null }; StoreReport(report, e); diff --git a/DevProxy.Plugins/Reporting/OpenApiValidationPluginReport.cs b/DevProxy.Plugins/Reporting/OpenApiValidationPluginReport.cs index fda2af37..047c52b4 100644 --- a/DevProxy.Plugins/Reporting/OpenApiValidationPluginReport.cs +++ b/DevProxy.Plugins/Reporting/OpenApiValidationPluginReport.cs @@ -47,6 +47,11 @@ public sealed class OpenApiValidationPluginReport : IMarkdownReport, IPlainTextR public required int ValidatedRequests { get; init; } public required IEnumerable Results { get; init; } public IEnumerable? UnmatchedRequests { get; init; } + /// + /// Parts of validated requests that weren't validated, e.g. bodies + /// exceeding the maximum supported size, with the reason. + /// + public IEnumerable? SkippedValidations { get; init; } public string? ToMarkdown() { @@ -55,8 +60,12 @@ public sealed class OpenApiValidationPluginReport : IMarkdownReport, IPlainTextR .AppendLine() .AppendLine(CultureInfo.InvariantCulture, $"- Validated requests: {ValidatedRequests}") .AppendLine(CultureInfo.InvariantCulture, $"- Requests with issues: {Results.Count()}"); + if (SkippedValidations?.Any() == true) + { + _ = sb.AppendLine(CultureInfo.InvariantCulture, $"- Skipped validations: {SkippedValidations.Count()}"); + } - if (!Results.Any()) + if (!Results.Any() && SkippedValidations?.Any() != true) { _ = sb.AppendLine() .AppendLine("✅ All validated requests and responses match their OpenAPI specs."); @@ -92,6 +101,15 @@ public sealed class OpenApiValidationPluginReport : IMarkdownReport, IPlainTextR } } + if (SkippedValidations?.Any() == true) + { + _ = sb.AppendLine() + .AppendLine("## Skipped validations") + .AppendLine() + .AppendJoin(Environment.NewLine, SkippedValidations.Select(r => $"- {r}")) + .AppendLine(); + } + if (UnmatchedRequests?.Any() == true) { _ = sb.AppendLine() @@ -111,6 +129,10 @@ public sealed class OpenApiValidationPluginReport : IMarkdownReport, IPlainTextR .AppendLine() .AppendLine(CultureInfo.InvariantCulture, $"Validated requests: {ValidatedRequests}") .AppendLine(CultureInfo.InvariantCulture, $"Requests with issues: {Results.Count()}"); + if (SkippedValidations?.Any() == true) + { + _ = sb.AppendLine(CultureInfo.InvariantCulture, $"Skipped validations: {SkippedValidations.Count()}"); + } foreach (var result in Results) { @@ -137,6 +159,15 @@ public sealed class OpenApiValidationPluginReport : IMarkdownReport, IPlainTextR } } + if (SkippedValidations?.Any() == true) + { + _ = sb.AppendLine() + .AppendLine("Skipped validations:") + .AppendLine() + .AppendJoin(Environment.NewLine, SkippedValidations.Select(r => $"- {r}")) + .AppendLine(); + } + if (UnmatchedRequests?.Any() == true) { _ = sb.AppendLine() From d2333a7d17780881d091eb216b1bf3b83ea2781c Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 14:38:07 +0100 Subject: [PATCH 08/13] Fix composed-schema edge cases in OpenApiValidationPlugin - Dedupe allOf errors with a hash set instead of a linear scan - Include depth in the composed-schema memo key so depth-limited results aren't reused at shallower depths - Accept parameter values that are valid as strings when the coerced value isn't, e.g. for schemas allowing both integers and strings Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OpenApiValidationPluginTests.cs | 63 +++++++++++++++++++ .../Reporting/OpenApiValidationPlugin.cs | 14 ++++- .../Utils/OpenApiSchemaValidator.cs | 21 ++++--- 3 files changed, 89 insertions(+), 9 deletions(-) diff --git a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs index 9fa26b45..a2461a83 100644 --- a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs +++ b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs @@ -642,6 +642,30 @@ public async Task InspectsRepeatedParameterSchemaReferencesEfficiently() Assert.Empty(result!.Issues); } + [Fact] + public async Task AcceptsParameterValuesValidAsStrings() + { + var plugin = await CreatePluginWithSpecsAsync(("strings.yaml", """ + openapi: 3.0.3 + info: { title: Strings, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /items: + get: + parameters: + - { name: x, in: query, schema: { anyOf: [ { type: integer, enum: [1, 2] }, { type: string } ] } } + - { name: n, in: query, schema: { type: integer, enum: [1, 2] } } + responses: { '204': { description: ok } } + """)); + + var valid = plugin.Validate(Call("GET", "https://api.fabrikam.com/items?x=42&n=1").Session); + var invalid = plugin.Validate(Call("GET", "https://api.fabrikam.com/items?n=42").Session); + + Assert.Empty(valid!.Issues); + Assert.Contains("isn't one of the allowed values", Assert.Single(Messages(invalid)), StringComparison.Ordinal); + } + [Fact] public async Task ReportsInvalidJsonForMediaTypesWithoutSchema() { @@ -775,6 +799,45 @@ public async Task SkipsValidatingLargeBodies() Assert.Empty(plugin.GetNotes(small)); } + [Fact] + public async Task DoesNotReuseDepthLimitedResultsAtShallowerDepths() + { + // the chain reaches Str exactly beyond the depth limit, while the + // second anyOf branch reaches it at a shallow depth + var schemas = string.Join(",\n", Enumerable.Range(0, 63).Select(i => + $$""" "S{{i}}": { "allOf": [ { "$ref": "#/components/schemas/S{{i + 1}}" } ] }""")); + var plugin = await CreatePluginWithSpecsAsync(("depthcache.json", $$""" + { + "openapi": "3.0.3", + "info": { "title": "Depth cache", "version": "1.0" }, + "servers": [ { "url": "https://api.fabrikam.com" } ], + "paths": { + "/items": { + "get": { + "responses": { + "200": { "description": "ok", "content": { "application/json": { "schema": { + "anyOf": [ { "$ref": "#/components/schemas/S0" }, { "$ref": "#/components/schemas/Str" } ] + } } } } + } + } + } + }, + "components": { + "schemas": { + {{schemas}}, + "S63": { "allOf": [ { "$ref": "#/components/schemas/Str" } ] }, + "Str": { "type": "string" } + } + } + } + """)); + + var result = plugin.Validate(TestExchange.Request("GET", "https://api.fabrikam.com/items") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: "\"ok\"").Session); + + Assert.Empty(result!.Issues); + } + [Fact] public async Task ReportsSchemasExceedingMaxDepth() { diff --git a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs index 275bbbcd..14dc5fc8 100644 --- a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs +++ b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs @@ -507,7 +507,19 @@ private static void ValidateParameterValue(string name, string kind, List 0) + { + // the value might be valid as a string when the schema allows + // multiple types, e.g. anyOf: [{ type: integer, enum: [1] }, { type: string }] + var raw = OpenApiSchemaValidator.CoerceParameterValue(values, schema, name, style, explode, coerce: false); + if (OpenApiSchemaValidator.Validate(raw, schema, direction, name).Count == 0) + { + return; + } + } + + foreach (var error in errors) { issues.Add(new() { diff --git a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs index b775faf7..6c0ad48d 100644 --- a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs +++ b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs @@ -35,7 +35,7 @@ private sealed class ValidationState { public int Evaluations { get; set; } public bool BudgetExceeded { get; set; } - public Dictionary<(IOpenApiSchema Schema, string Path), List> Memo { get; } = []; + public Dictionary<(IOpenApiSchema Schema, string Path, int Depth), List> Memo { get; } = []; } public static List Validate( @@ -69,13 +69,15 @@ public static List Validate( /// according to the parameter's serialization style and coercing them /// to the primitive type declared by the schema when possible, so that they /// can be validated with . + /// When is false, values are kept as strings. /// public static JsonNode? CoerceParameterValue( IReadOnlyList values, IOpenApiSchema? schema, string name, ParameterStyle style, - bool explode) + bool explode, + bool coerce = true) { var type = GetEffectiveType(schema); if (type is not null && type.Value.HasFlag(JsonSchemaType.Array)) @@ -84,13 +86,14 @@ public static List Validate( var array = new JsonArray(); foreach (var item in SplitArrayValues(values, name, style, explode)) { - array.Add(CoercePrimitive(item, itemsSchema)); + array.Add(coerce ? CoercePrimitive(item, itemsSchema) : JsonValue.Create(item)); } return array; } var value = values.Count > 0 ? values[0] : string.Empty; - return CoercePrimitive(StripStylePrefix(value, name, style), schema); + var stripped = StripStylePrefix(value, name, style); + return coerce ? CoercePrimitive(stripped, schema) : JsonValue.Create(stripped); } private static IEnumerable SplitArrayValues(IReadOnlyList values, string name, ParameterStyle style, bool explode) @@ -286,6 +289,7 @@ private static void ValidateCore( if (schema.AllOf is { Count: > 0 }) { + var seen = new HashSet(errors); for (var i = 0; i < schema.AllOf.Count; i++) { foreach (var subError in ValidateComposed(value, schema.AllOf[i], direction, path, state, depth + 1)) @@ -305,7 +309,7 @@ private static void ValidateCore( } // subschemas can report the same error, e.g. when they reference the same schema - if (!errors.Contains(error)) + if (seen.Add(error)) { errors.Add(error); } @@ -338,18 +342,19 @@ private static bool IsValid(JsonElement value, IOpenApiSchema schema, SchemaVali // Composition keywords can reference the same subschema many times for the // same value. Within a single validation, a path identifies a value, so - // results are memoized per (resolved schema, path) to avoid exponential work. + // results are memoized per (resolved schema, path, depth) to avoid exponential + // work. Depth is part of the key, because results near the depth limit differ. private static List ValidateComposed(JsonElement value, IOpenApiSchema schema, SchemaValidationDirection direction, string path, ValidationState state, int depth) { var target = ResolveTarget(schema); - if (state.Memo.TryGetValue((target, path), out var cached)) + if (state.Memo.TryGetValue((target, path, depth), out var cached)) { return cached; } var subErrors = new List(); ValidateCore(value, schema, direction, path, subErrors, state, depth); - state.Memo[(target, path)] = subErrors; + state.Memo[(target, path, depth)] = subErrors; return subErrors; } From 5bc98b14c4c68efa12ec610fdd2100f49d1ee737 Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 16:41:55 +0100 Subject: [PATCH 09/13] Address review findings for OpenApiValidationPlugin - Most specific matching path takes precedence; a missing method is reported as undocumented - OpenAPI 3.1+ specs require null to be listed in enums - Honor readOnly/writeOnly declared in allOf subschemas and references Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OpenApiValidationPluginTests.cs | 83 +++++++++++++++++++ .../Reporting/OpenApiValidationPlugin.cs | 49 ++++++----- .../Utils/OpenApiSchemaValidator.cs | 59 ++++++++++--- 3 files changed, 156 insertions(+), 35 deletions(-) diff --git a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs index a2461a83..59ee68cb 100644 --- a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs +++ b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs @@ -838,6 +838,89 @@ public async Task DoesNotReuseDepthLimitedResultsAtShallowerDepths() Assert.Empty(result!.Issues); } + [Fact] + public async Task ReportsMethodNotDocumentedOnMoreSpecificLiteralPath() + { + var plugin = await CreatePluginWithSpecsAsync(("literal.yaml", """ + openapi: 3.0.3 + info: { title: Literal, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /users/me: + get: + responses: { '204': { description: ok } } + /users/{id}: + post: + parameters: + - { name: id, in: path, required: true, schema: { type: string } } + responses: { '204': { description: ok } } + """)); + + var result = plugin.Validate(Call("POST", "https://api.fabrikam.com/users/me").Session); + + Assert.Equal("Method POST isn't documented for path '/users/me'", Assert.Single(Messages(result))); + } + + [Fact] + public async Task RejectsNullNotInEnumForOpenApi31TypeArrays() + { + var plugin = await CreatePluginWithSpecsAsync(("nullenum.yaml", """ + openapi: 3.1.0 + info: { title: Null enum, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /items: + get: + responses: + '200': + description: ok + content: + application/json: + schema: { type: [string, 'null'], enum: [active, inactive] } + """)); + + var result = plugin.Validate(TestExchange.Request("GET", "https://api.fabrikam.com/items") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: "null").Session); + + Assert.Contains("isn't one of the allowed values", Assert.Single(Messages(result)), StringComparison.Ordinal); + } + + [Fact] + public async Task ExemptsReadOnlyPropertiesDeclaredInComposedSchemas() + { + var plugin = await CreatePluginWithSpecsAsync(("readonly.yaml", """ + openapi: 3.0.3 + info: { title: Read-only, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /items: + post: + requestBody: + content: + application/json: + schema: + required: [id, name] + allOf: + - $ref: '#/components/schemas/Base' + - { type: object, properties: { name: { type: string } } } + responses: { '204': { description: ok } } + components: + schemas: + Base: { type: object, properties: { id: { type: integer, readOnly: true } } } + """)); + + var valid = plugin.Validate(TestExchange.Request("POST", "https://api.fabrikam.com/items", + headers: [("Content-Type", "application/json")], body: """{"name":"Alex"}""").WithResponse(HttpStatusCode.NoContent).Session); + var invalid = plugin.Validate(TestExchange.Request("POST", "https://api.fabrikam.com/items", + headers: [("Content-Type", "application/json")], body: """{"id":1}""").WithResponse(HttpStatusCode.NoContent).Session); + + Assert.Empty(valid!.Issues); + Assert.Equal("$: Missing required property 'name'", Assert.Single(Messages(invalid))); + } + [Fact] public async Task ReportsSchemasExceedingMaxDepth() { diff --git a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs index 14dc5fc8..3e8b08ec 100644 --- a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs +++ b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs @@ -270,11 +270,12 @@ private ValidationOutcome GetOrValidate(IProxySession session) => }; } + var strict = match.Spec.StrictNullableEnums; ValidateParameters(request, match, issues); - ValidateRequestBody(request, match.Operation, issues, notes); + ValidateRequestBody(request, match.Operation, issues, notes, strict); if (response is not null) { - ValidateResponse(response, match.Operation, issues, notes); + ValidateResponse(response, match.Operation, issues, notes, strict); } return new() @@ -317,18 +318,18 @@ private ValidationOutcome GetOrValidate(IProxySession session) => relativePath = "/"; } - var matchedAnyPath = false; - foreach (var (path, pathValues) in FindPaths(server, relativePath)) + // paths are ordered from the most specific, which takes precedence + // per the OpenAPI spec, even if it doesn't document the method + var bestPath = FindPaths(server, relativePath).FirstOrDefault(); + if (bestPath.Path is { } path) { - matchedAnyPath = true; if (path.Operations.TryGetValue(method, out var operation)) { - return new OperationMatch(spec, relativePath, path.Template, path.PathItem, operation, pathValues); + return new OperationMatch(spec, relativePath, path.Template, path.PathItem, operation, bestPath.PathValues); } - firstPathMatch ??= new OperationMatch(spec, relativePath, path.Template, path.PathItem, null, pathValues); + firstPathMatch ??= new OperationMatch(spec, relativePath, path.Template, path.PathItem, null, bestPath.PathValues); } - - if (!matchedAnyPath) + else { firstServerMatch ??= new OperationMatch(spec, relativePath, null, null, null, []); } @@ -425,7 +426,7 @@ private static void ValidateParameters(IHttpRequest request, OperationMatch matc ValidateParameterValue(parameter.Name, $"{locationName} parameter", values, parameter.Schema, parameter.Style ?? GetDefaultStyle(parameter.In.Value), parameter.Explode, - OpenApiValidationIssueLocation.Request, issues); + OpenApiValidationIssueLocation.Request, issues, match.Spec.StrictNullableEnums); } // unknown query parameters @@ -496,7 +497,8 @@ private static IEnumerable GetObjectQueryKeys(IOpenApiParameter } private static void ValidateParameterValue(string name, string kind, List values, IOpenApiSchema? schema, - ParameterStyle style, bool explode, OpenApiValidationIssueLocation location, List issues) + ParameterStyle style, bool explode, OpenApiValidationIssueLocation location, List issues, + bool strictNullableEnums) { if (schema is null || OpenApiSchemaValidator.GetEffectiveType(schema) == JsonSchemaType.Object) { @@ -507,13 +509,13 @@ private static void ValidateParameterValue(string name, string kind, List 0) { // the value might be valid as a string when the schema allows // multiple types, e.g. anyOf: [{ type: integer, enum: [1] }, { type: string }] var raw = OpenApiSchemaValidator.CoerceParameterValue(values, schema, name, style, explode, coerce: false); - if (OpenApiSchemaValidator.Validate(raw, schema, direction, name).Count == 0) + if (OpenApiSchemaValidator.Validate(raw, schema, direction, name, strictNullableEnums).Count == 0) { return; } @@ -530,7 +532,7 @@ private static void ValidateParameterValue(string name, string kind, List issues, List notes) + private static void ValidateRequestBody(IHttpRequest request, OpenApiOperation operation, List issues, List notes, bool strictNullableEnums) { var requestBody = operation.RequestBody; var hasBody = request.Body.Length > 0; @@ -558,10 +560,10 @@ private static void ValidateRequestBody(IHttpRequest request, OpenApiOperation o return; } - ValidateBody(request, requestBody.Content, OpenApiValidationIssueLocation.Request, issues, notes); + ValidateBody(request, requestBody.Content, OpenApiValidationIssueLocation.Request, issues, notes, strictNullableEnums); } - private static void ValidateResponse(IHttpResponse response, OpenApiOperation operation, List issues, List notes) + private static void ValidateResponse(IHttpResponse response, OpenApiOperation operation, List issues, List notes, bool strictNullableEnums) { var statusCode = ((int)response.StatusCode).ToString(CultureInfo.InvariantCulture); var responses = operation.Responses; @@ -608,7 +610,7 @@ private static void ValidateResponse(IHttpResponse response, OpenApiOperation op ValidateParameterValue(headerName, "response header", values, header.Schema, header.Style ?? ParameterStyle.Simple, header.Explode, - OpenApiValidationIssueLocation.Response, issues); + OpenApiValidationIssueLocation.Response, issues, strictNullableEnums); } if (response.Body.Length == 0) @@ -626,11 +628,11 @@ private static void ValidateResponse(IHttpResponse response, OpenApiOperation op return; } - ValidateBody(response, specResponse.Content, OpenApiValidationIssueLocation.Response, issues, notes); + ValidateBody(response, specResponse.Content, OpenApiValidationIssueLocation.Response, issues, notes, strictNullableEnums); } private static void ValidateBody(IHttpMessage message, IDictionary? content, - OpenApiValidationIssueLocation location, List issues, List notes) + OpenApiValidationIssueLocation location, List issues, List notes, bool strictNullableEnums) { if (content is not { Count: > 0 }) { @@ -694,7 +696,7 @@ private static void ValidateBody(IHttpMessage message, IDictionary> LoadSpecsAsync(IEnumerable files, C Logger.LogDebug("Loading OpenAPI spec from '{File}'...", file); OpenApiDocument? document; string? diagnostics; + var strictNullableEnums = false; try { var fileContents = await File.ReadAllTextAsync(file, cancellationToken); @@ -817,6 +820,7 @@ private async Task> LoadSpecsAsync(IEnumerable files, C settings.AddYamlReader(); var readResult = OpenApiDocument.Parse(fileContents, null, settings); document = readResult.Document; + strictNullableEnums = readResult.Diagnostic?.SpecificationVersion >= OpenApiSpecVersion.OpenApi3_1; diagnostics = readResult.Diagnostic?.Errors is { Count: > 0 } errors ? string.Join("; ", errors.Select(e => e.ToString())) : null; @@ -840,7 +844,7 @@ private async Task> LoadSpecsAsync(IEnumerable files, C continue; } - specs.Add(new LoadedSpec(file, servers)); + specs.Add(new LoadedSpec(file, servers, strictNullableEnums)); Logger.LogInformation("Loaded OpenAPI spec '{File}'", file); } @@ -961,7 +965,8 @@ private sealed record CompiledPath( private sealed record ServerScope(Regex Regex, List Paths); - private sealed record LoadedSpec(string FilePath, List Servers); + // OAS 3.1 requires null to be listed in enums, unlike OAS 3.0 nullable enums + private sealed record LoadedSpec(string FilePath, List Servers, bool StrictNullableEnums); private sealed record OperationMatch( LoadedSpec Spec, diff --git a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs index 6c0ad48d..0ba029c4 100644 --- a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs +++ b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs @@ -35,6 +35,7 @@ private sealed class ValidationState { public int Evaluations { get; set; } public bool BudgetExceeded { get; set; } + public bool StrictNullableEnums { get; init; } public Dictionary<(IOpenApiSchema Schema, string Path, int Depth), List> Memo { get; } = []; } @@ -42,10 +43,11 @@ public static List Validate( JsonElement value, IOpenApiSchema? schema, SchemaValidationDirection direction, - string path = "$") + string path = "$", + bool strictNullableEnums = false) { var errors = new List(); - var state = new ValidationState(); + var state = new ValidationState { StrictNullableEnums = strictNullableEnums }; ValidateCore(value, schema, direction, path, errors, state, 0); if (state.BudgetExceeded) { @@ -58,10 +60,11 @@ public static List Validate( JsonNode? value, IOpenApiSchema? schema, SchemaValidationDirection direction, - string path = "$") + string path = "$", + bool strictNullableEnums = false) { using var doc = JsonDocument.Parse(value?.ToJsonString() ?? "null"); - return Validate(doc.RootElement.Clone(), schema, direction, path); + return Validate(doc.RootElement.Clone(), schema, direction, path, strictNullableEnums); } /// @@ -203,6 +206,36 @@ private static IEnumerable EnumerateComposition(IOpenApiSchema? private static IOpenApiSchema ResolveTarget(IOpenApiSchema schema) => schema is OpenApiSchemaReference reference ? (IOpenApiSchema?)reference.Target ?? schema : schema; + // Finds the declarations of the property in the schema and its allOf + // subschemas (incl. references), which all apply to the value + private static IEnumerable FindAllOfPropertySchemas(IOpenApiSchema schema, string propertyName) + { + var visited = new HashSet(ReferenceEqualityComparer.Instance); + var stack = new Stack<(IOpenApiSchema Schema, int Depth)>(); + stack.Push((schema, 0)); + while (stack.Count > 0) + { + var (current, depth) = stack.Pop(); + if (!visited.Add(ResolveTarget(current))) + { + continue; + } + + if (current.Properties?.TryGetValue(propertyName, out var propertySchema) == true) + { + yield return propertySchema; + } + + if (depth < MaxDepth) + { + foreach (var subSchema in current.AllOf ?? []) + { + stack.Push((subSchema, depth + 1)); + } + } + } + } + private static bool IsDeclaredInSibling(IList allOf, int index, string propertyName) => allOf.Where((_, i) => i != index) .Any(s => GetPropertyNames(s).Contains(propertyName, StringComparer.Ordinal)); @@ -271,7 +304,7 @@ private static void ValidateCore( return; } - ValidateEnum(value, schema, path, errors); + ValidateEnum(value, schema, path, errors, state.StrictNullableEnums); if (value.ValueKind == JsonValueKind.Object) { @@ -390,7 +423,7 @@ private static bool IsInteger(JsonElement value) => value.TryGetInt64(out _) || (value.TryGetDecimal(out var d) && decimal.Truncate(d) == d); - private static void ValidateEnum(JsonElement value, IOpenApiSchema schema, string path, List errors) + private static void ValidateEnum(JsonElement value, IOpenApiSchema schema, string path, List errors, bool strictNullableEnums) { if (schema.Enum is not { Count: > 0 }) { @@ -403,8 +436,10 @@ private static void ValidateEnum(JsonElement value, IOpenApiSchema schema, strin return; } - // OAS 3.0 nullable enums: null is allowed if the schema allows null - if (node is null && schema.Type is { } type && type.HasFlag(JsonSchemaType.Null)) + // OAS 3.0 nullable enums: null is allowed if the schema allows null. + // Microsoft.OpenApi maps 'nullable' to the null type, so this can't be + // distinguished from OAS 3.1, where null must be listed in the enum + if (!strictNullableEnums && node is null && schema.Type is { } type && type.HasFlag(JsonSchemaType.Null)) { return; } @@ -433,11 +468,9 @@ private static void ValidateObject( // readOnly properties aren't sent in requests, writeOnly // properties aren't returned in responses - IOpenApiSchema? propertySchema = null; - _ = schema.Properties?.TryGetValue(requiredProperty, out propertySchema); - if (propertySchema is not null && - ((direction == SchemaValidationDirection.Request && propertySchema.ReadOnly) || - (direction == SchemaValidationDirection.Response && propertySchema.WriteOnly))) + if (FindAllOfPropertySchemas(schema, requiredProperty).Any(propertySchema => + (direction == SchemaValidationDirection.Request && propertySchema.ReadOnly) || + (direction == SchemaValidationDirection.Response && propertySchema.WriteOnly))) { continue; } From c43ae1c9929d8c454fdd93ae45b0d2cb87e9dd41 Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 16:50:35 +0100 Subject: [PATCH 10/13] Avoid quadratic allOf error deduplication for large arrays Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OpenApiValidationPluginTests.cs | 34 +++++++++++++++++++ .../Utils/OpenApiSchemaValidator.cs | 5 ++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs index 59ee68cb..fd06d280 100644 --- a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs +++ b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs @@ -991,4 +991,38 @@ public async Task ValidatesRepeatedSchemaReferencesEfficiently() Assert.Empty(valid!.Issues); Assert.Equal("$: Expected string but got integer", Assert.Single(Messages(invalid))); } + + [Fact] + public async Task ValidatesLargeInvalidArraysInLinearTime() + { + var plugin = await CreatePluginWithSpecsAsync(("array.yaml", """ + openapi: 3.0.3 + info: { title: Array, version: "1.0" } + servers: + - url: https://api.fabrikam.com + paths: + /items: + get: + responses: + "200": + description: ok + content: + application/json: + schema: + type: array + items: + allOf: + - type: string + """)); + const int count = 20_000; + var body = $"[{string.Join(',', Enumerable.Repeat("1", count))}]"; + + var stopwatch = System.Diagnostics.Stopwatch.StartNew(); + var result = plugin.Validate(TestExchange.Request("GET", "https://api.fabrikam.com/items") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: body).Session); + stopwatch.Stop(); + + Assert.Equal(count, result!.Issues.Count()); + Assert.True(stopwatch.Elapsed < TimeSpan.FromSeconds(5), $"Validation took {stopwatch.Elapsed}"); + } } diff --git a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs index 0ba029c4..136c2764 100644 --- a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs +++ b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs @@ -298,6 +298,9 @@ private static void ValidateCore( return; } + // errors reported for this value start here; earlier errors belong to + // other values, so they don't need to be considered for deduplication + var firstError = errors.Count; if (!ValidateType(value, schema, path, errors)) { // no point validating further if the type doesn't match @@ -322,7 +325,7 @@ private static void ValidateCore( if (schema.AllOf is { Count: > 0 }) { - var seen = new HashSet(errors); + var seen = new HashSet(errors.Skip(firstError)); for (var i = 0; i < schema.AllOf.Count; i++) { foreach (var subError in ValidateComposed(value, schema.AllOf[i], direction, path, state, depth + 1)) From e5c86f99ecd5cd5ddcb6e729c168761a4f54a737 Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 17:00:57 +0100 Subject: [PATCH 11/13] Warn when OpenAPI specs load with errors Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OpenApiValidationPluginTests.cs | 46 ++++++++++++++++++- .../Reporting/OpenApiValidationPlugin.cs | 6 +++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs index fd06d280..12c878a8 100644 --- a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs +++ b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs @@ -6,6 +6,7 @@ using DevProxy.Abstractions.Proxy; using DevProxy.Abstractions.Utils; using DevProxy.Plugins.Reporting; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Xunit; @@ -150,7 +151,8 @@ private static RecordingArgs Recording(IEnumerable logs) => GlobalData = new() { [ProxyUtils.ReportsKey] = new Dictionary() }, }; - private async Task CreatePluginAsync(string configJson = """{ "specFiles": [ "contoso.json" ] }""") + private async Task CreatePluginAsync(string configJson = """{ "specFiles": [ "contoso.json" ] }""", + ILogger? logger = null) { await File.WriteAllTextAsync(Path.Combine(_dir.FullName, "contoso.json"), Spec); @@ -160,7 +162,7 @@ private async Task CreatePluginAsync(string configJson }; var plugin = new OpenApiValidationPlugin( TestDefaults.HttpClient, - NullLogger.Instance, + logger ?? NullLogger.Instance, KestrelProxyHarness.BuildUrlsToWatch("api.contoso.com"), proxyConfig, PluginConfig.FromJson(configJson)); @@ -1025,4 +1027,44 @@ public async Task ValidatesLargeInvalidArraysInLinearTime() Assert.Equal(count, result!.Issues.Count()); Assert.True(stopwatch.Elapsed < TimeSpan.FromSeconds(5), $"Validation took {stopwatch.Elapsed}"); } + + [Fact] + public async Task WarnsAboutSpecErrors() + { + await File.WriteAllTextAsync(Path.Combine(_dir.FullName, "broken.yaml"), """ + openapi: 3.0.3 + info: { title: Broken, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /items: + get: + parameters: + - { name: id, in: nowhere, schema: { type: integer } } + responses: { '204': { description: ok } } + """); + var logger = new WarningLogger(); + + var plugin = await CreatePluginAsync("""{ "specFiles": [ "broken.yaml" ] }""", logger); + + Assert.NotNull(plugin.Validate(Call("GET", "https://api.fabrikam.com/items").Session)); + Assert.Contains(logger.Warnings, w => w.Contains("broken.yaml' has errors", StringComparison.Ordinal)); + } + + private sealed class WarningLogger : ILogger + { + public List Warnings { get; } = []; + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + { + if (logLevel == LogLevel.Warning) + { + Warnings.Add(formatter(state, exception)); + } + } + } } diff --git a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs index 3e8b08ec..65fcb372 100644 --- a/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs +++ b/DevProxy.Plugins/Reporting/OpenApiValidationPlugin.cs @@ -837,6 +837,12 @@ private async Task> LoadSpecsAsync(IEnumerable files, C continue; } + if (diagnostics is not null) + { + // the spec was loaded, but parts of it might've been ignored + Logger.LogWarning("OpenAPI spec '{File}' has errors and might not be validated completely: {Diagnostics}", file, diagnostics); + } + var servers = BuildServerScopes(document, file); if (servers.Count == 0) { From c8ca15efe2bf2eca3067c396a2d51233f9d0c80f Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 16:27:05 +0100 Subject: [PATCH 12/13] Support discriminator for oneOf in OpenApiValidationPlugin Closes #2040 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OpenApiValidationPluginTests.cs | 90 +++++++++++++++++++ .../Utils/OpenApiSchemaValidator.cs | 87 +++++++++++++++++- 2 files changed, 175 insertions(+), 2 deletions(-) diff --git a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs index 12c878a8..e6e475d4 100644 --- a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs +++ b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs @@ -1067,4 +1067,94 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except } } } + + private const string DiscriminatorSpec = """ + openapi: 3.0.3 + info: { title: Pets, version: '1.0' } + servers: + - url: https://api.fabrikam.com + paths: + /pets: + post: + requestBody: + content: + application/json: + schema: + oneOf: + - $ref: '#/components/schemas/Cat' + - $ref: '#/components/schemas/Dog' + - $ref: '#/components/schemas/Lizard' + discriminator: + propertyName: petType + mapping: + doggo: '#/components/schemas/Dog' + responses: + '204': { description: ok } + components: + schemas: + Cat: + type: object + required: [ petType ] + properties: { petType: { type: string }, name: { type: string }, meows: { type: boolean } } + Dog: + type: object + required: [ petType ] + properties: { petType: { type: string }, name: { type: string }, barks: { type: boolean } } + Lizard: + type: object + required: [ petType ] + properties: { petType: { type: string }, name: { type: string } } + """; + + private static TestExchange PostPet(string body) => + TestExchange.Request("POST", "https://api.fabrikam.com/pets", + headers: [("Content-Type", "application/json")], body: body) + .WithResponse(HttpStatusCode.NoContent); + + [Fact] + public async Task UsesDiscriminatorToSelectOneOfBranch() + { + var plugin = await CreatePluginWithSpecsAsync(("pets.yaml", DiscriminatorSpec)); + + // without the discriminator, these would match all overlapping branches + var implicitName = plugin.Validate(PostPet("""{ "petType": "Cat", "name": "Tom" }""").Session); + var mapped = plugin.Validate(PostPet("""{ "petType": "doggo", "name": "Rex" }""").Session); + var implicitNameWithMapping = plugin.Validate(PostPet("""{ "petType": "Dog", "name": "Rex" }""").Session); + var invalidBranch = plugin.Validate(PostPet("""{ "petType": "doggo", "barks": "loud" }""").Session); + + Assert.Empty(implicitName!.Issues); + Assert.Empty(mapped!.Issues); + Assert.Empty(implicitNameWithMapping!.Issues); + Assert.Equal("$.barks: Expected boolean but got string", Assert.Single(Messages(invalidBranch))); + } + + [Fact] + public async Task ReportsMissingOrUnknownDiscriminatorValues() + { + var plugin = await CreatePluginWithSpecsAsync(("pets.yaml", DiscriminatorSpec)); + + var missing = plugin.Validate(PostPet("""{ "name": "Tom" }""").Session); + var unknown = plugin.Validate(PostPet("""{ "petType": "Fish" }""").Session); + var notString = plugin.Validate(PostPet("""{ "petType": 1 }""").Session); + + Assert.Equal("$: Missing discriminator property 'petType' required to select a schema in 'oneOf'", Assert.Single(Messages(missing))); + Assert.Equal("$.petType: Discriminator value 'Fish' doesn't map to any schema in 'oneOf'. Allowed values: 'doggo', 'Cat', 'Dog', 'Lizard'", Assert.Single(Messages(unknown))); + Assert.Equal("$.petType: Discriminator property 'petType' must be a string but got integer", Assert.Single(Messages(notString))); + } + + [Fact] + public async Task FallsBackToDiscriminatorDefaultMapping() + { + var spec = DiscriminatorSpec + .Replace("openapi: 3.0.3", "openapi: 3.2.0", StringComparison.Ordinal) + .Replace("propertyName: petType", "propertyName: petType\n defaultMapping: '#/components/schemas/Lizard'", StringComparison.Ordinal) + .Replace("Lizard:\n type: object\n required: [ petType ]", "Lizard:\n type: object\n required: [ name ]", StringComparison.Ordinal); + var plugin = await CreatePluginWithSpecsAsync(("pets.yaml", spec)); + + var missing = plugin.Validate(PostPet("""{ "name": "Spike" }""").Session); + var unknown = plugin.Validate(PostPet("""{ "petType": "Gecko" }""").Session); + + Assert.Empty(missing!.Issues); + Assert.Equal("$: Missing required property 'name'", Assert.Single(Messages(unknown))); + } } diff --git a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs index 136c2764..0b71e92c 100644 --- a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs +++ b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs @@ -23,7 +23,7 @@ internal sealed record SchemaValidationError(string Path, string Message, string /// Lightweight validator of JSON values against OpenAPI schemas /// (Microsoft.OpenApi object model). Supports: type (incl. nullable / type null), /// required, properties, additionalProperties, items, enum, allOf, anyOf, oneOf -/// and $ref. Other keywords (format, min/max, pattern, etc.) are ignored. +/// (incl. discriminator) and $ref. Other keywords (format, min/max, pattern, etc.) are ignored. /// internal static class OpenApiSchemaValidator { @@ -359,7 +359,13 @@ private static void ValidateCore( errors.Add(new(path, "Value doesn't match any of the schemas in 'anyOf'")); } - if (schema.OneOf is { Count: > 0 }) + if (schema.OneOf is { Count: > 0 } && + schema.Discriminator?.PropertyName is { Length: > 0 } && + value.ValueKind == JsonValueKind.Object) + { + ValidateDiscriminatedOneOf(value, schema, schema.Discriminator, direction, path, errors, firstError, state, depth); + } + else if (schema.OneOf is { Count: > 0 }) { var matches = schema.OneOf.Count(s => IsValid(value, s, direction, path, state, depth)); if (matches == 0) @@ -373,6 +379,83 @@ private static void ValidateCore( } } + // Validates the value only against the oneOf branch selected by the + // discriminator, so that overlapping branches don't cause false positives + private static void ValidateDiscriminatedOneOf( + JsonElement value, + IOpenApiSchema schema, + OpenApiDiscriminator discriminator, + SchemaValidationDirection direction, + string path, + List errors, + int firstError, + ValidationState state, + int depth) + { + var propertyName = discriminator.PropertyName!; + IOpenApiSchema? selected; + if (!value.TryGetProperty(propertyName, out var discriminatorValue)) + { + selected = discriminator.DefaultMapping; + if (selected is null) + { + errors.Add(new(path, $"Missing discriminator property '{propertyName}' required to select a schema in 'oneOf'")); + return; + } + } + else if (discriminatorValue.ValueKind != JsonValueKind.String) + { + errors.Add(new(GetPropertyPath(path, propertyName), $"Discriminator property '{propertyName}' must be a string but got {DescribeValueKind(discriminatorValue)}")); + return; + } + else + { + var discriminatorName = discriminatorValue.GetString()!; + selected = FindDiscriminatedSchema(schema.OneOf!, discriminator, discriminatorName) ?? discriminator.DefaultMapping; + if (selected is null) + { + var allowed = GetDiscriminatorValues(schema.OneOf!, discriminator); + var allowedMessage = allowed.Count > 0 + ? $". Allowed values: {string.Join(", ", allowed.Select(v => $"'{v}'"))}" + : string.Empty; + errors.Add(new(GetPropertyPath(path, propertyName), $"Discriminator value '{discriminatorName}' doesn't map to any schema in 'oneOf'{allowedMessage}")); + return; + } + } + + var seen = new HashSet(errors.Skip(firstError)); + foreach (var error in ValidateComposed(value, selected, direction, path, state, depth + 1)) + { + if (seen.Add(error)) + { + errors.Add(error); + } + } + } + + // Explicit mappings take precedence over implicit ones, which use + // the name of the referenced component schema + private static IOpenApiSchema? FindDiscriminatedSchema(IList oneOf, OpenApiDiscriminator discriminator, string discriminatorValue) + { + if (discriminator.Mapping is not null && + discriminator.Mapping.TryGetValue(discriminatorValue, out var mapped)) + { + return mapped.Target is not null + ? mapped + : oneOf.FirstOrDefault(s => GetSchemaName(s) == mapped.Reference?.Id); + } + + return oneOf.FirstOrDefault(s => GetSchemaName(s) == discriminatorValue); + } + + private static List GetDiscriminatorValues(IList oneOf, OpenApiDiscriminator discriminator) => + [.. (discriminator.Mapping?.Keys ?? Enumerable.Empty()) + .Concat(oneOf.Select(GetSchemaName).OfType()) + .Distinct(StringComparer.Ordinal)]; + + private static string? GetSchemaName(IOpenApiSchema schema) => + schema is OpenApiSchemaReference reference ? reference.Reference?.Id : null; + private static bool IsValid(JsonElement value, IOpenApiSchema schema, SchemaValidationDirection direction, string path, ValidationState state, int depth) => ValidateComposed(value, schema, direction, path, state, depth + 1).Count == 0; From e82a5a15c55eb61123fbc98172cb625512c29e5b Mon Sep 17 00:00:00 2001 From: waldekmastykarz Date: Sat, 31 Oct 2026 19:12:50 +0100 Subject: [PATCH 13/13] Restrict discriminator mappings to declared oneOf alternatives Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OpenApiValidationPluginTests.cs | 24 +++++++ .../Utils/OpenApiSchemaValidator.cs | 62 +++++++++++++------ 2 files changed, 66 insertions(+), 20 deletions(-) diff --git a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs index e6e475d4..057cfb02 100644 --- a/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs +++ b/DevProxy.Integration.Tests/OpenApiValidationPluginTests.cs @@ -1142,6 +1142,30 @@ public async Task ReportsMissingOrUnknownDiscriminatorValues() Assert.Equal("$.petType: Discriminator property 'petType' must be a string but got integer", Assert.Single(Messages(notString))); } + [Fact] + public async Task ReportsDiscriminatorMappingsOutsideOneOf() + { + var spec = DiscriminatorSpec + .Replace("doggo: '#/components/schemas/Dog'", "doggo: '#/components/schemas/Dog'\n fish: '#/components/schemas/Fish'", StringComparison.Ordinal) + .Replace(" Lizard:", " Fish:\n type: object\n properties: { fins: { type: integer } }\n Lizard:", StringComparison.Ordinal); + var defaultSpec = spec + .Replace("openapi: 3.0.3", "openapi: 3.2.0", StringComparison.Ordinal) + .Replace("propertyName: petType", "propertyName: petType\n defaultMapping: '#/components/schemas/Fish'", StringComparison.Ordinal); + var plugin = await CreatePluginWithSpecsAsync(("pets.yaml", spec)); + + var stale = plugin.Validate(PostPet("""{ "petType": "fish", "fins": 2 }""").Session); + var unknown = plugin.Validate(PostPet("""{ "petType": "Bird" }""").Session); + + Assert.Equal("$.petType: Discriminator value 'fish' maps to schema 'Fish', which isn't one of the schemas in 'oneOf'", Assert.Single(Messages(stale))); + Assert.DoesNotContain("'fish'", Assert.Single(Messages(unknown)), StringComparison.Ordinal); + + plugin = await CreatePluginWithSpecsAsync(("pets-default.yaml", defaultSpec)); + + var missing = plugin.Validate(PostPet("""{ "fins": 2 }""").Session); + + Assert.Equal("$: Discriminator default mapping maps to schema 'Fish', which isn't one of the schemas in 'oneOf'", Assert.Single(Messages(missing))); + } + [Fact] public async Task FallsBackToDiscriminatorDefaultMapping() { diff --git a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs index 0b71e92c..d76f7d3f 100644 --- a/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs +++ b/DevProxy.Plugins/Utils/OpenApiSchemaValidator.cs @@ -393,15 +393,19 @@ private static void ValidateDiscriminatedOneOf( int depth) { var propertyName = discriminator.PropertyName!; - IOpenApiSchema? selected; + var oneOf = schema.OneOf!; + var errorPath = path; + IOpenApiSchema? candidate; + string source; if (!value.TryGetProperty(propertyName, out var discriminatorValue)) { - selected = discriminator.DefaultMapping; - if (selected is null) + if (discriminator.DefaultMapping is null) { errors.Add(new(path, $"Missing discriminator property '{propertyName}' required to select a schema in 'oneOf'")); return; } + candidate = discriminator.DefaultMapping; + source = "Discriminator default mapping"; } else if (discriminatorValue.ValueKind != JsonValueKind.String) { @@ -411,18 +415,40 @@ private static void ValidateDiscriminatedOneOf( else { var discriminatorName = discriminatorValue.GetString()!; - selected = FindDiscriminatedSchema(schema.OneOf!, discriminator, discriminatorName) ?? discriminator.DefaultMapping; - if (selected is null) + errorPath = GetPropertyPath(path, propertyName); + source = $"Discriminator value '{discriminatorName}'"; + // explicit mappings take precedence over implicit ones, which use + // the name of the referenced component schema + candidate = discriminator.Mapping is not null && + discriminator.Mapping.TryGetValue(discriminatorName, out var mapped) + ? mapped + : oneOf.FirstOrDefault(s => GetSchemaName(s) == discriminatorName); + if (candidate is null && discriminator.DefaultMapping is not null) + { + candidate = discriminator.DefaultMapping; + source = "Discriminator default mapping"; + } + if (candidate is null) { - var allowed = GetDiscriminatorValues(schema.OneOf!, discriminator); + var allowed = GetDiscriminatorValues(oneOf, discriminator); var allowedMessage = allowed.Count > 0 ? $". Allowed values: {string.Join(", ", allowed.Select(v => $"'{v}'"))}" : string.Empty; - errors.Add(new(GetPropertyPath(path, propertyName), $"Discriminator value '{discriminatorName}' doesn't map to any schema in 'oneOf'{allowedMessage}")); + errors.Add(new(errorPath, $"Discriminator value '{discriminatorName}' doesn't map to any schema in 'oneOf'{allowedMessage}")); return; } } + // mappings must not select a schema outside the declared alternatives, + // otherwise the value could bypass oneOf validation + var selected = FindOneOfBranch(oneOf, candidate); + if (selected is null) + { + var schemaName = GetSchemaName(candidate) is { } name ? $"schema '{name}'" : "a schema"; + errors.Add(new(errorPath, $"{source} maps to {schemaName}, which isn't one of the schemas in 'oneOf'")); + return; + } + var seen = new HashSet(errors.Skip(firstError)); foreach (var error in ValidateComposed(value, selected, direction, path, state, depth + 1)) { @@ -433,23 +459,19 @@ private static void ValidateDiscriminatedOneOf( } } - // Explicit mappings take precedence over implicit ones, which use - // the name of the referenced component schema - private static IOpenApiSchema? FindDiscriminatedSchema(IList oneOf, OpenApiDiscriminator discriminator, string discriminatorValue) + private static IOpenApiSchema? FindOneOfBranch(IList oneOf, IOpenApiSchema candidate) { - if (discriminator.Mapping is not null && - discriminator.Mapping.TryGetValue(discriminatorValue, out var mapped)) - { - return mapped.Target is not null - ? mapped - : oneOf.FirstOrDefault(s => GetSchemaName(s) == mapped.Reference?.Id); - } - - return oneOf.FirstOrDefault(s => GetSchemaName(s) == discriminatorValue); + var target = ResolveTarget(candidate); + var name = GetSchemaName(candidate); + return oneOf.FirstOrDefault(s => + ReferenceEquals(ResolveTarget(s), target) || + (name is not null && GetSchemaName(s) == name)); } private static List GetDiscriminatorValues(IList oneOf, OpenApiDiscriminator discriminator) => - [.. (discriminator.Mapping?.Keys ?? Enumerable.Empty()) + [.. (discriminator.Mapping ?? Enumerable.Empty>()) + .Where(m => FindOneOfBranch(oneOf, m.Value) is not null) + .Select(m => m.Key) .Concat(oneOf.Select(GetSchemaName).OfType()) .Distinct(StringComparer.Ordinal)];