Skip to content

Commit

Permalink
Ignore comments in SQL composability check
Browse files Browse the repository at this point in the history
And rewrite without regex

Fixes #18412
  • Loading branch information
roji committed Jun 9, 2020
1 parent d3ad139 commit cc35fd2
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 4 deletions.
49 changes: 45 additions & 4 deletions src/EFCore.Relational/Query/QuerySqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq;
using System.Linq.Expressions;
using System.Text.RegularExpressions;
using System.Threading;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
Expand All @@ -26,9 +27,6 @@ namespace Microsoft.EntityFrameworkCore.Query
/// </summary>
public class QuerySqlGenerator : SqlExpressionVisitor
{
private static readonly Regex _composableSql
= new Regex(@"^\s*?SELECT\b", RegexOptions.IgnoreCase, TimeSpan.FromMilliseconds(value: 1000.0));

private readonly IRelationalCommandBuilderFactory _relationalCommandBuilderFactory;
private readonly ISqlGenerationHelper _sqlGenerationHelper;
private IRelationalCommandBuilder _relationalCommandBuilder;
Expand Down Expand Up @@ -437,10 +435,53 @@ protected virtual void CheckComposableSql([NotNull] string sql)
{
Check.NotNull(sql, nameof(sql));

if (!_composableSql.IsMatch(sql))
var pos = -1;
char c;

do
{
c = NextChar();

if (char.IsWhiteSpace(c))
{
continue;
}

if (c == '-')
{
if (NextChar() != '-')
{
throw new InvalidOperationException(RelationalStrings.FromSqlNonComposable);
}

while (NextChar() != '\n') { }

continue;
}

if (char.ToLowerInvariant(c) == 's' &&
char.ToLowerInvariant(NextChar()) == 'e' &&
char.ToLowerInvariant(NextChar()) == 'l' &&
char.ToLowerInvariant(NextChar()) == 'e' &&
char.ToLowerInvariant(NextChar()) == 'c' &&
char.ToLowerInvariant(NextChar()) == 't')
{
c = NextChar();
if (char.IsWhiteSpace(c)
|| c == '-' && NextChar() == '-')
{
return;
}
}

throw new InvalidOperationException(RelationalStrings.FromSqlNonComposable);
}
while (true);

char NextChar()
=> ++pos < sql.Length
? sql[pos]
: throw new InvalidOperationException(RelationalStrings.FromSqlNonComposable);
}

/// <inheritdoc />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;

namespace Microsoft.EntityFrameworkCore.Query.Internal
{
public class QuerySqlGeneratorTest
{
[Theory]
[InlineData("INSERT something")]
[InlineData("SELECTANDSOMEOTHERSTUFF")]
[InlineData("SELECT")]
[InlineData("SELEC")]
[InlineData("- bad comment\nSELECT something")]
[InlineData("SELECT-\n1")]
[InlineData("")]
public void CheckComposableSql_throws(string sql)
=> Assert.Equal(
RelationalStrings.FromSqlNonComposable,
Assert.Throws<InvalidOperationException>(
() => CreateDummyQuerySqlGenerator().CheckComposableSql(sql)).Message);

[Theory]
[InlineData("SELECT something")]
[InlineData(" SELECT something")]
[InlineData("-- comment\n SELECT something")]
[InlineData("-- comment1\r\n --\t\rcomment2\r\nSELECT something")]
[InlineData("SELECT--\n1")]
public void CheckComposableSql_does_not_throw(string sql)
=> CreateDummyQuerySqlGenerator().CheckComposableSql(sql);

private DummyQuerySqlGenerator CreateDummyQuerySqlGenerator()
=> new DummyQuerySqlGenerator(
new QuerySqlGeneratorDependencies(
new RelationalCommandBuilderFactory(
new RelationalCommandBuilderDependencies(
new TestRelationalTypeMappingSource(
TestServiceFactory.Instance.Create<TypeMappingSourceDependencies>(),
TestServiceFactory.Instance.Create<RelationalTypeMappingSourceDependencies>()))),
new RelationalSqlGenerationHelper(
new RelationalSqlGenerationHelperDependencies())));

class DummyQuerySqlGenerator : QuerySqlGenerator
{
public DummyQuerySqlGenerator([NotNull] QuerySqlGeneratorDependencies dependencies)
: base(dependencies)
{
}

public new void CheckComposableSql(string sql)
=> base.CheckComposableSql(sql);
}
}
}

0 comments on commit cc35fd2

Please sign in to comment.