From 75399e97a85433a3805da49a5a11350610a369ae Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 9 Nov 2018 14:13:15 +0000 Subject: test case to cover bug [35a8f1c04a] --- tests/list.test | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/list.test b/tests/list.test index 5a002a9..56414f7 100644 --- a/tests/list.test +++ b/tests/list.test @@ -108,6 +108,21 @@ test list-3.1 {SetListFromAny and lrange/concat results} { test list-4.1 {Bug 3173086} { string is list "{[list \\\\\}]}" } 1 +test list-4.2 {Bug 35a8f1c04a, check correct str-rep} { + set result {} + foreach i { + {#"} {#"""} {#"""""""""""""""} + "#\"{" "#\"\"\"{" "#\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"\{" + "#\"}" "#\"\"\"}" "#\"\"\"\"\"\"\"\"\"\"\"\"\"\"\"\}" + } { + set list [list $i] + set list [string trim " $list "] + if {[llength $list] > 1 || $i ne [lindex $list 0]} { + lappend result "wrong string-representation of list by '$i', length: [llength $list], list: '$list'" + } + } + set result [join $result \n] +} {} # cleanup ::tcltest::cleanupTests -- cgit v0.12 From ddae61a859c90b865bf0d82684638b80522d7922 Mon Sep 17 00:00:00 2001 From: sebres Date: Fri, 9 Nov 2018 14:19:08 +0000 Subject: closes [35a8f1c04a]: minimalist fix - beware of discrepancy between TclScanElement and TclConvertElement (prefer braces considering TCL_DONT_QUOTE_HASH in case of first list element) --- generic/tclUtil.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/generic/tclUtil.c b/generic/tclUtil.c index fb5b21c..2a0d51a 100644 --- a/generic/tclUtil.c +++ b/generic/tclUtil.c @@ -944,6 +944,10 @@ TclScanElement( int preferEscape = 0; /* Use preferences to track whether to use */ int preferBrace = 0; /* CONVERT_MASK mode. */ int braceCount = 0; /* Count of all braces '{' '}' seen. */ + + if ((*src == '#') && !(*flagPtr & TCL_DONT_QUOTE_HASH)) { + preferBrace = 1; + } #endif if ((p == NULL) || (length == 0) || ((*p == '\0') && (length == -1))) { -- cgit v0.12 From 83ec300abdbc8486b4fd3272cd74a9c3f162ad7f Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 9 Nov 2018 15:39:26 +0000 Subject: Revise bug fix to support (length == 0) correctly. Added comments and improved safety in caller. --- generic/tclListObj.c | 19 ++++++++++++++++++- generic/tclUtil.c | 21 +++++++++++++++++---- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/generic/tclListObj.c b/generic/tclListObj.c index e1dba8c..09ad034 100644 --- a/generic/tclListObj.c +++ b/generic/tclListObj.c @@ -1872,7 +1872,21 @@ UpdateStringOfList( * Pass 2: copy into string rep buffer. */ + /* + * We used to set the string length here, relying on a presumed + * guarantee that the number of bytes TclScanElement() calls reported + * to be needed was a precise count and not an over-estimate, so long + * as the same flag values were passed to TclConvertElement(). + * + * Then we saw [35a8f1c04a], where a bug in TclScanElement() caused + * that guarantee to fail. Rather than trust there are no more bugs, + * we set the length after the loop based on what was actually written, + * an not on what was predicted. + * listPtr->length = bytesNeeded - 1; + * + */ + listPtr->bytes = ckalloc((unsigned) bytesNeeded); dst = listPtr->bytes; for (i = 0; i < numElems; i++) { @@ -1881,7 +1895,10 @@ UpdateStringOfList( dst += TclConvertElement(elem, length, dst, flagPtr[i]); *dst++ = ' '; } - listPtr->bytes[listPtr->length] = '\0'; + dst[-1] = '\0'; + + /* Here is the safe setting of the string length. */ + listPtr->length = dst - 1 - listPtr->bytes; if (flagPtr != localFlags) { ckfree((char *) flagPtr); diff --git a/generic/tclUtil.c b/generic/tclUtil.c index 2a0d51a..d065069 100644 --- a/generic/tclUtil.c +++ b/generic/tclUtil.c @@ -944,10 +944,6 @@ TclScanElement( int preferEscape = 0; /* Use preferences to track whether to use */ int preferBrace = 0; /* CONVERT_MASK mode. */ int braceCount = 0; /* Count of all braces '{' '}' seen. */ - - if ((*src == '#') && !(*flagPtr & TCL_DONT_QUOTE_HASH)) { - preferBrace = 1; - } #endif if ((p == NULL) || (length == 0) || ((*p == '\0') && (length == -1))) { @@ -956,6 +952,23 @@ TclScanElement( return 2; } +#if COMPAT + /* + * We have an established history in TclConvertElement() when quoting + * because of a leading hash character to force what would be the + * CONVERT_MASK mode into the CONVERT_BRACE mode. That is, we format + * the element #{a"b} like this: + * {#{a"b}} + * and not like this: + * \#{a\"b} + * This is inconsistent with [list x{a"b}], but we will not change that now. + * Set that preference here so that we compute a tight size requirement. + */ + if ((*src == '#') && !(*flagPtr & TCL_DONT_QUOTE_HASH)) { + preferBrace = 1; + } +#endif + if ((*p == '{') || (*p == '"')) { /* * Must escape or protect so leading character of value is not -- cgit v0.12 From 4df4dafad67c2feba67b0d628d7174937e6c2742 Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 9 Nov 2018 15:47:37 +0000 Subject: Additional test --- tests/list.test | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/list.test b/tests/list.test index 56414f7..18cc42a 100644 --- a/tests/list.test +++ b/tests/list.test @@ -123,6 +123,9 @@ test list-4.2 {Bug 35a8f1c04a, check correct str-rep} { } set result [join $result \n] } {} +test list-4.3 {Bug 35a8f1c04a, check correct string length} { + string length [list #""] +} 5 # cleanup ::tcltest::cleanupTests -- cgit v0.12