improve parsing and diagnostics for HTTP version node (#3153)

* add some negative combinatorial tests; improve URL diagnostics * improve parsing and diagnostics for HTTP version node
dotnet · Aug 25, 2023 · 4788158 · 4788158
1 parent 2ebbb6a
commit 4788158
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 47 deletions.
diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Combinatorial.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Combinatorial.cs
@@ -1,7 +1,6 @@
 // Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-using System;
 using System.Collections.Generic;
 using System.Linq;
 using FluentAssertions;
@@ -63,32 +62,6 @@ public void Invalid_syntax_produces_diagnostics(ISyntaxSpec syntaxSpec, int inde
             syntaxSpec.Validate(parseResult.SyntaxTree.RootNode.ChildNodes.Single());
         }
 
-        [Fact]
-        public void DEBUG_ME()
-        {
-            var code = """
-
-                 hptps://example.com 
-                Authorization: Basic {{token}}
-                Cookie: {{cookie}}
-
-
-                { 
-                    "number": {{numberValue}},
-                    "string": 
-                        {{stringValue}} 
-                }
-
-
-                """;
-
-            var result = Parse(code);
-
-            result.SyntaxTree.RootNode.ChildNodes.Should().ContainSingle<HttpRequestNode>()
-                  .Which.ChildNodes.Should().ContainSingle<HttpUrlNode>()
-                  .Which.Text.Should().Be("hptps://example.com");
-        }
-
         public static IEnumerable<object[]> GenerateValidRequests()
         {
             var i = 0;
@@ -144,20 +117,20 @@ public static IEnumerable<object[]> GenerateInvalidRequests()
             }
 
             // FIX: (GenerateInvalidRequests) 
-            // foreach (var method in ValidMethods())
-            // foreach (var url in ValidUrls())
-            // foreach (var version in InvalidVersions())
-            // foreach (var headerSection in ValidHeaderSections())
-            // foreach (var bodySection in ValidBodySections())
-            // {
-            //     ++i;
-            //     yield return new object[]
-            //     {
-            //         new HttpRequestNodeSyntaxSpec(method, url, version, headerSection, bodySection),
-            //         i
-            //     };
-            // }
-            //
+            foreach (var method in ValidMethods())
+            foreach (var url in ValidUrls())
+            foreach (var version in InvalidVersions())
+            foreach (var headerSection in ValidHeaderSections())
+            foreach (var bodySection in ValidBodySections())
+            {
+                ++i;
+                yield return new object[]
+                {
+                    new HttpRequestNodeSyntaxSpec(method, url, version, headerSection, bodySection),
+                    i
+                };
+            }
+
             // foreach (var method in ValidMethods())
             // foreach (var url in ValidUrls())
             // foreach (var version in ValidVersions())
@@ -206,7 +179,6 @@ private static IEnumerable<HttpUrlNodeSyntaxSpec> InvalidUrls()
         private static IEnumerable<HttpVersionNodeSyntaxSpec> ValidVersions()
         {
             yield return new("");
-            yield return new("HTTP/1.0");
             yield return new("HTTP/1.1");
         }
 

diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Url.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Url.cs
@@ -34,7 +34,7 @@ public void newline_is_legal_at_the_after_url()
 
             result.SyntaxTree.RootNode
                   .ChildNodes.Should().ContainSingle<HttpRequestNode>().Which
-                  .UrlNode.ChildTokens.Last().Kind.Should().Be(HttpTokenKind.NewLine);
+                  .ChildTokens.Last().Kind.Should().Be(HttpTokenKind.NewLine);
         }
 
         [Theory]

diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Version.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Version.cs
@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System;
 using FluentAssertions;
 using Microsoft.DotNet.Interactive.HttpRequest.Tests.Utility;
 using Xunit;
@@ -20,5 +21,26 @@ public void http_version_is_parsed_correctly()
                   .ChildNodes.Should().ContainSingle<HttpRequestNode>().Which
                   .VersionNode.Text.Should().Be("HTTP/1.1");
         }
+
+        [Fact]
+        public void http_version_containing_whitespace_produces_a_diagnostic()
+        {
+            var version = """HTTP 1.1""";
+
+            var code = $"""
+                https://example.com {version}
+                Accept: application/json
+                """;
+
+            var result = Parse(code);
+
+            var versionNode = result.SyntaxTree.RootNode.ChildNodes.Should().ContainSingle<HttpRequestNode>()
+                                        .Which.ChildNodes.Should().ContainSingle<HttpVersionNode>()
+                                        .Which;
+
+            versionNode.Text.Should().Be(version);
+            versionNode.GetDiagnostics().Should().ContainSingle()
+                       .Which.Message.Should().Be("Invalid HTTP version");
+        }
     }
 }
diff --git a/src/Microsoft.DotNet.Interactive.HttpRequestParser/HttpRequestParser.cs b/src/Microsoft.DotNet.Interactive.HttpRequestParser/HttpRequestParser.cs
@@ -189,6 +189,10 @@ private HttpRequestNode ParseRequest()
             {
                 requestNode.Add(versionNode);
             }
+            else 
+            {
+                ParseTrailingTrivia(requestNode, stopAfterNewLine: true);
+            }
 
             var headersNode = ParseHeaders();
             if (headersNode is not null)
@@ -298,7 +302,7 @@ private HttpRequestNode ParseRequest()
             }
 
             return node is not null
-                       ? ParseTrailingTrivia(node, stopAfterNewLine: true)
+                       ? ParseTrailingTrivia(node, stopBeforeNewline: true)
                        : null;
         }
 
@@ -382,8 +386,7 @@ private HttpExpressionEndNode ParseExpressionEnd()
         private HttpVersionNode? ParseVersion()
         {
             if (MoreTokens() &&
-                CurrentToken.Kind is HttpTokenKind.Word &&
-                CurrentToken.Text.ToLowerInvariant() is "http" or "https")
+                CurrentToken.Kind is HttpTokenKind.Word)
             {
                 var node = new HttpVersionNode(_sourceText, _syntaxTree);
 

diff --git a/src/Microsoft.DotNet.Interactive.HttpRequestParser/HttpVersionNode.cs b/src/Microsoft.DotNet.Interactive.HttpRequestParser/HttpVersionNode.cs
@@ -3,6 +3,8 @@
 
 #nullable enable
 
+using System.Collections.Generic;
+using System.Linq;
 using Microsoft.CodeAnalysis.Text;
 
 namespace Microsoft.DotNet.Interactive.HttpRequest;
@@ -12,4 +14,25 @@ internal class HttpVersionNode : HttpSyntaxNode
     internal HttpVersionNode(SourceText sourceText, HttpSyntaxTree? syntaxTree) : base(sourceText, syntaxTree)
     {
     }
-}
+
+    public override IEnumerable<Diagnostic> GetDiagnostics()
+    {
+        foreach (var diagnostic in base.GetDiagnostics())
+        {
+            yield return diagnostic;
+        }
+
+        if (ChildTokens.FirstOrDefault() is { Kind: HttpTokenKind.Word } word)
+        {
+            if (word.Text.ToLowerInvariant() is not "http" and not "https")
+            {
+                yield return CreateDiagnostic("Invalid HTTP version");
+            }
+
+            if (ChildNodesAndTokens.OfType<HttpSyntaxToken>().Any(t => t.Kind is HttpTokenKind.Whitespace))
+            {
+                yield return CreateDiagnostic("Invalid HTTP version");
+            }
+        }
+    }
+}