Skip to content

Commit 0c0fea0

Browse files
haackedhaacked
authored
fix: use invariant culture when stringifying property values for matching (#270)
* Use invariant culture when stringifying property values Property values were stringified with the current culture during local evaluation, so a numeric property like 3.14 rendered as "3,14" under comma-decimal locales (e.g. de-DE). That made exact, icontains, regex, and string comparisons diverge from the PostHog flags service, which always stringifies numbers with an invariant format. Stringify with Convert.ToString(value, CultureInfo.InvariantCulture) everywhere a property value is compared as a string. Generated-By: PostHog Code Task-Id: bb15888a-04c4-4dc0-916e-793a0540ece3 * Address PR review feedback Generated-By: PostHog Code Task-Id: 1d912da7-63e6-4f96-a192-955cb104f698 * Extract invariant stringification into a shared helper ToInvariantString centralizes the invariant-culture stringification policy in PropertyFilterValue and applies it to IsPrefixOf and IsSuffixOf, so starts_with and ends_with operators also match locale-independently. TestCulture.Use scopes the current culture in tests, replacing repeated try/finally blocks. Generated-By: PostHog Code Task-Id: 1d912da7-63e6-4f96-a192-955cb104f698
1 parent 31825eb commit 0c0fea0

5 files changed

Lines changed: 119 additions & 9 deletions

File tree

‎.changeset/invariant-culture-stringification.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"PostHog": patch
3+
---
4+
5+
Use the invariant culture when stringifying property values for feature flag local evaluation. Numeric property values such as `3.14` now stringify as `"3.14"` regardless of the host locale, so `exact`, `icontains`, `starts_with`, `ends_with`, and regex matching behave the same way the PostHog flags service does on machines using comma-decimal cultures.

‎src/PostHog/Json/PropertyFilterValue.cs‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ public bool IsRegexMatch(object? input)
125125
return false;
126126
}
127127

128-
return regex.IsMatch(NotNull(input.ToString()));
128+
return regex.IsMatch(NotNull(ToInvariantString(input)));
129129
}
130130

131131
/// <summary>
@@ -135,7 +135,7 @@ public bool IsRegexMatch(object? input)
135135
/// <param name="stringComparison">The type of comparison if these are strings.</param>
136136
/// <returns><c>true</c> if this instance contains the other.</returns>
137137
public bool IsContainedBy(object? other, StringComparison stringComparison) =>
138-
other?.ToString() is { } comparandString
138+
ToInvariantString(other) is { } comparandString
139139
&& StringValue is not null
140140
&& comparandString.Contains(StringValue, stringComparison);
141141

@@ -146,7 +146,7 @@ public bool IsContainedBy(object? other, StringComparison stringComparison) =>
146146
/// <param name="stringComparison">The type of comparison if these are strings.</param>
147147
/// <returns><c>true</c> if the other value starts with this instance.</returns>
148148
public bool IsPrefixOf(object? other, StringComparison stringComparison) =>
149-
other?.ToString() is { } comparandString
149+
ToInvariantString(other) is { } comparandString
150150
&& StringValue is not null
151151
&& comparandString.StartsWith(StringValue, stringComparison);
152152

@@ -157,7 +157,7 @@ public bool IsPrefixOf(object? other, StringComparison stringComparison) =>
157157
/// <param name="stringComparison">The type of comparison if these are strings.</param>
158158
/// <returns><c>true</c> if the other value ends with this instance.</returns>
159159
public bool IsSuffixOf(object? other, StringComparison stringComparison) =>
160-
other?.ToString() is { } comparandString
160+
ToInvariantString(other) is { } comparandString
161161
&& StringValue is not null
162162
&& comparandString.EndsWith(StringValue, stringComparison);
163163

@@ -172,7 +172,7 @@ public bool IsExactMatch(object? overrideValue)
172172
return this switch
173173
{
174174
{ ListOfStrings: { } listOfStrings } => IsExactListMatch(listOfStrings, _numericListValues, overrideValue),
175-
{ StringValue: { } stringValue } => stringValue.Equals(overrideValue?.ToString(), StringComparison.OrdinalIgnoreCase),
175+
{ StringValue: { } stringValue } => stringValue.Equals(ToInvariantString(overrideValue), StringComparison.OrdinalIgnoreCase),
176176
{ BooleanValue: { } booleanValue } => overrideValue switch
177177
{
178178
bool boolOverride => booleanValue == boolOverride,
@@ -193,7 +193,7 @@ static bool IsExactListMatch(
193193
return false;
194194
}
195195

196-
var stringValue = Convert.ToString(overrideValue, CultureInfo.InvariantCulture);
196+
var stringValue = ToInvariantString(overrideValue);
197197
if (stringValue is not null && values.Contains(stringValue, StringComparer.OrdinalIgnoreCase))
198198
{
199199
return true;
@@ -221,6 +221,11 @@ or TypeCode.SByte or TypeCode.UInt16 or TypeCode.UInt32 or TypeCode.UInt64
221221
};
222222
}
223223

224+
// Override values must stringify with the invariant culture ("3.14", never "3,14") to match how the
225+
// PostHog flags service stringifies values. Null stays null so null overrides never match string filters.
226+
static string? ToInvariantString(object? value) =>
227+
value is null ? null : Convert.ToString(value, CultureInfo.InvariantCulture);
228+
224229
static bool TryParseDoubleWithoutUnderflow(string value, out double number) =>
225230
double.TryParse(value, NumberStyles.Float, CultureInfo.InvariantCulture, out number)
226231
&& (number != 0 || RepresentsZero(value));
@@ -277,7 +282,7 @@ public int CompareTo(object? overrideValue)
277282
{
278283
_ when TryCompareNumbers(overrideValue, out var result) => result.Value,
279284
_ when BooleanValue.HasValue => CompareBooleanValue(overrideValue),
280-
_ => string.Compare(StringValue, overrideValue.ToString(), StringComparison.OrdinalIgnoreCase)
285+
_ => string.Compare(StringValue, ToInvariantString(overrideValue), StringComparison.OrdinalIgnoreCase)
281286
};
282287
}
283288

@@ -311,7 +316,7 @@ int CompareBooleanValue(object overrideValue)
311316
{
312317
bool boolOverride => BooleanValue.Value.CompareTo(boolOverride),
313318
string stringOverride when bool.TryParse(stringOverride, out var boolValue) => BooleanValue.Value.CompareTo(boolValue),
314-
_ => string.Compare(BooleanValue.Value.ToString(), overrideValue.ToString(), StringComparison.OrdinalIgnoreCase)
319+
_ => string.Compare(BooleanValue.Value.ToString(), ToInvariantString(overrideValue), StringComparison.OrdinalIgnoreCase)
315320
};
316321
}
317322

@@ -395,7 +400,7 @@ or DateOnly
395400

396401
static SemanticVersion ParseOverrideSemver(object? overrideValue)
397402
{
398-
var overrideVersionString = overrideValue?.ToString();
403+
var overrideVersionString = ToInvariantString(overrideValue);
399404
if (!SemanticVersion.TryParse(overrideVersionString, out var version))
400405
{
401406
throw new InconclusiveMatchException($"Cannot parse override value '{overrideVersionString}' as a semantic version");

‎tests/TestLibrary/TestCulture.cs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
using System.Globalization;
2+
3+
namespace UnitTests.Library;
4+
5+
/// <summary>
6+
/// Provides a scoped override of the current culture for tests of culture-sensitive behavior.
7+
/// </summary>
8+
public static class TestCulture
9+
{
10+
/// <summary>
11+
/// Sets <see cref="CultureInfo.CurrentCulture"/> to the specified culture until the returned scope is
12+
/// disposed. For example, "de-DE" formats 3.14 as "3,14", which exercises culture-sensitive formatting.
13+
/// </summary>
14+
/// <param name="cultureName">The name of the culture, e.g. "de-DE".</param>
15+
/// <returns>A scope that restores the original culture when disposed.</returns>
16+
public static IDisposable Use(string cultureName)
17+
{
18+
var originalCulture = CultureInfo.CurrentCulture;
19+
CultureInfo.CurrentCulture = new CultureInfo(cultureName);
20+
return Disposable.Create(() => CultureInfo.CurrentCulture = originalCulture);
21+
}
22+
}

‎tests/UnitTests/Features/LocalEvaluatorTests.cs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using PostHog.Api;
77
using PostHog.Features;
88
using PostHog.Json;
9+
using UnitTests.Library;
910

1011
namespace LocalEvaluatorTests;
1112

@@ -521,6 +522,41 @@ public void ThrowsInconclusiveMatchExceptionWhenPropertyKeyMissingForStartsWithA
521522
personProperties: properties));
522523
}
523524

525+
[Theory]
526+
[InlineData(ComparisonOperator.ContainsIgnoreCase)]
527+
[InlineData(ComparisonOperator.Exact)]
528+
[InlineData(ComparisonOperator.StartsWith)]
529+
[InlineData(ComparisonOperator.EndsWith)]
530+
public void MatchesNumericPropertyValueRegardlessOfCurrentCulture(ComparisonOperator comparison)
531+
{
532+
using var _ = TestCulture.Use("de-DE");
533+
var flags = CreateFlags(
534+
key: "pi",
535+
properties:
536+
[
537+
new PropertyFilter
538+
{
539+
Type = FilterType.Person,
540+
Key = "pi",
541+
Value = new PropertyFilterValue("3.14"),
542+
Operator = comparison
543+
}
544+
]
545+
);
546+
var properties = new Dictionary<string, object?>
547+
{
548+
["pi"] = 3.14
549+
};
550+
var localEvaluator = new LocalEvaluator(flags);
551+
552+
var result = localEvaluator.EvaluateFeatureFlag(
553+
key: "pi",
554+
distinctId: "distinct-id",
555+
personProperties: properties);
556+
557+
Assert.True(result.Value);
558+
}
559+
524560
[Theory]
525561
[InlineData(22, ComparisonOperator.GreaterThan, "\"21\"", true)]
526562
[InlineData(22, ComparisonOperator.GreaterThanOrEquals, "\"21\"", true)]

‎tests/UnitTests/Json/PropertyFilterValueTests.cs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System.Text.Json;
22
using PostHog.Json;
3+
using UnitTests.Library;
34

45
namespace PropertyFilterValueTests;
56

@@ -67,6 +68,47 @@ public void NumericArrayDoesNotThrowForLargeSingleOverride()
6768
Assert.NotNull(filterPropertyValue);
6869
Assert.False(filterPropertyValue.IsExactMatch(float.MaxValue));
6970
}
71+
72+
[Theory]
73+
[InlineData(3.14, "\"3.14\"", true)]
74+
[InlineData(3.14, "\"3,14\"", false)]
75+
[InlineData(1.618, "\"3.14\"", false)]
76+
[InlineData(3.14, """["1", "3.14", "42"]""", true)]
77+
public void StringifiesNumbersWithInvariantCulture(object overrideValue, string jsonValue, bool expected)
78+
{
79+
using var _ = TestCulture.Use("de-DE");
80+
var filterPropertyValue = PropertyFilterValue.Create(JsonDocument.Parse(jsonValue).RootElement);
81+
82+
Assert.NotNull(filterPropertyValue);
83+
Assert.Equal(expected, filterPropertyValue.IsExactMatch(overrideValue));
84+
}
85+
86+
[Fact]
87+
public void StringifiesDecimalsWithInvariantCulture()
88+
{
89+
using var _ = TestCulture.Use("de-DE");
90+
var filterPropertyValue = PropertyFilterValue.Create(JsonDocument.Parse("\"3.14\"").RootElement);
91+
92+
Assert.NotNull(filterPropertyValue);
93+
Assert.True(filterPropertyValue.IsExactMatch(3.14m));
94+
}
95+
}
96+
97+
public class TheIsContainedByMethod
98+
{
99+
[Theory]
100+
[InlineData(3.14, "\"3.14\"", true)]
101+
[InlineData(3.14, "\".14\"", true)]
102+
[InlineData(3.14, "\"3,14\"", false)]
103+
[InlineData(1.618, "\"3.14\"", false)]
104+
public void StringifiesNumbersWithInvariantCulture(object overrideValue, string jsonValue, bool expected)
105+
{
106+
using var _ = TestCulture.Use("de-DE");
107+
var filterPropertyValue = PropertyFilterValue.Create(JsonDocument.Parse(jsonValue).RootElement);
108+
109+
Assert.NotNull(filterPropertyValue);
110+
Assert.Equal(expected, filterPropertyValue.IsContainedBy(overrideValue, StringComparison.OrdinalIgnoreCase));
111+
}
70112
}
71113

72114
public class TheEqualsMethod

0 commit comments

Comments
 (0)