Skip to content

Commit 9f742a6

Browse files
xPawxPaw
committed
Fix KVObject.ToString() default culture
1 parent 1c6e81a commit 9f742a6

3 files changed

Lines changed: 252 additions & 19 deletions

File tree

‎ValveKeyValue/ValveKeyValue.Test/CultureInvariantFormattingTestCase.cs‎

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
using System.Diagnostics.CodeAnalysis;
2+
3+
namespace ValveKeyValue.Test
4+
{
5+
// de-DE and fa-IR are chosen because together they differ from the invariant culture in every
6+
// way that matters: comma/Arabic decimal separators, a period group separator, and a non-ASCII
7+
// negative sign (U+2212) on negative numbers. Every scalar KVValueType is covered, including
8+
// already-safe ones, so the guarantee is locked down for all types and both directions.
9+
class CultureInvariantFormattingTestCase
10+
{
11+
static IEnumerable<TestCaseData> ScalarCases()
12+
{
13+
yield return Named(new KVObject("plain string"), "plain string", "string");
14+
yield return Named(new KVObject(true), "1", "bool true");
15+
yield return Named(new KVObject(false), "0", "bool false");
16+
yield return Named(new KVObject((short)-12345), "-12345", "short negative");
17+
yield return Named(new KVObject((ushort)54321), "54321", "ushort");
18+
yield return Named(new KVObject(-1234567), "-1234567", "int negative");
19+
yield return Named(new KVObject(4000000000u), "4000000000", "uint");
20+
yield return Named(new KVObject(-9876543210L), "-9876543210", "long negative");
21+
yield return Named(new KVObject(18000000000000000000UL), "18000000000000000000", "ulong");
22+
yield return Named(new KVObject(new IntPtr(-1234)), "-1234", "pointer negative");
23+
yield return Named(new KVObject(0.8f), "0.8", "float positive fractional");
24+
yield return Named(new KVObject(-2.75f), "-2.75", "float negative fractional");
25+
yield return Named(new KVObject(1234.5d), "1234.5", "double positive fractional");
26+
yield return Named(new KVObject(-0.25d), "-0.25", "double negative fractional");
27+
}
28+
29+
// bool/string have format quirks (true/false, quoting) unrelated to culture.
30+
static IEnumerable<TestCaseData> NumericCases()
31+
{
32+
foreach (var data in ScalarCases())
33+
{
34+
var value = (KVObject)data.Arguments[0]!;
35+
if (value.ValueType is not (KVValueType.String or KVValueType.Boolean))
36+
{
37+
yield return data;
38+
}
39+
}
40+
}
41+
42+
static TestCaseData Named(KVObject value, string expected, string description)
43+
=> new TestCaseData(value, expected).SetName($"{{m}} - {description}");
44+
45+
static string Serialize(KVSerializationFormat format, KVObject value)
46+
{
47+
var root = KVObject.Collection();
48+
root.Add("v", value);
49+
50+
using var ms = new MemoryStream();
51+
KVSerializer.Create(format).Serialize(ms, root, "root");
52+
ms.Seek(0, SeekOrigin.Begin);
53+
using var reader = new StreamReader(ms);
54+
return reader.ReadToEnd();
55+
}
56+
57+
[TestCaseSource(nameof(ScalarCases))]
58+
[SetCulture("de-DE")]
59+
public void ToStringNullProviderIsInvariant_DE(KVObject value, string expected)
60+
=> Assert.That(value.ToString(null), Is.EqualTo(expected));
61+
62+
[TestCaseSource(nameof(ScalarCases))]
63+
[SetCulture("fa-IR")]
64+
public void ToStringNullProviderIsInvariant_FA(KVObject value, string expected)
65+
=> Assert.That(value.ToString(null), Is.EqualTo(expected));
66+
67+
[TestCaseSource(nameof(ScalarCases))]
68+
[SetCulture("de-DE")]
69+
[SuppressMessage("Globalization", "CA1305:Specify IFormatProvider", Justification = "Deliberately exercising the parameterless overload to assert it is invariant.")]
70+
public void ParameterlessToStringIsInvariant_DE(KVObject value, string expected)
71+
=> Assert.That(value.ToString(), Is.EqualTo(expected));
72+
73+
[TestCaseSource(nameof(ScalarCases))]
74+
[SetCulture("fa-IR")]
75+
[SuppressMessage("Globalization", "CA1305:Specify IFormatProvider", Justification = "Deliberately exercising the parameterless overload to assert it is invariant.")]
76+
public void ParameterlessToStringIsInvariant_FA(KVObject value, string expected)
77+
=> Assert.That(value.ToString(), Is.EqualTo(expected));
78+
79+
[TestCaseSource(nameof(ScalarCases))]
80+
[SetCulture("de-DE")]
81+
public void ExplicitStringCastIsInvariant_DE(KVObject value, string expected)
82+
=> Assert.That((string)value, Is.EqualTo(expected));
83+
84+
[TestCaseSource(nameof(ScalarCases))]
85+
[SetCulture("fa-IR")]
86+
public void ExplicitStringCastIsInvariant_FA(KVObject value, string expected)
87+
=> Assert.That((string)value, Is.EqualTo(expected));
88+
89+
[TestCaseSource(nameof(NumericCases))]
90+
[SetCulture("de-DE")]
91+
public void ToTypeStringIsInvariant_DE(KVObject value, string expected)
92+
=> Assert.That(value.ToType(typeof(string), null), Is.EqualTo(expected));
93+
94+
[TestCaseSource(nameof(NumericCases))]
95+
[SetCulture("fa-IR")]
96+
public void ToTypeStringIsInvariant_FA(KVObject value, string expected)
97+
=> Assert.That(value.ToType(typeof(string), null), Is.EqualTo(expected));
98+
99+
[TestCaseSource(nameof(NumericCases))]
100+
[SetCulture("de-DE")]
101+
public void KeyValues1TextSerializationIsInvariant_DE(KVObject value, string expected)
102+
=> Assert.That(Serialize(KVSerializationFormat.KeyValues1Text, value), Does.Contain(expected));
103+
104+
[TestCaseSource(nameof(NumericCases))]
105+
[SetCulture("fa-IR")]
106+
public void KeyValues1TextSerializationIsInvariant_FA(KVObject value, string expected)
107+
=> Assert.That(Serialize(KVSerializationFormat.KeyValues1Text, value), Does.Contain(expected));
108+
109+
[TestCaseSource(nameof(NumericCases))]
110+
[SetCulture("de-DE")]
111+
public void KeyValues3TextSerializationIsInvariant_DE(KVObject value, string expected)
112+
=> Assert.That(Serialize(KVSerializationFormat.KeyValues3Text, value), Does.Contain(expected));
113+
114+
[TestCaseSource(nameof(NumericCases))]
115+
[SetCulture("fa-IR")]
116+
public void KeyValues3TextSerializationIsInvariant_FA(KVObject value, string expected)
117+
=> Assert.That(Serialize(KVSerializationFormat.KeyValues3Text, value), Does.Contain(expected));
118+
119+
// The array-index key path already formats invariantly; the dictionary key path did not.
120+
[Test]
121+
[SetCulture("de-DE")]
122+
public void DictionaryFloatKeysSerializeInvariant_DE()
123+
{
124+
var data = new Dictionary<float, string> { [0.8f] = "value" };
125+
126+
using var ms = new MemoryStream();
127+
KVSerializer.Create(KVSerializationFormat.KeyValues1Text).Serialize(ms, data, "root");
128+
ms.Seek(0, SeekOrigin.Begin);
129+
using var reader = new StreamReader(ms);
130+
var text = reader.ReadToEnd();
131+
132+
Assert.That(text, Does.Contain("\"0.8\""));
133+
}
134+
135+
[Test]
136+
[SetCulture("fa-IR")]
137+
public void DictionaryNegativeIntKeysSerializeInvariant_FA()
138+
{
139+
var data = new Dictionary<int, string> { [-1234] = "value" };
140+
141+
using var ms = new MemoryStream();
142+
KVSerializer.Create(KVSerializationFormat.KeyValues1Text).Serialize(ms, data, "root");
143+
ms.Seek(0, SeekOrigin.Begin);
144+
using var reader = new StreamReader(ms);
145+
var text = reader.ReadToEnd();
146+
147+
Assert.That(text, Does.Contain("\"-1234\""));
148+
}
149+
150+
static IEnumerable<TestCaseData> StringToFractionalCases()
151+
{
152+
yield return new TestCaseData(new KVObject("1.5"), 1.5d).SetName("{m} - 1.5");
153+
yield return new TestCaseData(new KVObject("-2.75"), -2.75d).SetName("{m} - -2.75");
154+
yield return new TestCaseData(new KVObject("1234.5"), 1234.5d).SetName("{m} - 1234.5");
155+
yield return new TestCaseData(new KVObject("-0.25"), -0.25d).SetName("{m} - -0.25");
156+
}
157+
158+
[TestCaseSource(nameof(StringToFractionalCases))]
159+
[SetCulture("de-DE")]
160+
public void StringToDoubleIsInvariant_DE(KVObject value, double expected)
161+
=> Assert.That(value.ToDouble(null), Is.EqualTo(expected));
162+
163+
[TestCaseSource(nameof(StringToFractionalCases))]
164+
[SetCulture("fa-IR")]
165+
public void StringToDoubleIsInvariant_FA(KVObject value, double expected)
166+
=> Assert.That(value.ToDouble(null), Is.EqualTo(expected));
167+
168+
[TestCaseSource(nameof(StringToFractionalCases))]
169+
[SetCulture("fa-IR")]
170+
public void StringToSingleIsInvariant_FA(KVObject value, double expected)
171+
=> Assert.That(value.ToSingle(null), Is.EqualTo((float)expected));
172+
173+
[TestCaseSource(nameof(StringToFractionalCases))]
174+
[SetCulture("fa-IR")]
175+
public void StringToDecimalIsInvariant_FA(KVObject value, double expected)
176+
=> Assert.That(value.ToDecimal(null), Is.EqualTo((decimal)expected));
177+
178+
[Test]
179+
[SetCulture("fa-IR")]
180+
public void StringToInt64IsInvariant_FA()
181+
=> Assert.That(new KVObject("-9876543210").ToInt64(null), Is.EqualTo(-9876543210L));
182+
183+
[Test]
184+
[SetCulture("fa-IR")]
185+
public void FloatCastFromStringIsInvariant_FA()
186+
=> Assert.That((float)new KVObject("-2.75"), Is.EqualTo(-2.75f));
187+
188+
[Test]
189+
[SetCulture("fa-IR")]
190+
public void IntCastFromStringIsInvariant_FA()
191+
=> Assert.That((int)new KVObject("-1234567"), Is.EqualTo(-1234567));
192+
193+
[TestCase(KVSerializationFormat.KeyValues1Text)]
194+
[TestCase(KVSerializationFormat.KeyValues3Text)]
195+
[SetCulture("de-DE")]
196+
public void TextRoundTripPreservesNumbers_DE(KVSerializationFormat format) => AssertNumberRoundTrip(format);
197+
198+
[TestCase(KVSerializationFormat.KeyValues1Text)]
199+
[TestCase(KVSerializationFormat.KeyValues3Text)]
200+
[SetCulture("fa-IR")]
201+
public void TextRoundTripPreservesNumbers_FA(KVSerializationFormat format) => AssertNumberRoundTrip(format);
202+
203+
static void AssertNumberRoundTrip(KVSerializationFormat format)
204+
{
205+
var root = KVObject.Collection();
206+
root.Add("floatValue", new KVObject(0.8f));
207+
root.Add("doubleValue", new KVObject(-2.75d));
208+
root.Add("intValue", new KVObject(-1234567));
209+
210+
var kv = KVSerializer.Create(format);
211+
212+
using var ms = new MemoryStream();
213+
kv.Serialize(ms, root, "root");
214+
ms.Seek(0, SeekOrigin.Begin);
215+
var result = kv.Deserialize(ms);
216+
217+
using (Assert.EnterMultipleScope())
218+
{
219+
Assert.That(result.Root["floatValue"].ToSingle(null), Is.EqualTo(0.8f));
220+
Assert.That(result.Root["doubleValue"].ToDouble(null), Is.EqualTo(-2.75d));
221+
Assert.That(result.Root["intValue"].ToInt32(null), Is.EqualTo(-1234567));
222+
}
223+
}
224+
}
225+
}

‎ValveKeyValue/ValveKeyValue/KVObject_IConvertible.cs‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -188,24 +188,32 @@ KVValueType.Collection or KVValueType.Array or KVValueType.BinaryBlob
188188
};
189189

190190
/// <inheritdoc cref="IConvertible.ToString"/>
191-
public string ToString(IFormatProvider? provider) => ValueType switch
191+
public string ToString(IFormatProvider? provider)
192192
{
193-
KVValueType.String => (string)_ref!,
194-
KVValueType.Boolean => _scalar != 0 ? "1" : "0",
195-
KVValueType.Null => "null",
196-
KVValueType.FloatingPoint => BitConverter.Int32BitsToSingle((int)_scalar).ToString(provider),
197-
KVValueType.FloatingPoint64 => BitConverter.Int64BitsToDouble(_scalar).ToString(provider),
198-
KVValueType.Int32 or KVValueType.Pointer => ((int)_scalar).ToString(provider),
199-
KVValueType.Int64 => _scalar.ToString(provider),
200-
KVValueType.UInt64 => ((ulong)_scalar).ToString(provider),
201-
KVValueType.UInt32 => ((uint)_scalar).ToString(provider),
202-
KVValueType.Int16 => ((short)_scalar).ToString(provider),
203-
KVValueType.UInt16 => ((ushort)_scalar).ToString(provider),
204-
KVValueType.BinaryBlob => FormatBlob(),
205-
KVValueType.Collection => "[Collection]",
206-
KVValueType.Array => "[Array]",
207-
_ => string.Empty,
208-
};
193+
// Default to invariant culture so callers passing null (the (string) cast operator and
194+
// the text serializers) never emit locale-specific separators or digits. Mirrors the
195+
// parse-side fallback in ConvertFromString.
196+
provider ??= CultureInfo.InvariantCulture;
197+
198+
return ValueType switch
199+
{
200+
KVValueType.String => (string)_ref!,
201+
KVValueType.Boolean => _scalar != 0 ? "1" : "0",
202+
KVValueType.Null => "null",
203+
KVValueType.FloatingPoint => BitConverter.Int32BitsToSingle((int)_scalar).ToString(provider),
204+
KVValueType.FloatingPoint64 => BitConverter.Int64BitsToDouble(_scalar).ToString(provider),
205+
KVValueType.Int32 or KVValueType.Pointer => ((int)_scalar).ToString(provider),
206+
KVValueType.Int64 => _scalar.ToString(provider),
207+
KVValueType.UInt64 => ((ulong)_scalar).ToString(provider),
208+
KVValueType.UInt32 => ((uint)_scalar).ToString(provider),
209+
KVValueType.Int16 => ((short)_scalar).ToString(provider),
210+
KVValueType.UInt16 => ((ushort)_scalar).ToString(provider),
211+
KVValueType.BinaryBlob => FormatBlob(),
212+
KVValueType.Collection => "[Collection]",
213+
KVValueType.Array => "[Array]",
214+
_ => string.Empty,
215+
};
216+
}
209217

210218
/// <inheritdoc cref="IConvertible.ToUInt16"/>
211219
public ushort ToUInt16(IFormatProvider? provider) => checked((ushort)ToUInt64(provider));
@@ -239,7 +247,7 @@ KVValueType.Int32 or KVValueType.Pointer or KVValueType.Int16
239247

240248
/// <inheritdoc cref="IConvertible.ToType"/>
241249
public object ToType(Type conversionType, IFormatProvider? provider)
242-
=> Convert.ChangeType(GetBoxedValue(), conversionType, provider)!;
250+
=> Convert.ChangeType(GetBoxedValue(), conversionType, provider ?? CultureInfo.InvariantCulture)!;
243251

244252
#endregion
245253

‎ValveKeyValue/ValveKeyValue/ObjectCopier.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ static KVObject ConvertObjectToValue(
105105
var entry = enumerator.Entry;
106106

107107
var childObjectValue = ConvertObjectToValue(entry.Value!.GetType(), entry.Value, reflector, visitedObjects);
108-
childItems.Add(new KeyValuePair<string, KVObject>(entry.Key.ToString()!, childObjectValue));
108+
childItems.Add(new KeyValuePair<string, KVObject>(Convert.ToString(entry.Key, CultureInfo.InvariantCulture)!, childObjectValue));
109109
}
110110
}
111111
else if (objectType == typeof(byte[]))

0 commit comments

Comments
 (0)