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

correctly.

3 files changed, 26 insertions, 7 deletions

diff --git a/ChangeLog b/ChangeLog
index 28c3a18..3aeef35 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+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-29  Donal K. Fellows  <dkf@users.sf.net>
 
 	* generic/tclInterp.c (Tcl_MakeSafe): [Bug 2895741]: Make sure that
diff --git a/library/safe.tcl b/library/safe.tcl
index 4bee33b..e89ca29 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.16.4.5 2009/12/23 11:13:46 dkf Exp $
+# RCS: @(#) $Id: safe.tcl,v 1.16.4.6 2009/12/30 22:20:57 dkf Exp $
 
 #
 # The implementation is based on namespaces. These naming conventions are
@@ -841,7 +841,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 ""} {
@@ -851,13 +851,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 e4e3596..aedc7de 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.22.4.6 2009/12/29 13:13:18 dkf Exp $
+# RCS: @(#) $Id: safe.test,v 1.22.4.7 2009/12/30 22:20:57 dkf Exp $
 
 package require Tcl 8.5
 
@@ -300,6 +300,17 @@ test safe-8.8 {safe source forbids -rsrc} {
 	    $msg \
 	    [safe::interpDelete $i] ;
 } {1 {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} {
     set i "a";