diff --git a/README.md b/README.md index c86c4b1..f96f27e 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,41 @@ For more documentation and examples, see https://loupeteam.github.io/LoupeDocs/l # Installation To install using the Loupe Package Manager (LPM), in an initialized Automation Studio project directory run `lpm install stringext`. For more information about LPM, see https://loupeteam.github.io/LoupeDocs/tools/lpm.html +# Self tests + +The example project in `example/As6Project` carries table-driven self tests for +some of the library functions. Each is gated by a BOOL in the `Default` program: +set it, and the test runs once on the next cycle and writes its results back. + +| Trigger | Covers | Results | +|---|---|---| +| `formatTest` | `formatString()` | `formatTestPass`, `formatTestFail`, `formatTestFirstFail` | +| `hexTest` | `HexStringToUDINT()`, `UDINTToHexString()` | `hexTestPass`, `hexTestFail`, `hexTestFirstFail` | + +`...Fail` must come back 0. `...FirstFail` names the first input that did not +behave as expected, and is left empty when the failure was a scalar assertion +rather than a table case. + +Build and deploy the `Intel` configuration to ARsim, then set the trigger from +the Automation Studio watch window, or over OPC UA from any client — the four +variables of each suite are published in `OpcUaMap.uad`. + +The `Intel` configuration enables the OPC UA server on port 4840 and adds an +unencrypted, anonymous endpoint, so that the self tests can be driven without +putting a credential in this repository. The encrypted policies AS enables by +default are still offered alongside it. Role `Everyone` is granted browse, +read and write on the default view, so all eight variables above — results +included, not just the triggers — are anonymously writable. + +That is appropriate for this project, which exists to compile the library and +run its tests. **Do not copy this OPC UA configuration into a real machine.** + +The project pins OPC UA C/S 6.6.1, which is what the build runner carries. +Publishing has been confirmed at runtime on 6.5.0 and 6.7.0; 6.6.1 has only +been confirmed to build. 6.0.0 does **not** work — it compiles, and then the +generated NodeSet fails to load with `BadNodeIdUnknown`, so nothing is +published at all. If the variables do not appear, check this version first. + ## Licensing This project is licensed under the [MIT License](LICENSE). \ No newline at end of file diff --git a/example/As6Project/AsProject.apj b/example/As6Project/AsProject.apj index 4658f96..3124b04 100644 --- a/example/As6Project/AsProject.apj +++ b/example/As6Project/AsProject.apj @@ -7,4 +7,7 @@ + + + \ No newline at end of file diff --git a/example/As6Project/Logical/Programs/Default/Main.c b/example/As6Project/Logical/Programs/Default/Main.c index 14a2dee..16c9ddb 100644 --- a/example/As6Project/Logical/Programs/Default/Main.c +++ b/example/As6Project/Logical/Programs/Default/Main.c @@ -159,6 +159,284 @@ static void runHexTest(void) } +/* Self test for formatString(). + Set formatTest to run it; formatTestFail must come back 0. + + The format of each case is copied into a scratch buffer whose remaining + bytes are filled with 'X' after the terminator. A formatter that walks + past the terminator - as a trailing bare '%' once did - drags that filler + into the destination, so the expected-output compare catches it. Only the + trailing '%' path can reach past the terminator at all: every other branch + consumes a character it has already seen to be non-null. */ + +typedef struct formatCase_typ { + char* Format; + char* Expect; +} formatCase_typ; + +static const formatCase_typ formatCases[] = { + + /* Trailing bare '%' - no command character follows it */ + { "value: %", "value: " }, + { "%", "" }, + { "trail %d%", "trail 42" }, + { "%%%", "%" }, + + /* Ordinary substitutions */ + { "no format", "no format" }, + { "%d items", "42 items" }, + { "%d and %d", "42 and -7" }, + { "%s=%b", "abc=TRUE" }, + + /* Escaped percent, and an unknown command character */ + { "100%% done", "100% done" }, + { "a%zb", "ab" }, + + /* Both halves of %b, and a null entry in the string array, which is + skipped without consuming a later argument in its place */ + { "%b%b", "TRUEFALSE" }, + { "%s|%s|%s", "abc||xyz" }, + + /* One more %d than the argument array holds - the sixth is dropped, + which is why the expected output ends in a space */ + { "%d %d %d %d %d %d", "42 -7 0 0 0 " }, + + /* The %s array runs out the same way, but through a second condition: + a null entry is skipped by one test and an exhausted array by another */ + { "%s%s%s%s%s%s", "abcxyz" }, + + /* %i is an alias for %d and is otherwise never exercised */ + { "%i and %i", "42 and -7" }, + +}; + + +/* Cases that need a destination smaller than the formatted result. Same + scratch buffer treatment; DestSize is passed to formatString() verbatim. */ + +typedef struct formatSizeCase_typ { + char* Format; + unsigned long DestSize; + char* Expect; +} formatSizeCase_typ; + +static const formatSizeCase_typ formatSizeCases[] = { + + /* Trailing '%' reached with only the reserved null byte left. The loop + still has room to enter the '%' branch, so this is a genuine pre-fix + failure - the old code stepped onto the filler and copied it */ + { "abcdef%", 8, "abcdef" }, + + /* Destination fills before the format character is ever reached */ + { "abcdef%", 4, "abc" }, + + /* Only the reserved null byte fits - nothing is written at all */ + { "%", 1, "" }, + + /* Truncation part way through a substitution. %b is the only branch + whose lengths are hard coded rather than computed, so it is the + likeliest place for a length bug and gets its own case. */ + { "%d", 2, "4" }, + { "%s", 3, "ab" }, + { "%b", 3, "TR" }, + + /* Escaped percent with exactly one byte to spare, and an exact fit */ + { "%%", 2, "%" }, + { "abcd", 5, "abcd" }, + +}; + + +static void runFormatTest(void) +{ + unsigned long i; + unsigned long j; + StrExtArgs_typ Args; + char Scratch[64]; + char Dest[80]; + char Expected[64]; + signed long Length; + unsigned long Overrun; + + formatTestPass= 0; + formatTestFail= 0; + strcpy((char*)formatTestFirstFail, ""); + + for(i=0; i<(sizeof(formatCases)/sizeof(formatCases[0])); i++){ + + memset(&Args, 0, sizeof(Args)); + Args.i[0]= 42; + Args.i[1]= -7; + Args.b[0]= 1; + Args.s[0]= (unsigned long)"abc"; + Args.s[2]= (unsigned long)"xyz"; + + /* Fill with junk, then lay the format in - everything after the + terminator stays 'X' and must never reach the destination. The + last byte is terminated so that a library that does run off the + end stops here rather than walking out of the buffer. */ + for(j=0; j= (sizeof(Scratch) - 8)){ + + if(strlen((char*)formatTestFirstFail) == 0){ + strncpy((char*)formatTestFirstFail, formatCases[i].Format, 79); + } + + formatTestFail++; + continue; + + } + + strcpy(Scratch, formatCases[i].Format); + + memset(Dest, 0, sizeof(Dest)); + + Length= formatString(Dest, sizeof(Dest), Scratch, &Args); + + if( (strcmp(Dest, formatCases[i].Expect) == 0) + && (Length == (signed long)strlen(formatCases[i].Expect)) + ){ + + formatTestPass++; + + } + else{ + + formatTestFail++; + + if(strlen((char*)formatTestFirstFail) == 0){ + strncpy((char*)formatTestFirstFail, formatCases[i].Format, 79); + } + + } + + } + + + /* Null pointers */ + + if( formatString(Dest, sizeof(Dest), "abc", 0) == STREXT_ERR_INVALID_INPUT ) formatTestPass++; else formatTestFail++; + if( formatString(0, sizeof(Dest), "abc", &Args) == STREXT_ERR_INVALID_INPUT ) formatTestPass++; else formatTestFail++; + if( formatString(Dest, sizeof(Dest), 0, &Args) == STREXT_ERR_INVALID_INPUT ) formatTestPass++; else formatTestFail++; + + + /* Destinations too small for the formatted result. Arguments are set up + explicitly here rather than inherited from the loop above, so these + cases do not change meaning if that table is ever edited. */ + + memset(&Args, 0, sizeof(Args)); + Args.i[0]= 42; + Args.i[1]= -7; + Args.b[0]= 1; + Args.s[0]= (unsigned long)"abc"; + Args.s[2]= (unsigned long)"xyz"; + + for(i=0; i<(sizeof(formatSizeCases)/sizeof(formatSizeCases[0])); i++){ + + for(j=0; j= (sizeof(Scratch) - 8)){ + + if(strlen((char*)formatTestFirstFail) == 0){ + strncpy((char*)formatTestFirstFail, formatSizeCases[i].Format, 79); + } + + formatTestFail++; + continue; + + } + + strcpy(Scratch, formatSizeCases[i].Format); + + /* Fill the whole destination, so that anything written at or past + DestSize survives as a 'Z' and can be checked for below */ + memset(Dest, 'Z', sizeof(Dest)); + + Length= formatString(Dest, formatSizeCases[i].DestSize, Scratch, &Args); + + /* Nothing may be written at or beyond DestSize */ + Overrun= 0; + for(j=formatSizeCases[i].DestSize; j + + + + UaCsConfig.uacfg + + diff --git a/example/As6Project/Physical/ARM/X20CP0410/Connectivity/OpcUaCs/UaCsConfig.uacfg b/example/As6Project/Physical/ARM/X20CP0410/Connectivity/OpcUaCs/UaCsConfig.uacfg new file mode 100644 index 0000000..beb902d --- /dev/null +++ b/example/As6Project/Physical/ARM/X20CP0410/Connectivity/OpcUaCs/UaCsConfig.uacfg @@ -0,0 +1,6 @@ + + + + + + diff --git a/example/As6Project/Physical/ARM/X20CP0410/Connectivity/Package.pkg b/example/As6Project/Physical/ARM/X20CP0410/Connectivity/Package.pkg index bfea000..666b638 100644 --- a/example/As6Project/Physical/ARM/X20CP0410/Connectivity/Package.pkg +++ b/example/As6Project/Physical/ARM/X20CP0410/Connectivity/Package.pkg @@ -1,5 +1,7 @@  - + + OpcUaCs + \ No newline at end of file diff --git a/example/As6Project/Physical/Intel/5PC900_TS17_04/Connectivity/OpcUaCs/OpcUaMap.uad b/example/As6Project/Physical/Intel/5PC900_TS17_04/Connectivity/OpcUaCs/OpcUaMap.uad new file mode 100644 index 0000000..fd50fe1 --- /dev/null +++ b/example/As6Project/Physical/Intel/5PC900_TS17_04/Connectivity/OpcUaCs/OpcUaMap.uad @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/example/As6Project/Physical/Intel/5PC900_TS17_04/Connectivity/OpcUaCs/Package.pkg b/example/As6Project/Physical/Intel/5PC900_TS17_04/Connectivity/OpcUaCs/Package.pkg index c43b450..a636ccb 100644 --- a/example/As6Project/Physical/Intel/5PC900_TS17_04/Connectivity/OpcUaCs/Package.pkg +++ b/example/As6Project/Physical/Intel/5PC900_TS17_04/Connectivity/OpcUaCs/Package.pkg @@ -1,5 +1,9 @@  - - \ No newline at end of file + + UaCsConfig.uacfg + UaDvConfig.uadcfg + OpcUaMap.uad + + diff --git a/example/As6Project/Physical/Intel/5PC900_TS17_04/Connectivity/OpcUaCs/UaCsConfig.uacfg b/example/As6Project/Physical/Intel/5PC900_TS17_04/Connectivity/OpcUaCs/UaCsConfig.uacfg new file mode 100644 index 0000000..c6ab1ad --- /dev/null +++ b/example/As6Project/Physical/Intel/5PC900_TS17_04/Connectivity/OpcUaCs/UaCsConfig.uacfg @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + + + + + + + diff --git a/example/As6Project/Physical/Intel/5PC900_TS17_04/Connectivity/OpcUaCs/UaDvConfig.uadcfg b/example/As6Project/Physical/Intel/5PC900_TS17_04/Connectivity/OpcUaCs/UaDvConfig.uadcfg new file mode 100644 index 0000000..11c4857 --- /dev/null +++ b/example/As6Project/Physical/Intel/5PC900_TS17_04/Connectivity/OpcUaCs/UaDvConfig.uadcfg @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + diff --git a/src/Ar/StringExt/ANSIC.lby b/src/Ar/StringExt/ANSIC.lby index 7d9ff69..e695100 100644 --- a/src/Ar/StringExt/ANSIC.lby +++ b/src/Ar/StringExt/ANSIC.lby @@ -1,6 +1,6 @@  - + StringExt.typ StringExt.var diff --git a/src/Ar/StringExt/CHANGELOG.md b/src/Ar/StringExt/CHANGELOG.md index 7abcb7a..a781e25 100644 --- a/src/Ar/StringExt/CHANGELOG.md +++ b/src/Ar/StringExt/CHANGELOG.md @@ -1,3 +1,7 @@ +1.1.1 - Fix out of bounds read in formatString() + A format string ending in a bare '%' stepped the source pointer past the + null terminator and kept reading until it happened to hit a zero byte + 1.1.0 - Add HexStringToUDINT() and UDINTToHexString() Parse and format hex over the full 32-bit range, with 0x, 16# and $ prefixes supported and parse failures reported separately from the value diff --git a/src/Ar/StringExt/FormatString.c b/src/Ar/StringExt/FormatString.c index 1ec168f..b483298 100644 --- a/src/Ar/StringExt/FormatString.c +++ b/src/Ar/StringExt/FormatString.c @@ -70,7 +70,14 @@ signed long formatString(plcstring* destination, unsigned long destSize, plcstri *dst = '\0'; // Consume the char and investigate the command - switch (*(++src)) + src++; + + // A '%' at the very end of the source has no command character. + // Leave the loop here - the cases below would step past the terminator. + if (*src == '\0') + break; // exits the while loop, not a switch + + switch (*src) { // Insert bool case 'b':