Skip to content

fix(Engine): Evaluation fails when a numeric trait meets a condition value of another type #206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
emyller wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(Engine): Evaluation fails when a numeric trait meets a condition value of another type #206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 15 additions & 20 deletions Flagsmith.Engine/Engine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ private static bool MatchingFunctionName(object contextValue, Condition conditio
switch (contextValue.GetType().FullName)
{
case "System.Int32":
return IntOperations((Int32)contextValue, condition);
return LongOperations((Int32)contextValue, condition);
case "System.Int64":
return LongOperations((Int64)contextValue, condition);
case "System.Double":
Expand Down Expand Up @@ -411,30 +411,25 @@ private static bool LongOperations(long contextValue, Condition condition)
}
}

private static bool IntOperations(long contextValue, Condition condition)
private static bool DoubleOperations(double contextValue, Condition condition)
{
switch (condition.Operator)
double conditionValue;
try
{
case Operator.Equal: return contextValue == InvariantConvert.ToInt32(condition.Value.String);
case Operator.NotEqual: return contextValue != InvariantConvert.ToInt32(condition.Value.String);
case Operator.GreaterThan: return contextValue > InvariantConvert.ToInt32(condition.Value.String);
case Operator.GreaterThanInclusive: return contextValue >= InvariantConvert.ToInt32(condition.Value.String);
case Operator.LessThan: return contextValue < InvariantConvert.ToInt32(condition.Value.String);
case Operator.LessThanInclusive: return contextValue <= InvariantConvert.ToInt32(condition.Value.String);
default: throw new ArgumentException("Invalid Operator");
conditionValue = InvariantConvert.ToDouble(condition.Value.String);
}
catch (FormatException)
{
return false;
}
}

private static bool DoubleOperations(double contextValue, Condition condition)
{
switch (condition.Operator)
{
case Operator.Equal: return contextValue == InvariantConvert.ToDouble(condition.Value.String);
case Operator.NotEqual: return contextValue != InvariantConvert.ToDouble(condition.Value.String);
case Operator.GreaterThan: return contextValue > InvariantConvert.ToDouble(condition.Value.String);
case Operator.GreaterThanInclusive: return contextValue >= InvariantConvert.ToDouble(condition.Value.String);
case Operator.LessThan: return contextValue < InvariantConvert.ToDouble(condition.Value.String);
case Operator.LessThanInclusive: return contextValue <= InvariantConvert.ToDouble(condition.Value.String);
case Operator.Equal: return contextValue == conditionValue;
case Operator.NotEqual: return contextValue != conditionValue;
case Operator.GreaterThan: return contextValue > conditionValue;
case Operator.GreaterThanInclusive: return contextValue >= conditionValue;
case Operator.LessThan: return contextValue < conditionValue;
case Operator.LessThanInclusive: return contextValue <= conditionValue;
Comment on lines +414 to +432

@coderabbitai coderabbitai Bot Oct 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor · Stability & Availability · Quick win

Supported by static analysis

Script executed:

sed -n '410,440p' Flagsmith.Engine/Engine.cs
cat Flagsmith.Engine/Utils/InvariantConvert.cs
rg -n 'TargetFramework|TargetFrameworks|ToDouble\(' --glob '*.csproj' --glob '*.cs' Flagsmith.Engine Flagsmith.EngineTest

Repository: Flagsmith/flagsmith-dotnet-client

Length of output: 4186

Script executed:

set -o pipefail
printf '%s\n' '--- Engine dispatch and evaluation flow ---'
sed -n '1,190p' Flagsmith.Engine/Engine.cs
sed -n '190,380p' Flagsmith.Engine/Engine.cs
printf '%s\n' '--- MatchingFunctionName and DoubleOperations context ---'
sed -n '380,440p' Flagsmith.Engine/Engine.cs
printf '%s\n' '--- Relevant tests and project metadata ---'
rg -n -F --glob '*.cs' -e 'GetEvaluationResult' -e 'MatchingFunctionName' -e 'DoubleOperations' -e 'Trait' -e 'double' Flagsmith.EngineTest
cat Flagsmith.Engine/Flagsmith.Engine.csproj
cat Flagsmith.EngineTest/EngineTest.csproj
printf '%s\n' '--- Available .NET runtime metadata ---'
if command -v dotnet >/dev/null 2>&1; then dotnet --info; else printf '%s\n' 'dotnet executable unavailable'; fi

Repository: Flagsmith/flagsmith-dotnet-client

Length of output: 22973

Script executed:

set -o pipefail
printf '%s\n' '--- dispatch and callers ---'
rg -n -F --glob '*.cs' -e 'GetEvaluationResult' -e 'MatchingFunctionName' -e 'DoubleOperations' -e 'Trait' Flagsmith.Engine Flagsmith.EngineTest
sed -n '250,440p' Flagsmith.Engine/Engine.cs
printf '%s\n' '--- project files ---'
cat Flagsmith.Engine/Flagsmith.Engine.csproj
cat Flagsmith.EngineTest/EngineTest.csproj
printf '%s\n' '--- runtime availability ---'
command -v dotnet >/dev/null 2>&1 && dotnet --info || printf '%s\n' 'dotnet executable unavailable'

Repository: Flagsmith/flagsmith-dotnet-client

Length of output: 14018

Script executed:

set -o pipefail
printf '%s\n' '--- Engine flow ---'
sed -n '1,190p' Flagsmith.Engine/Engine.cs
sed -n '190,440p' Flagsmith.Engine/Engine.cs
printf '%s\n' '--- References and tests ---'
rg -n -F --glob '*.cs' -e 'GetEvaluationResult' -e 'MatchingFunctionName' -e 'DoubleOperations' -e 'System.Double' -e 'double' Flagsmith.Engine Flagsmith.EngineTest
printf '%s\n' '--- Target frameworks ---'
cat Flagsmith.Engine/Flagsmith.Engine.csproj
cat Flagsmith.EngineTest/EngineTest.csproj
printf '%s\n' '--- Available runtime ---'
if command -v dotnet >/dev/null 2>&1; then dotnet --info; else printf '%s\n' 'dotnet executable unavailable'; fi

Repository: Flagsmith/flagsmith-dotnet-client

Length of output: 22939

Catch OverflowException in DoubleOperations.

GetEvaluationResult can route a System.Double trait to DoubleOperations. On .NET Framework and pre-.NET Core 3 runtimes, Convert.ToDouble("1e309", CultureInfo.InvariantCulture) throws OverflowException. The current handler catches only FormatException, so evaluation can fail on older runtimes supported by the netstandard2.0 library target.

Suggested fix 
            catch (FormatException)
            {
                return false;
            }
+           catch (OverflowException)
+           {
+               return false;
+           }
Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private static bool DoubleOperations(double contextValue, Condition condition)
{
switch (condition.Operator)
double conditionValue;
try
{
case Operator.Equal: return contextValue == InvariantConvert.ToInt32(condition.Value.String);
case Operator.NotEqual: return contextValue != InvariantConvert.ToInt32(condition.Value.String);
case Operator.GreaterThan: return contextValue > InvariantConvert.ToInt32(condition.Value.String);
case Operator.GreaterThanInclusive: return contextValue >= InvariantConvert.ToInt32(condition.Value.String);
case Operator.LessThan: return contextValue < InvariantConvert.ToInt32(condition.Value.String);
case Operator.LessThanInclusive: return contextValue <= InvariantConvert.ToInt32(condition.Value.String);
default: throw new ArgumentException("Invalid Operator");
conditionValue = InvariantConvert.ToDouble(condition.Value.String);
}
catch (FormatException)
{
return false;
}
}
private static bool DoubleOperations(double contextValue, Condition condition)
{
switch (condition.Operator)
{
case Operator.Equal: return contextValue == InvariantConvert.ToDouble(condition.Value.String);
case Operator.NotEqual: return contextValue != InvariantConvert.ToDouble(condition.Value.String);
case Operator.GreaterThan: return contextValue > InvariantConvert.ToDouble(condition.Value.String);
case Operator.GreaterThanInclusive: return contextValue >= InvariantConvert.ToDouble(condition.Value.String);
case Operator.LessThan: return contextValue < InvariantConvert.ToDouble(condition.Value.String);
case Operator.LessThanInclusive: return contextValue <= InvariantConvert.ToDouble(condition.Value.String);
case Operator.Equal: return contextValue == conditionValue;
case Operator.NotEqual: return contextValue != conditionValue;
case Operator.GreaterThan: return contextValue > conditionValue;
case Operator.GreaterThanInclusive: return contextValue >= conditionValue;
case Operator.LessThan: return contextValue < conditionValue;
case Operator.LessThanInclusive: return contextValue <= conditionValue;
private static bool DoubleOperations(double contextValue, Condition condition)
{
double conditionValue;
try
{
conditionValue = InvariantConvert.ToDouble(condition.Value.String);
}
catch (FormatException)
{
return false;
}
catch (OverflowException)
{
return false;
}
switch (condition.Operator)
{
case Operator.Equal: return contextValue == conditionValue;
case Operator.NotEqual: return contextValue != conditionValue;
case Operator.GreaterThan: return contextValue > conditionValue;
case Operator.GreaterThanInclusive: return contextValue >= conditionValue;
case Operator.LessThan: return contextValue < conditionValue;
case Operator.LessThanInclusive: return contextValue <= conditionValue;

default: throw new ArgumentException("Invalid Operator");
}
}
Expand Down
9 changes: 0 additions & 9 deletions Flagsmith.Engine/Utils/InvariantConvert.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,6 @@ namespace FlagsmithEngine.Utils
/// </remarks>
internal static class InvariantConvert
{
/// <summary>
/// Converts the string representation of a number to its 32-bit signed integer equivalent
/// using culture-invariant formatting.
/// </summary>
/// <param name="value">A string containing a number to convert.</param>
/// <returns>A 32-bit signed integer equivalent to the number in value.</returns>
public static int ToInt32(string value) =>
Convert.ToInt32(value, CultureInfo.InvariantCulture);

/// <summary>
/// Converts the string representation of a number to its 64-bit signed integer equivalent
/// using culture-invariant formatting.
Expand Down
47 changes: 47 additions & 0 deletions Flagsmith.EngineTest/EngineTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,53 @@ public static IEnumerable<object[]> ExtractTestCaseFilenames()
return testCases;
}

[Theory]
[InlineData(95, Operator.GreaterThan, "90.5")]
[InlineData(95, Operator.NotEqual, "90.5")]
[InlineData(1, Operator.Equal, "not_an_int")]
[InlineData(1.5, Operator.LessThan, "not_a_number")]
public void TestGetEvaluationResult_ShouldNotMatchSegment_WhenConditionValueIsNotOfNumericTraitType(object traitValue, Operator conditionOperator, string conditionValue)
{
// Given
var context = new EvaluationContext<object, object>
{
Environment = new EnvironmentContext { Key = "test-env", Name = "Test Environment" },
Identity = new IdentityContext
{
Identifier = "user-123",
Traits = new Dictionary<string, object> { { "score", traitValue } },
},
Features = new Dictionary<string, FeatureContext<object>>(),
Segments = new Dictionary<string, SegmentContext<object, object>>
{
{
"1", new SegmentContext<object, object>
{
Key = "1",
Name = "high_scorers",
Rules = new[]
{
new SegmentRule
{
Type = TypeEnum.All,
Conditions = new[]
{
new Condition { Operator = conditionOperator, Property = "score", Value = conditionValue },
},
},
},
}
},
},
};

// When
var result = new Engine().GetEvaluationResult(context);

// Then
Assert.Empty(result.Segments);
}

[Fact]
public void TestGetEvaluationResult_ShouldNotMutateOriginalContextIdentity()
{
Expand Down
Loading