Skip to content

Commit

Permalink
[CSharp] Generate (more) presence checks in accessors.
Browse files Browse the repository at this point in the history
Previously, some accessors did not check that the acting version (being
decoded) had the field/group/varData being accessed. Therefore, it might
return garbage. But other accessors did check.

In this commit, I've attempted to make more of the accessors (hopefully
all) check the acting version before attempting to read data from the
underlying buffer.

This change should make the C# codecs closer to the Java decoders.
  • Loading branch information
ZachBray committed Jul 5, 2023
1 parent 83f32da commit 921daf1
Showing 1 changed file with 49 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,15 @@ private void generateGroupClassHeader(
groupName));
}

sb.append("\n")
.append(indent).append(INDENT).append("public void NotPresent()\n")
.append(indent).append(INDENT).append("{\n")
.append(indent).append(TWO_INDENT).append("_count = 0;\n")
.append(indent).append(TWO_INDENT).append("_index = 0;\n")
.append(indent).append(TWO_INDENT).append("_buffer = null;\n")
.append(indent).append(TWO_INDENT).append("_offset = 0;\n")
.append(indent).append(INDENT).append("}\n");

sb.append(String.format("\n" +
indent + INDENT + "public void WrapForDecode(%s parentMessage, DirectBuffer buffer, int actingVersion)\n" +
indent + INDENT + "{\n" +
Expand Down Expand Up @@ -407,15 +416,18 @@ private CharSequence generateGroupProperty(

generateSinceActingDeprecated(sb, indent, toUpperFirstChar(groupName), token);

final String groupField = "_" + toLowerFirstChar(groupName);

sb.append(String.format("\n" +
"%1$s" +
indent + "public %2$sGroup %3$s\n" +
indent + "{\n" +
indent + INDENT + "get\n" +
indent + INDENT + "{\n" +
generateGroupNotPresentCondition(token.version(), indent + INDENT + INDENT, groupField) +
indent + INDENT + INDENT + "_%4$s.WrapForDecode(_parentMessage, _buffer, _actingVersion);\n" +
generateAccessOrderListenerCall(accessOrderModel, indent + TWO_INDENT, token,
"_" + groupName + ".Count", "\"decode\"") +
groupField + ".Count", "\"decode\"") +
indent + INDENT + INDENT + "return _%4$s;\n" +
indent + INDENT + "}\n" +
indent + "}\n",
Expand Down Expand Up @@ -480,6 +492,7 @@ private CharSequence generateVarData(
sb.append(String.format(indent + "\n" +
indent + "public int %1$sLength()\n" +
indent + "{\n" +
generateArrayFieldNotPresentCondition(token.version(), indent, "0") +
accessOrderListenerCall +
indent + INDENT + "_buffer.CheckLimit(_parentMessage.Limit + %2$d);\n" +
indent + INDENT + "return (int)_buffer.%3$sGet%4$s(_parentMessage.Limit);\n" +
Expand Down Expand Up @@ -509,7 +522,7 @@ private CharSequence generateVarData(
indent + INDENT + "return bytesCopied;\n" +
indent + "}\n",
propertyName,
generateArrayFieldNotPresentCondition(token.version(), indent),
generateArrayFieldNotPresentCondition(token.version(), indent, "0"),
sizeOfLengthField,
lengthTypePrefix,
byteOrderStr));
Expand All @@ -518,6 +531,7 @@ private CharSequence generateVarData(
indent + "// Allocates and returns a new byte array\n" +
indent + "public byte[] Get%1$sBytes()\n" +
indent + "{\n" +
generateArrayFieldNotPresentCondition(token.version(), indent, "new byte[0]") +
accessOrderListenerCall +
indent + INDENT + "const int sizeOfLengthField = %2$d;\n" +
indent + INDENT + "int limit = _parentMessage.Limit;\n" +
Expand Down Expand Up @@ -561,6 +575,7 @@ private CharSequence generateVarData(
.append(String.format(
indent + "public string Get%1$s()\n" +
indent + "{\n" +
generateArrayFieldNotPresentCondition(token.version(), indent, "\"\"") +
accessOrderListenerCall +
indent + INDENT + "const int sizeOfLengthField = %2$d;\n" +
indent + INDENT + "int limit = _parentMessage.Limit;\n" +
Expand Down Expand Up @@ -948,16 +963,34 @@ private CharSequence generateFieldNotPresentCondition(
literal);
}

private CharSequence generateArrayFieldNotPresentCondition(final int sinceVersion, final String indent)
private CharSequence generateGroupNotPresentCondition(
final int sinceVersion,
final String indent,
final String groupInstanceField)
{
if (0 == sinceVersion)
{
return "";
}

return String.format(
indent + INDENT + INDENT + "if (_actingVersion < %d) return 0;\n\n",
sinceVersion);
return indent + "if (_actingVersion < " + sinceVersion + ")" +
indent + "{\n" +
indent + INDENT + groupInstanceField + ".NotPresent();\n" +
indent + INDENT + "return " + groupInstanceField + ";\n" +
indent + "}\n\n";
}

private CharSequence generateArrayFieldNotPresentCondition(
final int sinceVersion,
final String indent,
final String defaultValue)
{
if (0 == sinceVersion)
{
return "";
}

return indent + INDENT + "if (_actingVersion < " + sinceVersion + ") return " + defaultValue + ";\n\n";
}

private CharSequence generateBitSetNotPresentCondition(
Expand Down Expand Up @@ -1053,6 +1086,7 @@ private CharSequence generateArrayProperty(
indent + "{\n" +
indent + INDENT + "get\n" +
indent + INDENT + "{\n" +
generateArrayFieldNotPresentCondition(fieldToken.version(), indent + INDENT + INDENT, "new %2$s[0]") +
accessOrderListenerCallDoubleIndent +
indent + INDENT + INDENT + "return _buffer.AsReadOnlySpan<%2$s>(_offset + %4$s, %3$sLength);\n" +
indent + INDENT + "}\n" +
Expand All @@ -1069,6 +1103,7 @@ private CharSequence generateArrayProperty(
"%1$s" +
indent + "public Span<%2$s> %3$sAsSpan()\n" +
indent + "{\n" +
generateArrayFieldNotPresentCondition(fieldToken.version(), indent + INDENT + INDENT, "new %2$s[0]") +
accessOrderListenerCall +
indent + INDENT + "return _buffer.AsSpan<%2$s>(_offset + %4$s, %3$sLength);\n" +
indent + "}\n",
Expand All @@ -1087,7 +1122,10 @@ private CharSequence generateArrayProperty(
accessOrderListenerCall +
indent + INDENT + "return Get%1$s(new Span<byte>(dst, dstOffset, length));\n" +
indent + "}\n",
propName, fieldLength, generateArrayFieldNotPresentCondition(fieldToken.version(), indent), offset));
propName,
fieldLength,
generateArrayFieldNotPresentCondition(fieldToken.version(), indent, "0"),
offset));

sb.append(String.format("\n" +
indent + "public int Get%1$s(Span<byte> dst)\n" +
Expand All @@ -1102,7 +1140,10 @@ private CharSequence generateArrayProperty(
indent + INDENT + "_buffer.GetBytes(_offset + %4$d, dst);\n" +
indent + INDENT + "return length;\n" +
indent + "}\n",
propName, fieldLength, generateArrayFieldNotPresentCondition(fieldToken.version(), indent), offset));
propName,
fieldLength,
generateArrayFieldNotPresentCondition(fieldToken.version(), indent, "0"),
offset));

sb.append(String.format("\n" +
indent + "public void Set%1$s(byte[] src, int srcOffset)\n" +
Expand Down

0 comments on commit 921daf1

Please sign in to comment.