From 2c28d716536ae394d95a2332a577e5f4f2b16fc5 Mon Sep 17 00:00:00 2001 From: jpmcmu Date: Wed, 9 Aug 2023 15:14:51 -0400 Subject: [PATCH] DFSClient: BinaryRecordWriter NPE with null UTF8 strings - Fixed BinaryRecordWriter pathways to check for null elements - Added a new null element test Signed-off-by: James McMullan James.McMullan@lexisnexis.com --- .../dfs/client/BinaryRecordWriter.java | 61 ++++++++++------ .../dfs/client/HPCCRecordAccessor.java | 18 +++-- .../dfs/client/DFSReadWriteTest.java | 69 ++++++++++++++++++- 3 files changed, 117 insertions(+), 31 deletions(-) diff --git a/dfsclient/src/main/java/org/hpccsystems/dfs/client/BinaryRecordWriter.java b/dfsclient/src/main/java/org/hpccsystems/dfs/client/BinaryRecordWriter.java index 8855c622a..00e362399 100644 --- a/dfsclient/src/main/java/org/hpccsystems/dfs/client/BinaryRecordWriter.java +++ b/dfsclient/src/main/java/org/hpccsystems/dfs/client/BinaryRecordWriter.java @@ -652,6 +652,18 @@ private long calculateFieldSize(FieldDef fd, IRecordAccessor recordAccessor, Obj case SET: case DATASET: { + long dataLen = BinaryRecordWriter.DataLenFieldSize; + boolean isSet = fd.getDef(0).getFieldType() != FieldType.RECORD; + if (isSet) + { + dataLen++; + } + + if (fieldValue == null) + { + return dataLen; + } + List listValue = null; if (fieldValue instanceof List) { @@ -662,13 +674,6 @@ private long calculateFieldSize(FieldDef fd, IRecordAccessor recordAccessor, Obj throw new Exception("Error writing list. Expected List, got: " + fieldValue.getClass().getName()); } - long dataLen = BinaryRecordWriter.DataLenFieldSize; - boolean isSet = fd.getDef(0).getFieldType() != FieldType.RECORD; - if (isSet) - { - dataLen++; - } - for (Object o : listValue) { dataLen += calculateFieldSize(fd.getDef(0), recordAccessor, o); @@ -681,6 +686,10 @@ private long calculateFieldSize(FieldDef fd, IRecordAccessor recordAccessor, Obj { return fd.getDataLen(); } + else if (fieldValue == null) + { + return BinaryRecordWriter.DataLenFieldSize; + } else { byte[] data = (byte[]) fieldValue; @@ -741,6 +750,27 @@ else if (fd.getSourceType() == HpccSrcType.QSTRING) return dataLen; } + long dataLen = 0; + if (fd.getFieldType() == FieldType.STRING) + { + dataLen = BinaryRecordWriter.DataLenFieldSize; + } + else + { + int eosLen = 1; + if (fd.getSourceType().isUTF16()) + { + eosLen++; + } + + dataLen = eosLen; + } + + if (fieldValue == null) + { + return dataLen; + } + String value = (String) fieldValue; byte[] data = null; @@ -775,21 +805,8 @@ else if (fd.getSourceType() == HpccSrcType.QSTRING) "Unsupported string encoding type: " + fd.getSourceType() + " encountered while writing field: " + fd.getFieldName()); } - if (fd.getFieldType() == FieldType.STRING) - { - return data.length + BinaryRecordWriter.DataLenFieldSize; - } - else - { - int eosLen = 1; - if (fd.getSourceType().isUTF16()) - { - eosLen++; - } - - return data.length + eosLen; - } - + dataLen += data.length; + return dataLen; } case RECORD: { diff --git a/dfsclient/src/main/java/org/hpccsystems/dfs/client/HPCCRecordAccessor.java b/dfsclient/src/main/java/org/hpccsystems/dfs/client/HPCCRecordAccessor.java index d8a6f6a84..cc1269984 100644 --- a/dfsclient/src/main/java/org/hpccsystems/dfs/client/HPCCRecordAccessor.java +++ b/dfsclient/src/main/java/org/hpccsystems/dfs/client/HPCCRecordAccessor.java @@ -22,7 +22,7 @@ import org.hpccsystems.commons.ecl.FieldDef; import org.hpccsystems.commons.ecl.FieldType; -/** +/** * Allows consumers of IRecordAccessor to access data within an @see org.hpccsystems.dfs.client.HPCCRecord. */ public class HPCCRecordAccessor implements IRecordAccessor @@ -61,12 +61,11 @@ public HPCCRecordAccessor(FieldDef fieldDef) childRecordAccessors[i] = null; } } - } /* * (non-Javadoc) - * + * * @see org.hpccsystems.dfs.client.IRecordAccessor#setRecord(java.lang.Object) */ public IRecordAccessor setRecord(Object rd) @@ -77,7 +76,7 @@ public IRecordAccessor setRecord(Object rd) /* * (non-Javadoc) - * + * * @see org.hpccsystems.dfs.client.IRecordAccessor#getNumFields() */ public int getNumFields() @@ -87,17 +86,22 @@ public int getNumFields() /* * (non-Javadoc) - * + * * @see org.hpccsystems.dfs.client.IRecordAccessor#getFieldValue(int) */ public Object getFieldValue(int index) { + if (this.record == null) + { + return null; + } + return this.record.getField(index); } /* * (non-Javadoc) - * + * * @see org.hpccsystems.dfs.client.IRecordAccessor#getFieldDefinition(int) */ public FieldDef getFieldDefinition(int index) @@ -107,7 +111,7 @@ public FieldDef getFieldDefinition(int index) /* * (non-Javadoc) - * + * * @see org.hpccsystems.dfs.client.IRecordAccessor#getChildRecordAccessor(int) */ public IRecordAccessor getChildRecordAccessor(int index) diff --git a/dfsclient/src/test/java/org/hpccsystems/dfs/client/DFSReadWriteTest.java b/dfsclient/src/test/java/org/hpccsystems/dfs/client/DFSReadWriteTest.java index 774025965..010dde693 100644 --- a/dfsclient/src/test/java/org/hpccsystems/dfs/client/DFSReadWriteTest.java +++ b/dfsclient/src/test/java/org/hpccsystems/dfs/client/DFSReadWriteTest.java @@ -261,6 +261,71 @@ else if (field instanceof List) } } + @Test + public void nullElementTests() + { + FieldDef[] stringSetElemFD = new FieldDef[1]; + stringSetElemFD[0] = new FieldDef("strValue", FieldType.STRING, "UTF8", 4, false, false, HpccSrcType.UTF8, new FieldDef[0]); + + FieldDef[] childDatasetElemFD = new FieldDef[1]; + { + FieldDef[] fieldDefs = new FieldDef[1]; + fieldDefs[0] = new FieldDef("strValue", FieldType.STRING, "UTF8", 4, false, false, HpccSrcType.UTF8, new FieldDef[0]); + childDatasetElemFD[0] = new FieldDef("RootRecord", FieldType.RECORD, "rec", 4, false, false, HpccSrcType.LITTLE_ENDIAN, fieldDefs); + } + + FieldDef recordDef = null; + { + FieldDef[] fieldDefs = new FieldDef[3]; + fieldDefs[0] = new FieldDef("stringSet", FieldType.SET, "SET", 4, false, false, HpccSrcType.LITTLE_ENDIAN, stringSetElemFD); + fieldDefs[1] = new FieldDef("childDataset", FieldType.DATASET, "DATASET", 4, false, false, HpccSrcType.LITTLE_ENDIAN, childDatasetElemFD); + fieldDefs[2] = new FieldDef("binaryField", FieldType.BINARY, "BINARY", 128, true, false, HpccSrcType.LITTLE_ENDIAN, new FieldDef[0]); + recordDef = new FieldDef("RootRecord", FieldType.RECORD, "rec", 4, false, false, HpccSrcType.LITTLE_ENDIAN, fieldDefs); + } + + List stringSet = new ArrayList(); + List childDataSet = new ArrayList(); + for (int i = 0; i < 10; i++) + { + if ((i % 2) == 1) + { + stringSet.add("" + i); + + Object[] fields = new Object[1]; + fields[0] = "" + i; + HPCCRecord childRecord = new HPCCRecord(fields, childDatasetElemFD[0]); + childDataSet.add(childRecord); + } + else + { + stringSet.add(null); + childDataSet.add(null); + } + } + + List records = new ArrayList(); + for (int i = 0; i < 10; i++) + { + + Object[] fields = new Object[3]; + fields[0] = stringSet; + fields[1] = childDataSet; + + if ((i % 2) == 1) + { + fields[2] = generateRandomString(128).getBytes(); + } + else + { + fields[2] = null; + } + + HPCCRecord record = new HPCCRecord(fields, recordDef); + records.add(record); + } + writeFile(records, "null::element::test", recordDef, connTO); + } + @Test public void getMetadataTest() throws Exception { @@ -915,7 +980,7 @@ public void protocolVersionTest() HPCCWsDFUClient dfuClient = wsclient.getWsDFUClient(); HpccRemoteFileReader fileReader = null; - try + try { HPCCFile file = new HPCCFile("benchmark::integer::20kb", connString , hpccUser, hpccPass); DataPartition[] fileParts = file.getFileParts(); @@ -928,7 +993,7 @@ public void protocolVersionTest() { Assert.fail("Exception while setting up protocol test: " + e.getMessage()); } - + Version remoteVersion = dfuClient.getTargetHPCCBuildVersion(); if (remoteVersion.isEqualOrNewerThan(newProtocolVersion))