Skip to content

Commit

Permalink
Add field access validation. (#252)
Browse files Browse the repository at this point in the history
  • Loading branch information
MarkPflug authored Apr 23, 2024
1 parent a778815 commit 332c8dc
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 15 deletions.
5 changes: 5 additions & 0 deletions docs/Csv/Sylvan.Data.Csv.Releases.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Sylvan.Data.Csv Release Notes

_1.3.8_
- Fixes an issue with detecting MacOS-style ('\r') line ends in some scenarios.
- Field accessors now throw `InvalidOperationException` when called at inappropriate times.
This might be a behavior breaking change for code that relied on invalid access patterns.

_1.3.7_
- Adds `CsvDataWriterOptions.QuoteStrings` which now allows quoting empty, non-empty, or all strings. This is useful when string data might otherwise be interpreted as a number. This obsoletes QuoteEmptyString option.

Expand Down
45 changes: 45 additions & 0 deletions source/Sylvan.Data.Csv.Tests/CsvDataReaderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1911,6 +1911,51 @@ public void FinalCharInCellIsEscapeError()
Assert.Equal("\\\n", value0);
}

[Fact]
public void FieldAccessInvalidState()
{
var data = "a,b\n1,2\n";
var r = CsvDataReader.Create(new StringReader(data));

Assert.Throws<InvalidOperationException>(() => r.GetString(0));
Assert.True(r.Read());
Assert.Equal("1", r.GetString(0));
Assert.False(r.Read());
Assert.Throws<InvalidOperationException>(() => r.GetString(0));
}

[Fact]
public void FieldAccessInvalidStateAccessors()
{
var data = "a,b\n1,2\n";
var r = CsvDataReader.Create(new StringReader(data));

Assert.Throws<InvalidOperationException>(() => r.GetString(0));
Assert.Throws<InvalidOperationException>(() => r.GetInt16(0));
Assert.Throws<InvalidOperationException>(() => r.GetInt32(0));
Assert.Throws<InvalidOperationException>(() => r.GetInt64(0));
Assert.Throws<InvalidOperationException>(() => r.GetFloat(0));
Assert.Throws<InvalidOperationException>(() => r.GetDouble(0));
Assert.Throws<InvalidOperationException>(() => r.GetDateTime(0));
Assert.Throws<InvalidOperationException>(() => r.GetDateTimeOffset(0));
Assert.Throws<InvalidOperationException>(() => r.GetByte(0));
Assert.Throws<InvalidOperationException>(() => r.GetChar(0));
}

[Fact]
public void MacOSLineEndDetect()
{
var data = "a,b\r1,2\r"; // weird line ends
var opts = new CsvDataReaderOptions { Delimiter = ',' };
var r = CsvDataReader.Create(new StringReader(data), opts);
Assert.Equal(2, r.FieldCount);
Assert.True(r.Read());
Assert.Equal("1", r[0]);
Assert.Equal("2", r[1]);
Assert.False(r.Read());
}


[Theory]
// These test cases were copied from the Sep parser library. Thanks, Nietras.
[InlineData("a", true, "a")]
Expand Down
59 changes: 45 additions & 14 deletions source/Sylvan.Data.Csv/CsvDataReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@ enum State
Closed,
}

void ValidateState()
{
if (this.state != State.Open)
{
throw new InvalidOperationException();
}
}

enum ReadResult
{
False,
Expand Down Expand Up @@ -319,7 +327,7 @@ public void Initialize()
this.fieldCount = count;
for (int i = 0; i < count; i++)
{
var name = hasHeaders ? GetString(i) : null;
var name = hasHeaders ? GetStringRaw(i) : null;
var columnSchema = schema?.GetColumn(name, i);
columns[i] = new CsvColumn(name, i, columnSchema);

Expand Down Expand Up @@ -1027,6 +1035,7 @@ ReadResult ConsumeLineEnd(char[] buffer, ref int idx)
/// <inheritdoc/>
public override bool GetBoolean(int ordinal)
{
ValidateState();
// four cases:
// true and false both not null. Any other value raises error.
// true not null, false null. True string true, anything else false.
Expand Down Expand Up @@ -1088,6 +1097,7 @@ public override bool GetBoolean(int ordinal)
/// <inheritdoc/>
public override byte GetByte(int ordinal)
{
ValidateState();
#if SPAN
return byte.Parse(this.GetFieldSpan(ordinal), provider: culture);
#else
Expand All @@ -1098,6 +1108,7 @@ public override byte GetByte(int ordinal)
/// <inheritdoc/>
public override long GetBytes(int ordinal, long dataOffset, byte[]? buffer, int bufferOffset, int length)
{
ValidateState();
if (buffer == null)
{
return GetBinaryLength(ordinal);
Expand Down Expand Up @@ -1264,6 +1275,7 @@ static void FromBase64Chars(char[] chars, int charsOffset, int charsLen, byte[]
/// <inheritdoc/>
public override char GetChar(int ordinal)
{
ValidateState();
var s = GetField(ordinal);
if (s.length == 1)
{
Expand All @@ -1275,6 +1287,7 @@ public override char GetChar(int ordinal)
/// <inheritdoc/>
public override long GetChars(int ordinal, long dataOffset, char[]? buffer, int bufferOffset, int length)
{
ValidateState();
if (buffer == null)
{
return this.GetCharLength(ordinal);
Expand All @@ -1294,18 +1307,18 @@ public override long GetChars(int ordinal, long dataOffset, char[]? buffer, int
/// </summary>
public TimeSpan GetTimeSpan(int ordinal)
{
ValidateState();
var format = columns[ordinal].Format;
var style = TimeSpanStyles.None;
#if SPAN
var span = this.GetFieldSpan(ordinal);
if (format != null && TimeSpan.TryParseExact(span, format, culture, style, out var value))
if (format != null && TimeSpan.TryParseExact(span, format, culture, TimeSpanStyles.None, out var value))
{
return value;
}
return TimeSpan.Parse(span, culture);
#else
var str = this.GetString(ordinal);
if (format != null && TimeSpan.TryParseExact(str, format, culture, style, out var value))
if (format != null && TimeSpan.TryParseExact(str, format, culture, TimeSpanStyles.None, out var value))
{
return value;
}
Expand All @@ -1318,61 +1331,62 @@ public TimeSpan GetTimeSpan(int ordinal)
/// </summary>
public DateTimeOffset GetDateTimeOffset(int ordinal)
{
ValidateState();
var format = columns[ordinal].Format ?? this.dateTimeFormat;
var style = DateTimeStyles.RoundtripKind;
DateTimeOffset value;
#if SPAN
var span = this.GetFieldSpan(ordinal);

if (IsoDate.TryParse(span, out value))
return value;

if (format != null && DateTimeOffset.TryParseExact(span, format, culture, style, out value))
if (format != null && DateTimeOffset.TryParseExact(span, format, culture, DateTimeStyles.RoundtripKind, out value))
{
return value;
}
return DateTimeOffset.Parse(span, culture, style);
return DateTimeOffset.Parse(span, culture, DateTimeStyles.RoundtripKind);
#else
var str = this.GetString(ordinal);
if (format != null && DateTimeOffset.TryParseExact(str, format, culture, style, out value))
if (format != null && DateTimeOffset.TryParseExact(str, format, culture, DateTimeStyles.RoundtripKind, out value))
{
return value;
}
return DateTimeOffset.Parse(str, culture, style);
return DateTimeOffset.Parse(str, culture, DateTimeStyles.RoundtripKind);
#endif
}

/// <inheritdoc/>
public override DateTime GetDateTime(int ordinal)
{
var style = DateTimeStyles.RoundtripKind;
ValidateState();
DateTime value;
#if SPAN
var span = this.GetFieldSpan(ordinal);
if (IsoDate.TryParse(span, out value))
return value;

var format = columns[ordinal].Format ?? this.dateTimeFormat;
if (format != null && DateTime.TryParseExact(span, format, culture, style, out value))
if (format != null && DateTime.TryParseExact(span, format, culture, DateTimeStyles.RoundtripKind, out value))
{
return value;
}
return DateTime.Parse(span, culture, style);
return DateTime.Parse(span, culture, DateTimeStyles.RoundtripKind);
#else
var dateStr = this.GetString(ordinal);

var format = columns[ordinal].Format ?? this.dateTimeFormat ?? "O";
if (format != null && DateTime.TryParseExact(dateStr, format, culture, style, out value))
if (format != null && DateTime.TryParseExact(dateStr, format, culture, DateTimeStyles.RoundtripKind, out value))
{
return value;
}
return DateTime.Parse(dateStr, culture, style);
return DateTime.Parse(dateStr, culture, DateTimeStyles.RoundtripKind);
#endif
}

/// <inheritdoc/>
public override decimal GetDecimal(int ordinal)
{
ValidateState();
#if SPAN
var field = this.GetField(ordinal);
return
Expand All @@ -1386,6 +1400,7 @@ public override decimal GetDecimal(int ordinal)
/// <inheritdoc/>
public override double GetDouble(int ordinal)
{
ValidateState();
#if SPAN
var field = this.GetField(ordinal);
return
Expand Down Expand Up @@ -1418,6 +1433,7 @@ public override Type GetFieldType(int ordinal)
/// <inheritdoc/>
public override float GetFloat(int ordinal)
{
ValidateState();
#if SPAN
var field = this.GetField(ordinal);
return
Expand All @@ -1431,6 +1447,7 @@ public override float GetFloat(int ordinal)
/// <inheritdoc/>
public override Guid GetGuid(int ordinal)
{
ValidateState();
#if SPAN
return Guid.Parse(this.GetFieldSpan(ordinal));
#else
Expand All @@ -1441,6 +1458,7 @@ public override Guid GetGuid(int ordinal)
/// <inheritdoc/>
public override short GetInt16(int ordinal)
{
ValidateState();
#if SPAN
var field = this.GetField(ordinal);
return
Expand All @@ -1454,6 +1472,7 @@ public override short GetInt16(int ordinal)
/// <inheritdoc/>
public override int GetInt32(int ordinal)
{
ValidateState();
#if SPAN
var field = this.GetField(ordinal);
return
Expand All @@ -1467,6 +1486,7 @@ public override int GetInt32(int ordinal)
/// <inheritdoc/>
public override long GetInt64(int ordinal)
{
ValidateState();
#if SPAN
var field = this.GetField(ordinal);
return
Expand Down Expand Up @@ -1503,6 +1523,12 @@ public override int GetOrdinal(string name)
/// <inheritdoc/>
public override string GetString(int ordinal)
{
ValidateState();
return GetStringRaw(ordinal);
}

string GetStringRaw(int ordinal)
{
if ((uint)ordinal < (uint)curFieldCount)
{
var s = GetFieldUnsafe(ordinal);
Expand Down Expand Up @@ -1744,6 +1770,7 @@ CharSpan PrepareInvalidField(int offset, int len)
/// <inheritdoc/>
public override object GetValue(int ordinal)
{
ValidateState();
var max = Math.Max(this.fieldCount, this.curFieldCount);

if (ordinal > max)
Expand Down Expand Up @@ -1814,6 +1841,7 @@ int GetCharLength(int ordinal)
/// <inheritdoc/>
public override int GetValues(object[] values)
{
ValidateState();
var count = Math.Min(this.fieldCount, values.Length);
for (int i = 0; i < count; i++)
{
Expand All @@ -1825,6 +1853,7 @@ public override int GetValues(object[] values)
/// <inheritdoc/>
public override bool IsDBNull(int ordinal)
{
ValidateState();
if (ordinal < 0) throw new ArgumentOutOfRangeException(nameof(ordinal));

if ((uint)ordinal < this.fieldCount)
Expand Down Expand Up @@ -2012,6 +2041,7 @@ public override object? this[string property]
/// <returns>A span containing the characters of the field.</returns>
public ReadOnlySpan<char> GetFieldSpan(int ordinal)
{
ValidateState();
var s = GetField(ordinal);
return s.ToSpan();
}
Expand Down Expand Up @@ -2075,6 +2105,7 @@ static IFieldAccessor<T> GetAccessor()
/// <inheritdoc/>
public override T GetFieldValue<T>(int ordinal)
{
ValidateState();
var acc = Accessor<T>.Instance;
return acc.GetValue(this, ordinal);
}
Expand Down
2 changes: 1 addition & 1 deletion source/Sylvan.Data.Csv/Sylvan.Data.Csv.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<TargetFrameworks>net6.0;netstandard2.1;netstandard2.0</TargetFrameworks>
<VersionPrefix>1.3.7</VersionPrefix>
<VersionPrefix>1.3.8</VersionPrefix>
<Description>A .NET library for reading and writing delimited CSV data.</Description>
<PackageTags>csv;delimited;data;datareader;datawriter;simd</PackageTags>
<Nullable>enable</Nullable>
Expand Down

0 comments on commit 332c8dc

Please sign in to comment.