[Bug 2923613]: Make the safer [source] handle a [return] at the end of the

file correctly.

3 files changed, 33 insertions, 14 deletions

diff --git a/ChangeLog b/ChangeLog
index 4308e41..d401823 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,17 +1,23 @@
+2009-12-30  Donal K. Fellows  <dkf@users.sf.net>
+
+	* library/safe.tcl (AliasSource): [Bug 2923613]: Make the safer
+	* tests/safe.test (safe-8.9):	  [source] handle a [return] at the
+					  end of the file correctly.
+
 2009-12-30  Miguel Sofer  <msofer@users.sf.net>
 
-	* library/init.tcl (unknown): fix infinite recursion of ::unknown
-	when [set] is undefined [Bug 2824981].
+	* library/init.tcl (unknown): [Bug 2824981]: Fix infinite recursion of
+	::unknown when [set] is undefined.
 
 2009-12-29  Donal K. Fellows  <dkf@users.sf.net>
 
-	* generic/tclHistory.c (Tcl_RecordAndEvalObj): Reduce the amount
-	of allocation and deallocation of memory by caching objects in the
+	* generic/tclHistory.c (Tcl_RecordAndEvalObj): Reduce the amount of
+	allocation and deallocation of memory by caching objects in the
 	interpreter assocData table.
 
-	* generic/tclObj.c (Tcl_GetCommandFromObj): Rewrite the logic so
-	that it does not require making assignments part way through an
-	'if' condition, which was deeply unclear.
+	* generic/tclObj.c (Tcl_GetCommandFromObj): Rewrite the logic so that
+	it does not require making assignments part way through an 'if'
+	condition, which was deeply unclear.
 
 	* generic/tclInterp.c (Tcl_MakeSafe): [Bug 2895741]: Make sure that
 	the min() and max() functions are supported in safe interpreters.
diff --git a/library/safe.tcl b/library/safe.tcl
index eaae00d..7c81f92 100644
--- a/library/safe.tcl
+++ b/library/safe.tcl
@@ -12,7 +12,7 @@
 # See the file "license.terms" for information on usage and redistribution of
 # this file, and for a DISCLAIMER OF ALL WARRANTIES.
 #
-# RCS: @(#) $Id: safe.tcl,v 1.36 2009/12/23 11:17:33 dkf Exp $
+# RCS: @(#) $Id: safe.tcl,v 1.37 2009/12/30 22:26:43 dkf Exp $
 
 #
 # The implementation is based on namespaces. These naming conventions are
@@ -842,7 +842,7 @@ proc ::safe::AliasSource {slave args} {
     # because we want to control [info script] in the slave so information
     # doesn't leak so much. [Bug 2913625]
     set old [::interp eval $slave {info script}]
-    if {[catch {
+    set code [catch {
 	set f [open $realfile]
 	fconfigure $f -eofchar \032
 	if {$encoding ne ""} {
@@ -852,13 +852,15 @@ proc ::safe::AliasSource {slave args} {
 	close $f
 	::interp eval $slave [list info script $file]
 	::interp eval $slave $contents
-    } msg]} {
-	catch {interp eval $slave [list info script $old]}
+    } msg opt]
+    catch {interp eval $slave [list info script $old]}
+    # Note that all non-errors are fine result codes from [source], so we must
+    # take a little care to do it properly. [Bug 2923613]
+    if {$code == 1} {
 	Log $slave $msg
 	return -code error "script error"
     }
-    catch {interp eval $slave [list info script $old]}
-    return $msg
+    return -code $code -options $opt $msg
 }
 
 # AliasLoad is the target of the "load" alias in safe interpreters.
diff --git a/tests/safe.test b/tests/safe.test
index 223559a..db8952b 100644
--- a/tests/safe.test
+++ b/tests/safe.test
@@ -10,7 +10,7 @@
 # See the file "license.terms" for information on usage and redistribution of
 # this file, and for a DISCLAIMER OF ALL WARRANTIES.
 #
-# RCS: @(#) $Id: safe.test,v 1.31 2009/12/29 14:55:42 dkf Exp $
+# RCS: @(#) $Id: safe.test,v 1.32 2009/12/30 22:26:43 dkf Exp $
 
 package require Tcl 8.5
 
@@ -320,6 +320,17 @@ test safe-8.8 {safe source forbids -rsrc} -setup {
 } -returnCodes error -cleanup {
     safe::interpDelete $i
 } -result {wrong # args: should be "source ?-encoding E? fileName"}
+test safe-8.9 {safe source and return} -setup {
+    set returnScript [makeFile {return "ok"} return.tcl]
+    catch {safe::interpDelete $i}
+} -body {
+    safe::interpCreate $i
+    set token [safe::interpAddToAccessPath $i [file dirname $returnScript]]
+    $i eval [list source $token/[file tail $returnScript]]
+} -cleanup {
+    catch {safe::interpDelete $i}
+    removeFile $returnScript
+} -result ok
 
 test safe-9.1 {safe interps' deleteHook} -setup {
     set i "a"