From f24f565e58f28462cf861e130da8599e86f84d10 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Wed, 23 Sep 2015 15:32:46 -0400 Subject: [PATCH 1/4] validate field names check for invalid characters in field names (eg '.'). Restrict character set to ascii alpha numeric and '_'. --- src/factory/FieldCreateFactory.cpp | 36 ++++++++++++++++++++++++++++++ testApp/pv/testPVData.cpp | 21 ++++++++++++++++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/factory/FieldCreateFactory.cpp b/src/factory/FieldCreateFactory.cpp index 76eb7f1..bcf7038 100644 --- a/src/factory/FieldCreateFactory.cpp +++ b/src/factory/FieldCreateFactory.cpp @@ -947,9 +947,41 @@ StructureConstPtr FieldCreate::createStructure () const return createStructure(fieldNames,fields); } +namespace { +void validateFieldName(const std::string& n) +{ + for(size_t i=0, N=n.size(); i='a' && c<='z') {} + else if(c>='A' && c<='Z') {} + else if(c>='0' && c<='9') {} + else { + switch(c){ + case '_': + break; + default: + { + std::ostringstream msg; + msg<<"Invalid charactor '"< sp(new Structure(fieldNames,fields), Field::Deleter()); StructureConstPtr structure = sp; @@ -961,6 +993,7 @@ StructureConstPtr FieldCreate::createStructure ( StringArray const & fieldNames, FieldConstPtrArray const & fields) const { + validateFieldNames(fieldNames); // TODO use std::make_shared std::tr1::shared_ptr sp(new Structure(fieldNames,fields,id), Field::Deleter()); StructureConstPtr structure = sp; @@ -979,6 +1012,7 @@ StructureArrayConstPtr FieldCreate::createStructureArray( UnionConstPtr FieldCreate::createUnion ( StringArray const & fieldNames,FieldConstPtrArray const & fields) const { + validateFieldNames(fieldNames); // TODO use std::make_shared std::tr1::shared_ptr sp(new Union(fieldNames,fields), Field::Deleter()); UnionConstPtr punion = sp; @@ -990,6 +1024,7 @@ UnionConstPtr FieldCreate::createUnion ( StringArray const & fieldNames, FieldConstPtrArray const & fields) const { + validateFieldNames(fieldNames); // TODO use std::make_shared std::tr1::shared_ptr sp(new Union(fieldNames,fields,id), Field::Deleter()); UnionConstPtr punion = sp; @@ -1039,6 +1074,7 @@ StructureConstPtr FieldCreate::appendFields( StringArray const & fieldNames, FieldConstPtrArray const & fields) const { + validateFieldNames(fieldNames); StringArray const & oldNames = structure->getFieldNames(); FieldConstPtrArray const & oldFields = structure->getFields(); size_t oldLen = oldNames.size(); diff --git a/testApp/pv/testPVData.cpp b/testApp/pv/testPVData.cpp index 1c83f88..000d808 100644 --- a/testApp/pv/testPVData.cpp +++ b/testApp/pv/testPVData.cpp @@ -65,6 +65,24 @@ static void testCreatePVStructure() std::cout << "testCreatePVStructure PASSED" << std::endl; } +static void testCreatePVStructureWithInvalidName() +{ + testDiag("testCreatePVStructureWithInvalidName"); + StringArray fieldNames; + fieldNames.push_back("ok"); + fieldNames.push_back("this.is-wrong"); + PVFieldPtrArray pvFields; + pvFields.push_back(pvDataCreate->createPVScalar(pvString)); + pvFields.push_back(pvDataCreate->createPVScalar(pvInt)); + try{ + PVStructurePtr pvParent = pvDataCreate->createPVStructure( + fieldNames,pvFields); + testFail("Creation of invalid field name '%s' was allowed", fieldNames[1].c_str()); + } catch(std::invalid_argument& e) { + testPass("Creation of invalid field name '%s' fails as expected", fieldNames[1].c_str()); + } +} + static void testPVScalarCommon(string /*fieldName*/,ScalarType stype) { PVScalarPtr pvScalar = pvDataCreate->createPVScalar(stype); @@ -641,13 +659,14 @@ static void testFieldAccess() MAIN(testPVData) { - testPlan(242); + testPlan(243); fieldCreate = getFieldCreate(); pvDataCreate = getPVDataCreate(); standardField = getStandardField(); standardPVField = getStandardPVField(); convert = getConvert(); testCreatePVStructure(); + testCreatePVStructureWithInvalidName(); testPVScalar(); testScalarArray(); testRequest(); From 3714be4f16ef953a9fd26d1d2f8f477968f51d96 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Thu, 24 Sep 2015 13:04:49 -0400 Subject: [PATCH 2/4] fail zero length field names --- src/factory/FieldCreateFactory.cpp | 11 ++++++++--- testApp/pv/testPVData.cpp | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/factory/FieldCreateFactory.cpp b/src/factory/FieldCreateFactory.cpp index bcf7038..696b3a1 100644 --- a/src/factory/FieldCreateFactory.cpp +++ b/src/factory/FieldCreateFactory.cpp @@ -948,14 +948,19 @@ StructureConstPtr FieldCreate::createStructure () const } namespace { +bool xisalnum(char c) +{ + return (c>='a' && c<='z') || (c>='A' && c<='Z') || (c>='0' && c<='9'); +} + void validateFieldName(const std::string& n) { + if(n.size()==0) + throw std::invalid_argument("zero length field names not allowed"); for(size_t i=0, N=n.size(); i='a' && c<='z') {} - else if(c>='A' && c<='Z') {} - else if(c>='0' && c<='9') {} + if(xisalnum(c)) {} else { switch(c){ case '_': diff --git a/testApp/pv/testPVData.cpp b/testApp/pv/testPVData.cpp index 000d808..6d061e8 100644 --- a/testApp/pv/testPVData.cpp +++ b/testApp/pv/testPVData.cpp @@ -79,6 +79,7 @@ static void testCreatePVStructureWithInvalidName() fieldNames,pvFields); testFail("Creation of invalid field name '%s' was allowed", fieldNames[1].c_str()); } catch(std::invalid_argument& e) { + testDiag("Exception: \"%s\"", e.what()); testPass("Creation of invalid field name '%s' fails as expected", fieldNames[1].c_str()); } } From f4a00f2b0f8eb5668f015c24d7c32b800e2c2bcb Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Mon, 23 Nov 2015 14:31:27 -0500 Subject: [PATCH 3/4] field names may not begin with a digit Enforce C identifer syntax [A-Za-z_][A-Za-z0-9_]* --- src/factory/FieldCreateFactory.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/factory/FieldCreateFactory.cpp b/src/factory/FieldCreateFactory.cpp index 696b3a1..7551315 100644 --- a/src/factory/FieldCreateFactory.cpp +++ b/src/factory/FieldCreateFactory.cpp @@ -955,8 +955,14 @@ bool xisalnum(char c) void validateFieldName(const std::string& n) { + // enforce [A-Za-z_][A-Za-z0-9_]* if(n.size()==0) throw std::invalid_argument("zero length field names not allowed"); + if(n[0]>='0' && n[0]<='9') { + std::ostringstream msg; + msg<<"Field name \""< Date: Mon, 23 Nov 2015 14:33:43 -0500 Subject: [PATCH 4/4] fix testCreateRequest field names may not begin with a digit --- testApp/copy/testCreateRequest.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/testApp/copy/testCreateRequest.cpp b/testApp/copy/testCreateRequest.cpp index d183bb1..616c6d1 100644 --- a/testApp/copy/testCreateRequest.cpp +++ b/testApp/copy/testCreateRequest.cpp @@ -216,8 +216,8 @@ static void testCreateRequestInternal() { testPass("request %s",request.c_str()); request = string("field(alarm,timeStamp,supply{") - + "0{voltage.value,current.value,power.value}," - + "1{voltage.value,current.value,power.value}" + + "zero{voltage.value,current.value,power.value}," + + "one{voltage.value,current.value,power.value}" + "})"; if(debug) { cout << "request " << request <createRequest(request); @@ -226,12 +226,12 @@ static void testCreateRequestInternal() { testOk1(pvRequest.get()!=NULL); testOk1(pvRequest->getSubField("field.alarm").get()!=NULL); testOk1(pvRequest->getSubField("field.timeStamp").get()!=NULL); - testOk1(pvRequest->getSubField("field.supply.0.voltage.value").get()!=NULL); - testOk1(pvRequest->getSubField("field.supply.0.current.value").get()!=NULL); - testOk1(pvRequest->getSubField("field.supply.0.power.value").get()!=NULL); - testOk1(pvRequest->getSubField("field.supply.1.voltage.value").get()!=NULL); - testOk1(pvRequest->getSubField("field.supply.1.current.value").get()!=NULL); - testOk1(pvRequest->getSubField("field.supply.1.power.value").get()!=NULL); + testOk1(pvRequest->getSubField("field.supply.zero.voltage.value").get()!=NULL); + testOk1(pvRequest->getSubField("field.supply.zero.current.value").get()!=NULL); + testOk1(pvRequest->getSubField("field.supply.zero.power.value").get()!=NULL); + testOk1(pvRequest->getSubField("field.supply.one.voltage.value").get()!=NULL); + testOk1(pvRequest->getSubField("field.supply.one.current.value").get()!=NULL); + testOk1(pvRequest->getSubField("field.supply.one.power.value").get()!=NULL); testPass("request %s",request.c_str()); request = string("record[process=true,xxx=yyy]")