Merge pull request #2660 from microsoft/fix/pointer-to-v2

fix: wrap extension parser calls in try-catch to ensure correct error pointers

Author: baywet

src/Microsoft.OpenApi/Reader/V2/OpenApiV2Deserializer.cs

@@ -79,12 +79,18 @@ private static IOpenApiExtension LoadExtension(string name, ParseNode node)
         {
             if (node.Context.ExtensionParsers is not null && node.Context.ExtensionParsers.TryGetValue(name, out var parser))
             {
-                return parser(node.CreateAny(), OpenApiSpecVersion.OpenApi2_0);
-            }
-            else
-            {
-                return new JsonNodeExtension(node.CreateAny());
+                try
+                {
+                    return parser(node.CreateAny(), OpenApiSpecVersion.OpenApi2_0);
+                }
+                catch (OpenApiException ex)
+                {
+                    ex.Pointer = node.Context.GetLocation();
+                    node.Context.Diagnostic.Errors.Add(new(ex));
+                }
             }
+
+            return new JsonNodeExtension(node.CreateAny());
         }
 
         private static string? LoadString(ParseNode node)

src/Microsoft.OpenApi/Reader/V3/OpenApiV3Deserializer.cs

@@ -130,15 +130,24 @@ public static JsonNodeExtension LoadAny(ParseNode node, OpenApiDocument hostDocu
 
         private static IOpenApiExtension LoadExtension(string name, ParseNode node)
         {
-            if (node.Context.ExtensionParsers is not null && node.Context.ExtensionParsers.TryGetValue(name, out var parser) && parser(
-                node.CreateAny(), OpenApiSpecVersion.OpenApi3_0) is { } result)
+            if (node.Context.ExtensionParsers is not null && node.Context.ExtensionParsers.TryGetValue(name, out var parser))
             {
-                return result;
-            }
-            else
-            {
-                return new JsonNodeExtension(node.CreateAny());
+                try
+                {
+                    var result = parser(node.CreateAny(), OpenApiSpecVersion.OpenApi3_0);
+                    if (result is { })
+                    {
+                        return result;
+                    }
+                }
+                catch (OpenApiException ex)
+                {
+                    ex.Pointer = node.Context.GetLocation();
+                    node.Context.Diagnostic.Errors.Add(new(ex));
+                }
             }
+
+            return new JsonNodeExtension(node.CreateAny());
         }
 
         private static string? LoadString(ParseNode node)

src/Microsoft.OpenApi/Reader/V31/OpenApiV31Deserializer.cs

@@ -131,9 +131,20 @@ public static JsonNode LoadAny(ParseNode node, OpenApiDocument hostDocument)
 
         private static IOpenApiExtension LoadExtension(string name, ParseNode node)
         {
-            return node.Context.ExtensionParsers is not null && node.Context.ExtensionParsers.TryGetValue(name, out var parser)
-                ? parser(node.CreateAny(), OpenApiSpecVersion.OpenApi3_1)
-                : new JsonNodeExtension(node.CreateAny());
+            if (node.Context.ExtensionParsers is not null && node.Context.ExtensionParsers.TryGetValue(name, out var parser))
+            {
+                try
+                {
+                    return parser(node.CreateAny(), OpenApiSpecVersion.OpenApi3_1);
+                }
+                catch (OpenApiException ex)
+                {
+                    ex.Pointer = node.Context.GetLocation();
+                    node.Context.Diagnostic.Errors.Add(new(ex));
+                }
+            }
+
+            return new JsonNodeExtension(node.CreateAny());
         }
 
         private static string? LoadString(ParseNode node)

test/Microsoft.OpenApi.Readers.Tests/TestCustomExtension.cs

@@ -44,6 +44,80 @@ public void ParseCustomExtension()
             Assert.Equal("hey", fooExtension.Bar);
             Assert.Equal("hi!", fooExtension.Baz);
         }
+
+        [Fact]
+        public void ExtensionParserThrowingOpenApiException_V2_ShouldHaveCorrectPointer()
+        {
+            var json = """
+{
+  "swagger": "2.0",
+  "info": {
+    "title": "Demo",
+    "version": "1"
+  },
+  "paths": {},
+  "definitions": {
+    "demo": {
+      "x-tag": null
+    }
+  }
+}
+""";
+            var settings = new OpenApiReaderSettings
+            {
+                ExtensionParsers =
+                {
+                    { "x-tag", (any, version) => throw new OpenApiException("Testing") }
+                }
+            };
+
+            var result = OpenApiDocument.Parse(json, "json", settings);
+
+            Assert.NotNull(result.Diagnostic);
+            Assert.NotEmpty(result.Diagnostic.Errors);
+            var error = result.Diagnostic.Errors[0];
+            Assert.Equal("Testing", error.Message);
+            Assert.Equal("#/definitions/demo/x-tag", error.Pointer);
+        }
+
+        [Theory]
+        [InlineData("3.0.4")]
+        [InlineData("3.1.1")]
+        public void ExtensionParserThrowingOpenApiException_V3_ShouldHaveCorrectPointer(string version)
+        {
+            var json = $$"""
+{
+  "openapi": "{{version}}",
+  "info": {
+    "title": "Demo",
+    "version": "1"
+  },
+  "paths": {},
+  "components": {
+    "schemas": {
+      "demo": {
+        "x-tag": null
+      }
+    }
+  }
+}
+""";
+            var settings = new OpenApiReaderSettings
+            {
+                ExtensionParsers =
+                {
+                    { "x-tag", (any, version) => throw new OpenApiException("Testing") }
+                }
+            };
+
+            var result = OpenApiDocument.Parse(json, "json", settings);
+
+            Assert.NotNull(result.Diagnostic);
+            Assert.NotEmpty(result.Diagnostic.Errors);
+            var error = result.Diagnostic.Errors[0];
+            Assert.Equal("Testing", error.Message);
+            Assert.Equal("#/components/schemas/demo/x-tag", error.Pointer);
+        }
     }
 
     internal class FooExtension : IOpenApiExtension, IOpenApiElement