[[ Bug 21326 ]] Preserve binary when modifying with data exec value

livecodeali · livecodeali · commit 205bda157b9b · 2018-06-05T13:44:54.000+01:00
Previously when variables were modified by prepending values obtained
from arrays, the checks for preserving binary strings were not correct
as they failed to take into account that the exec value could be of
valueref type but contain data. This patch changes the check to use
the value's actual type code.

Note the MCExecValue type is a union so the valueref_value field will
occupy the same address as dataref_value - hence this version of the
code will also work for exec values of dataref type.
diff --git a/docs/notes/bugfix-21326.md b/docs/notes/bugfix-21326.md
@@ -0,0 +1 @@
+# Ensure binary strings remain so when binary string is appended or prepended 
diff --git a/engine/src/exec-engine.cpp b/engine/src/exec-engine.cpp
@@ -704,7 +704,8 @@ void MCEngineExecPutIntoVariable(MCExecContext& ctxt, MCExecValue p_value, int p
 	else
     {
         if (MCValueGetTypeCode(p_var . mark . text) == kMCValueTypeCodeData &&
-            p_value . type == kMCExecValueTypeDataRef)
+            MCExecTypeIsValueRef(p_value.type) &&
+            MCValueGetTypeCode(p_value.valueref_value) == kMCValueTypeCodeData)
         {
             // AL-2014-11-20: Make sure the incoming exec value is released.
             MCAutoDataRef t_value_data;
diff --git a/engine/src/variable.cpp b/engine/src/variable.cpp
@@ -613,7 +613,8 @@ bool MCVariable::modify_ctxt(MCExecContext& ctxt, MCExecValue p_value, MCVariabl
 
 bool MCVariable::modify_ctxt(MCExecContext& ctxt, MCExecValue p_value, MCSpan<MCNameRef> p_path, MCVariableSettingStyle p_setting)
 {
-    if (p_value . type == kMCExecValueTypeDataRef)
+    if (MCExecTypeIsValueRef(p_value.type) &&
+        MCValueGetTypeCode(p_value.valueref_value) == kMCValueTypeCodeData)
     {
         if (can_become_data(ctxt, p_path))
         {
diff --git a/tests/lcs/core/engine/put.livecodescript b/tests/lcs/core/engine/put.livecodescript
@@ -0,0 +1,55 @@
+﻿script "CoreEnginePut"
+/*
+Copyright (C) 2018 LiveCode Ltd.
+
+This file is part of LiveCode.
+
+LiveCode is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License v3 as published by the Free
+Software Foundation.
+
+LiveCode is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with LiveCode.  If not see <http://www.gnu.org/licenses/>.  */
+
+// Test MCVariable::modify variant
+on TestPutAfterData
+	local tVar
+	put numToByte(1) into tVar
+	put numToByte(1) after tVar
+	TestAssert "put after preserves binary string", \
+		tVar is strictly a binary string
+end TestPutAfterData
+
+// Test MCEngineExecPutIntoVariable ValueRef chunk variant
+on TestPutAfterDataChunk
+	local tVar, tArray
+	put numToByte(1) into tVar
+	put numToByte(1) after byte 1 of tVar
+	TestAssert "put after chunk preserves binary string", \
+		tVar is strictly a binary string
+end TestPutAfterDataChunk
+
+// Test MCVariable::modify_ctxt variant
+on TestPutAfterDataExecValue
+	local tVar, tArray
+	put numToByte(1) into tVar
+	put numToByte(1) into tArray[1]
+	put tArray[1] after tVar
+	TestAssert "put as execvalue after preserves binary string", \
+		tVar is strictly a binary string
+end TestPutAfterDataExecValue
+
+// Test MCEngineExecPutIntoVariable ExecValue chunk variant
+on TestPutAfterDataChunkExecValue
+	local tVar, tArray
+	put numToByte(1) into tVar
+	put numToByte(1) into tArray[1]
+	put tArray[1] after byte 1 of tVar
+	TestAssert "put as execvalue after chunk preserves binary string", \
+		tVar is strictly a binary string
+end TestPutAfterDataChunkExecValue

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+# Ensure binary strings remain so when binary string is appended or prepended`
Original file line number	Diff line number	Diff line change
`@@ -704,7 +704,8 @@ void MCEngineExecPutIntoVariable(MCExecContext& ctxt, MCExecValue p_value, int p`
`704`	`704`	`else`
`705`	`705`	`{`
`706`	`706`	`if (MCValueGetTypeCode(p_var . mark . text) == kMCValueTypeCodeData &&`
`707`		`- p_value . type == kMCExecValueTypeDataRef)`
	`707`	`+ MCExecTypeIsValueRef(p_value.type) &&`
	`708`	`+ MCValueGetTypeCode(p_value.valueref_value) == kMCValueTypeCodeData)`
`708`	`709`	`{`
`709`	`710`	`// AL-2014-11-20: Make sure the incoming exec value is released.`
`710`	`711`	`MCAutoDataRef t_value_data;`
Original file line number	Diff line number	Diff line change
`@@ -613,7 +613,8 @@ bool MCVariable::modify_ctxt(MCExecContext& ctxt, MCExecValue p_value, MCVariabl`
`613`	`613`
`614`	`614`	`bool MCVariable::modify_ctxt(MCExecContext& ctxt, MCExecValue p_value, MCSpan<MCNameRef> p_path, MCVariableSettingStyle p_setting)`
`615`	`615`	`{`
`616`		`- if (p_value . type == kMCExecValueTypeDataRef)`
	`616`	`+ if (MCExecTypeIsValueRef(p_value.type) &&`
	`617`	`+ MCValueGetTypeCode(p_value.valueref_value) == kMCValueTypeCodeData)`
`617`	`618`	`{`
`618`	`619`	`if (can_become_data(ctxt, p_path))`
`619`	`620`	`{`