Fix tonumber to handle bad input without throwing exceptions

src/MoonSharp.Interpreter.Tests/EndToEnd/TonumberTests.cs

@@ -0,0 +1,119 @@
+using NUnit.Framework;
+
+namespace MoonSharp.Interpreter.Tests.EndToEnd
+{
+	/// <summary>
+	/// Fork tests for tonumber(e, base). The upstream implementation delegated to
+	/// Convert.ToUInt32, which let raw CLR FormatException/OverflowException escape the
+	/// interpreter on unparseable or oversized input instead of returning nil per Lua
+	/// semantics — a host-crash primitive when tonumber is fed untrusted strings.
+	/// </summary>
+	[TestFixture]
+	public class TonumberTests
+	{
+		private static DynValue Run(string code) { return Script.RunString(code); }
+		private static double Num(string code) { return Run(code).Number; }
+		private static bool IsNil(string code) { return Run(code).IsNil(); }
+
+		[Test]
+		public void Tonumber_WithBase_NullBackedStringReturnsNilNotCrash()
+		{
+			// A host can hand the interpreter a string DynValue wrapping a null
+			// (DynValue.NewString(null)). Feeding it to tonumber(x, base) must yield nil,
+			// not a raw NullReferenceException escaping into the host.
+			Script s = new Script();
+			s.Globals.Set("x", DynValue.NewString((string)null));
+			DynValue r = s.DoString("return tonumber(x, 16)");
+			Assert.IsTrue(r.IsNil());
+		}
+
+		[Test]
+		public void Tonumber_WithBase_ParsesValidNumerals()
+		{
+			Assert.AreEqual(255.0, Num("return tonumber('ff', 16)"));
+			Assert.AreEqual(255.0, Num("return tonumber('FF', 16)"));
+			Assert.AreEqual(2.0, Num("return tonumber('10', 2)"));
+			Assert.AreEqual(511.0, Num("return tonumber('777', 8)"));
+			Assert.AreEqual(123.0, Num("return tonumber('123', 10)"));
+			Assert.AreEqual(15.0, Num("return tonumber('120', 3)"));
+			Assert.AreEqual(71.0, Num("return tonumber('78', 9)"));
+		}
+
+		[Test]
+		public void Tonumber_WithBase_SupportsBasesUpTo36()
+		{
+			Assert.AreEqual(35.0, Num("return tonumber('z', 36)"));
+			Assert.AreEqual(1295.0, Num("return tonumber('zz', 36)"));
+			Assert.AreEqual(19.0, Num("return tonumber('j', 20)"));
+		}
+
+		[Test]
+		public void Tonumber_WithBase_HandlesSignAndWhitespace()
+		{
+			Assert.AreEqual(-255.0, Num("return tonumber('-ff', 16)"));
+			Assert.AreEqual(16.0, Num("return tonumber('  10  ', 16)"));
+			Assert.AreEqual(-5.0, Num("return tonumber(' -101 ', 2)"));
+		}
+
+		[Test]
+		public void Tonumber_WithBase_CoercesNumberFirstArgument()
+		{
+			Assert.AreEqual(5.0, Num("return tonumber(101, 2)"));
+		}
+
+		[Test]
+		public void Tonumber_WithBase_ReturnsNilOnUnparseableInput()
+		{
+			// These previously escaped as a raw .NET FormatException.
+			Assert.IsTrue(IsNil("return tonumber('zz', 16)"));
+			Assert.IsTrue(IsNil("return tonumber('not hex!', 16)"));
+			Assert.IsTrue(IsNil("return tonumber('0x10', 16)")); // '0x' prefix is not a hex digit in Lua
+			Assert.IsTrue(IsNil("return tonumber('12.5', 10)"));
+			Assert.IsTrue(IsNil("return tonumber('1 2', 16)"));
+			Assert.IsTrue(IsNil("return tonumber('', 16)"));
+			Assert.IsTrue(IsNil("return tonumber('   ', 16)"));
+			Assert.IsTrue(IsNil("return tonumber('-', 16)"));
+			Assert.IsTrue(IsNil("return tonumber('+10', 16)")); // only '-' is a valid sign in Lua
+		}
+
+		[Test]
+		public void Tonumber_WithBase_ReturnsNilOnDigitOutOfBase()
+		{
+			// Previously raised ScriptRuntimeException 'invalid character' for bases 3-9.
+			Assert.IsTrue(IsNil("return tonumber('2', 2)"));
+			Assert.IsTrue(IsNil("return tonumber('9', 8)"));
+			Assert.IsTrue(IsNil("return tonumber('17', 6)"));
+			Assert.IsTrue(IsNil("return tonumber('ff', 10)"));
+		}
+
+		[Test]
+		public void Tonumber_WithBase_LargeValuesDoNotThrow()
+		{
+			// Previously escaped as a raw .NET OverflowException (> uint.MaxValue).
+			DynValue v = Run("return tonumber(string.rep('f', 20), 16)");
+			Assert.AreEqual(DataType.Number, v.Type);
+			Assert.Greater(v.Number, 1e23);
+		}
+
+		[Test]
+		public void Tonumber_WithBase_OutOfRangeBaseStillRaises()
+		{
+			Assert.Throws<ScriptRuntimeException>(() => Run("return tonumber('10', 1)"));
+			Assert.Throws<ScriptRuntimeException>(() => Run("return tonumber('10', 37)"));
+			Assert.Throws<ScriptRuntimeException>(() => Run("return tonumber('10', 200)"));
+		}
+
+		[Test]
+		public void Tonumber_WithBase_BadInputReturnsNilNotCrash()
+		{
+			// A script must be able to branch on bad input (nil) rather than have the
+			// call escape as a CLR exception into the embedding host.
+			DynValue v = Run(@"
+				local ok = tonumber('deadbeef', 16)
+				local bad = tonumber('DEAD BEEF QUOTE', 16)
+				return ok ~= nil and bad == nil
+			");
+			Assert.IsTrue(v.Boolean);
+		}
+	}
+}

src/MoonSharp.Interpreter.Tests/TestMore/301-basic.t

@@ -327,9 +327,7 @@ error_like(function () tonumber('111', 200) end,
            "^[^:]+:%d+: bad argument #2 to 'tonumber' %(base out of range%)",
            "function tonumber (bad base)")
 
-error_like(function () tonumber('17', 6) end,
-           "^[^:]+:%d+: bad argument #1 to 'tonumber' %(invalid character%)",
-           "function tonumber (bad base)")
+is(tonumber('17', 6), nil, "function tonumber (invalid digit for base)")
 
 is(tostring('text'), 'text', "function tostring")
 is(tostring(3.14), '3.14')

src/MoonSharp.Interpreter/CoreLib/BasicModule.cs

@@ -238,44 +238,71 @@ public static DynValue tonumber(ScriptExecutionContext executionContext, Callbac
 			}
 			else
 			{
-                //!COMPAT: tonumber supports only 2,8,10 or 16 as base
-                //UPDATE: added support for 3-9 base numbers
-                DynValue ee;
+				DynValue ee;
 
 				if (args[0].Type != DataType.Number)
 					ee = args.AsType(0, "tonumber", DataType.String, false);
 				else
-					ee = DynValue.NewString(args[0].Number.ToString(CultureInfo.InvariantCulture)); ;
+					ee = DynValue.NewString(args[0].Number.ToString(CultureInfo.InvariantCulture));
 
 				int bb = (int)b.Number;
 
-			    uint uiv = 0;
-                if (bb == 2 || bb == 8 || bb == 10 || bb == 16)
-			    {
-                    uiv = Convert.ToUInt32(ee.String.Trim(), bb);
-                }
-			    else if (bb < 10 && bb > 2) // Support for 3, 4, 5, 6, 7 and 9 based numbers
-			    {
-			        foreach (char digit in ee.String.Trim())
-			        {
-			            int value = digit - 48;
-			            if (value < 0 || value >= bb)
-			            {
-                            throw new ScriptRuntimeException("bad argument #1 to 'tonumber' (invalid character)");
-                        }
-
-                        uiv = (uint)(uiv * bb) + (uint)value;
-			        }
-                }
-			    else
-			    {
-                    throw new ScriptRuntimeException("bad argument #2 to 'tonumber' (base out of range)");
-                }
-
-				return DynValue.NewNumber(uiv);
+				if (bb < 2 || bb > 36)
+					throw new ScriptRuntimeException("bad argument #2 to 'tonumber' (base out of range)");
+
+				return StringToNumberInBase(ee.String, bb);
 			}
 		}
 
+		// Integer numeral parsing for tonumber(e, base), following standard Lua semantics:
+		// optional surrounding whitespace, optional '-' sign, then one or more digits
+		// (0-9, a-z/A-Z for 10-35) all below the base. Any other input yields nil — never
+		// a CLR exception, which would escape the interpreter as a non-Lua error.
+		private static DynValue StringToNumberInBase(string str, int numBase)
+		{
+			// A string DynValue can wrap a null (e.g. host-created DynValue.NewString(null));
+			// treat it as unparseable rather than dereferencing it into a NullReferenceException.
+			if (str == null)
+				return DynValue.Nil;
+
+			string s = str.Trim();
+			int i = 0;
+			bool neg = false;
+
+			if (i < s.Length && s[i] == '-')
+			{
+				neg = true;
+				i++;
+			}
+
+			if (i >= s.Length)
+				return DynValue.Nil;
+
+			double value = 0.0;
+
+			for (; i < s.Length; i++)
+			{
+				char c = s[i];
+				int digit;
+
+				if (c >= '0' && c <= '9')
+					digit = c - '0';
+				else if (c >= 'a' && c <= 'z')
+					digit = c - 'a' + 10;
+				else if (c >= 'A' && c <= 'Z')
+					digit = c - 'A' + 10;
+				else
+					return DynValue.Nil;
+
+				if (digit >= numBase)
+					return DynValue.Nil;
+
+				value = value * numBase + digit;
+			}
+
+			return DynValue.NewNumber(neg ? -value : value);
+		}
+
 		[MoonSharpModuleMethod]
 		public static DynValue print(ScriptExecutionContext executionContext, CallbackArguments args)
 		{