Repository: jpype-project/jpype (language: python)

Issue:
String concatenation with JChar causes segfault
When I attempt to use the `+` operator to concatenate a JChar and a Python string I get a seg fault:

``` python
In [1]: import jpype

In [2]: jpype.__version__
Out[2]: '1.3.0'

In [3]: jpype.startJVM()

In [4]: jpype.JChar("x")
Out[4]: 'x'

In [5]: jpype.JChar("x") + "test"
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x0000000000000000, pid=11156, tid=11156
#
# JRE version: OpenJDK Runtime Environment 21.9 (17.0.2+8) (build 17.0.2+8)
# Java VM: OpenJDK 64-Bit Server VM 21.9 (17.0.2+8, mixed mode, sharing, tiered, compressed oops, compressed class ptrs, g1 gc, linux-amd64)
# Problematic frame:
# C  0x0000000000000000
#
# Core dump will be written. Default location: Core dumps may be processed with "/usr/lib/systemd/systemd-coredump %P %u %g %s %t %c %h" (or dumping to /home/jim/Projects/ITP/pythonprocessing/py5generator/core.11156)
#
# An error report file with more information is saved as:
# /home/jim/Projects/ITP/pythonprocessing/py5generator/hs_err_pid11156.log
#
# If you would like to submit a bug report, please visit:
#   https://bugzilla.redhat.com/enter_bug.cgi?product=Fedora&component=java-17-openjdk&version=34
# The crash happened outside the Java Virtual Machine in native code.
# See problematic frame for where to report the bug.
#
Aborted (core dumped)
```

Reversing the order of the operands gives a `TypeError: cast of null pointer` exception.

The workaround in both cases is to use `str`:

``` python
In [5]: "test" + str(jpype.JChar("x"))
Out[5]: 'testx'

In [6]: str(jpype.JChar("x")) + "test"
Out[6]: 'xtest'
```

There may be a good reason why that operation is not allowed and using `str` is a simple enough workaround, but I wouldn't expect this to trigger a segfault.

My computer is using Fedora Linux, python 3.8.12, and OpenJDK Java 17



Discussion / hints:
Definitely something requiring repair.

Testing 
"foo"+JChar("x") => works "foox"
JChar("x")+"foo" => fails segfault
JChar("x") + 1 => works 121
1 + JChar("x") => fails "null pointer"
"foo" + JChar("x") => fails "null pointer"
JChar(1)+JChar("x") => fails
JChar('x')-1 => works
1-JChar('x') => fails


It appears that the code is calling a bad slot and it also does not consider that Python would call add with the argument order reversed.    I will have to check this for every possible operation with jchar.   Should be fixed shortly.



Fix patch:
diff --git a/native/python/pyjp_char.cpp b/native/python/pyjp_char.cpp
index 0adfdf892..5b4ab19da 100644
--- a/native/python/pyjp_char.cpp
+++ b/native/python/pyjp_char.cpp
@@ -63,11 +63,17 @@ static int isNull(JPValue *javaSlot)
 	return 1;
 }
 
+static PyObject* notSupported()
+{
+	PyErr_SetString(PyExc_TypeError, "unsupported operation");
+	return 0;
+}
+
 static int assertNotNull(JPValue *javaSlot)
 {
 	if (!isNull(javaSlot))
 		return 0;
-	PyErr_SetString(PyExc_TypeError, "cast of null pointer");
+	PyErr_SetString(PyExc_TypeError, "jchar cast of null pointer");
 	return 1;
 }
 
@@ -295,158 +301,152 @@ static Py_ssize_t PyJPChar_len(PyJPChar *self)
 	JP_PY_CATCH(-1);  // GCOVR_EXCL_LINE
 }
 
-static  PyObject *PyJPChar_and(PyJPChar *self, PyObject *other)
+static PyObject *apply(PyObject *first, PyObject *second, PyObject* (*func)(PyObject*, PyObject*))
+{
+	JPValue *slot0 = PyJPValue_getJavaSlot(first);
+	JPValue *slot1 = PyJPValue_getJavaSlot(second);
+	if (slot0 != NULL && slot1 != NULL)
+	{	
+		if (assertNotNull(slot0))
+			return 0;
+		if (assertNotNull(slot1))
+			return 0;
+		JPPyObject v1 = JPPyObject::call(PyLong_FromLong(fromJPChar((PyJPChar*)first)));
+		JPPyObject v2 = JPPyObject::call(PyLong_FromLong(fromJPChar((PyJPChar*)second)));
+		return func(v1.get(), v2.get());
+	}
+	else if (slot0 != NULL)
+	{
+		if (assertNotNull(slot0))
+			return 0;
+		// Promote to int as per Java rules
+		JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar((PyJPChar*)first)));
+		return func(v.get(), second);
+	}
+	else if (slot1 != NULL)
+	{
+		if (assertNotNull(slot1))
+			return 0;
+		// Promote to int as per Java rules
+		JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar((PyJPChar*)second)));
+		return func(first, v.get());
+	}
+	return notSupported();
+}
+
+
+static  PyObject *PyJPChar_and(PyObject *first, PyObject *second)
 {
 	JP_PY_TRY("PyJPChar_and");
 	PyJPModule_getContext();  // Check that JVM is running
-	JPValue *javaSlot = PyJPValue_getJavaSlot((PyObject*) self);
-	if (assertNotNull(javaSlot))
-		return 0;
-	// Promote to int as per Java rules
-	JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar(self)));
-	return PyNumber_And(v.get(), other);
+	return apply(first, second, PyNumber_And);
 	JP_PY_CATCH(NULL);  // GCOVR_EXCL_LINE
 }
 
-static  PyObject *PyJPChar_or(PyJPChar *self, PyObject *other)
+static  PyObject *PyJPChar_or(PyObject *first, PyObject *second)
 {
 	JP_PY_TRY("PyJPChar_or");
 	PyJPModule_getContext();  // Check that JVM is running
-	JPValue *javaSlot = PyJPValue_getJavaSlot((PyObject*) self);
-	if (assertNotNull(javaSlot))
-		return 0;
-	// Promote to int as per Java rules
-	JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar(self)));
-	return PyNumber_Or(v.get(), other);
+	return apply(first, second, PyNumber_Or);
 	JP_PY_CATCH(NULL);  // GCOVR_EXCL_LINE
 }
 
-static  PyObject *PyJPChar_xor(PyJPChar *self, PyObject *other)
+static  PyObject *PyJPChar_xor(PyObject *first, PyObject *second)
 {
 	JP_PY_TRY("PyJPChar_xor");
 	PyJPModule_getContext();  // Check that JVM is running
-	JPValue *javaSlot = PyJPValue_getJavaSlot((PyObject*) self);
-	if (assertNotNull(javaSlot))
-		return 0;
-	// Promote to int as per Java rules
-	JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar(self)));
-	return PyNumber_Xor(v.get(), other);
+	return apply(first, second, PyNumber_Xor);
 	JP_PY_CATCH(NULL);  // GCOVR_EXCL_LINE
 }
 
-static  PyObject *PyJPChar_add(PyJPChar *self, PyObject *other)
+static  PyObject *PyJPChar_add(PyObject *first, PyObject *second)
 {
 	JP_PY_TRY("PyJPChar_add");
 	PyJPModule_getContext();  // Check that JVM is running
-	JPValue *javaSlot = PyJPValue_getJavaSlot((PyObject*) self);
-	if (assertNotNull(javaSlot))
-		return 0;
-	if (PyUnicode_Check(other))
-		return PyUnicode_Type.tp_as_number->nb_add((PyObject*) self, other);
+	JPValue *slot0 = PyJPValue_getJavaSlot(first);
+	JPValue *slot1 = PyJPValue_getJavaSlot(second);
+	if (slot1 != NULL && slot0 != NULL)
+	{	
+		if (assertNotNull(slot0))
+			return 0;
+		if (assertNotNull(slot1))
+			return 0;
+		JPPyObject v1 = JPPyObject::call(PyLong_FromLong(fromJPChar((PyJPChar*)first)));
+		JPPyObject v2 = JPPyObject::call(PyLong_FromLong(fromJPChar((PyJPChar*)second)));
+		return PyNumber_Add(v1.get(), v2.get());
+	}
+	else if (slot0 != NULL)
+	{
+		if (assertNotNull(slot0))
+			return 0;
+		if (PyUnicode_Check(second))
+			return PyUnicode_Concat(first, second);
 
-	// Promote to int as per Java rules
-	JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar(self)));
-	return PyNumber_Add(v.get(), other);
+		// Promote to int as per Java rules
+		JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar((PyJPChar*)first)));
+		return PyNumber_Add(v.get(), second);
+	}
+	else if (slot1 != NULL)
+	{
+		if (assertNotNull(slot1))
+			return 0;
+		if (PyUnicode_Check(first))
+			return PyUnicode_Concat(first, second);
+
+		// Promote to int as per Java rules
+		JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar((PyJPChar*)second)));
+		return PyNumber_Add(first, v.get());
+	}
+	return notSupported();
 	JP_PY_CATCH(NULL);  // GCOVR_EXCL_LINE
 }
 
-static  PyObject *PyJPChar_subtract(PyJPChar *self, PyObject *other)
+
+static  PyObject *PyJPChar_subtract(PyObject *first, PyObject *second)
 {
 	JP_PY_TRY("PyJPChar_subtract");
 	PyJPModule_getContext();  // Check that JVM is running
-	JPValue *javaSlot = PyJPValue_getJavaSlot((PyObject*) self);
-	if (assertNotNull(javaSlot))
-		return 0;
-
-	// Promote to int as per Java rules
-	JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar(self)));
-	return PyNumber_Subtract(v.get(), other);
+	return apply(first, second, PyNumber_Subtract);
 	JP_PY_CATCH(NULL);  // GCOVR_EXCL_LINE
 }
 
-static  PyObject *PyJPChar_mult(PyJPChar *self, PyObject *other)
+static  PyObject *PyJPChar_mult(PyObject *first, PyObject *second)
 {
 	JP_PY_TRY("PyJPChar_mult");
 	PyJPModule_getContext();  // Check that JVM is running
-	JPValue *javaSlot = PyJPValue_getJavaSlot((PyObject*) self);
-	if (assertNotNull(javaSlot))
-		return 0;
-
-	// Promote to int as per Java rules
-	JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar(self)));
-	return PyNumber_Multiply(v.get(), other);
+	return apply(first, second, PyNumber_Multiply);
 	JP_PY_CATCH(NULL);  // GCOVR_EXCL_LINE
 }
 
-static  PyObject *PyJPChar_rshift(PyJPChar *self, PyObject *other)
+static  PyObject *PyJPChar_rshift(PyObject *first, PyObject *second)
 {
 	JP_PY_TRY("PyJPChar_rshift");
 	PyJPModule_getContext();  // Check that JVM is running
-	JPValue *javaSlot = PyJPValue_getJavaSlot((PyObject*) self);
-	if (assertNotNull(javaSlot))
-		return 0;
-
-	// Promote to int as per Java rules
-	JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar(self)));
-	return PyNumber_Rshift(v.get(), other);
+	return apply(first, second, PyNumber_Rshift);
 	JP_PY_CATCH(NULL);  // GCOVR_EXCL_LINE
 }
 
-static  PyObject *PyJPChar_lshift(PyJPChar *self, PyObject *other)
+static  PyObject *PyJPChar_lshift(PyObject *first, PyObject *second)
 {
 	JP_PY_TRY("PyJPChar_lshift");
 	PyJPModule_getContext();  // Check that JVM is running
-	JPValue *javaSlot = PyJPValue_getJavaSlot((PyObject*) self);
-	if (assertNotNull(javaSlot))
-		return 0;
-
-	// Promote to int as per Java rules
-	JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar(self)));
-	return PyNumber_Lshift(v.get(), other);
+	return apply(first, second, PyNumber_Lshift);
 	JP_PY_CATCH(NULL);  // GCOVR_EXCL_LINE
 }
 
-static  PyObject *PyJPChar_floordiv(PyObject *self, PyObject *other)
+static  PyObject *PyJPChar_floordiv(PyObject *first, PyObject *second)
 {
 	JP_PY_TRY("PyJPChar_floordiv");
 	PyJPModule_getContext();  // Check that JVM is running
-	JPValue *javaSlot = PyJPValue_getJavaSlot(self);
-	if (javaSlot == NULL)
-	{
-		javaSlot = PyJPValue_getJavaSlot(other);
-		if (assertNotNull(javaSlot))
-			return 0;
-		JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar((PyJPChar*) other)));
-		return PyNumber_FloorDivide(self, v.get());
-	}
-	if (assertNotNull(javaSlot))
-		return 0;
-
-	// Promote to int as per Java rules
-	JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar((PyJPChar*) self)));
-	return PyNumber_FloorDivide(v.get(), other);
+	return apply(first, second, PyNumber_FloorDivide);
 	JP_PY_CATCH(NULL);  // GCOVR_EXCL_LINE
 }
 
-static  PyObject *PyJPChar_divmod(PyObject *self, PyObject *other)
+static  PyObject *PyJPChar_divmod(PyObject *first, PyObject *second)
 {
 	JP_PY_TRY("PyJPChar_divmod");
 	PyJPModule_getContext();  // Check that JVM is running
-	JPValue *javaSlot = PyJPValue_getJavaSlot( self);
-	if (javaSlot == NULL)
-	{
-		javaSlot = PyJPValue_getJavaSlot(other);
-		if (assertNotNull(javaSlot))
-			return 0;
-		JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar((PyJPChar*) other)));
-		return PyNumber_Divmod(self, v.get());
-	}
-	if (assertNotNull(javaSlot))
-		return 0;
-
-	// Promote to int as per Java rules
-	JPPyObject v = JPPyObject::call(PyLong_FromLong(fromJPChar((PyJPChar*) self)));
-	return PyNumber_Divmod(v.get(), other);
+	return apply(first, second, PyNumber_Divmod);
 	JP_PY_CATCH(NULL);  // GCOVR_EXCL_LINE
 }
 


Test patch:
diff --git a/test/jpypetest/test_jchar.py b/test/jpypetest/test_jchar.py
index cda4817da..410dbc7a5 100644
--- a/test/jpypetest/test_jchar.py
+++ b/test/jpypetest/test_jchar.py
@@ -472,3 +472,22 @@ def testEq(self):
     def testPass(self):
         fixture = jpype.JClass('jpype.common.Fixture')()
         self.assertEqual(fixture.callObject(self.nc), None)
+
+
+class JCharTestCase(common.JPypeTestCase):
+
+    def testOps(self):
+        self.assertEqual(JChar("x") + "test", "xtest")
+        self.assertEqual("test" + JChar("x"), "testx")
+        self.assertEqual(JChar("x") + 1, 121)
+        self.assertEqual(1 + JChar("x"), 121)
+        self.assertEqual(JChar(1) + JChar("x"), 121)
+        self.assertEqual(JChar("x") - 1, 119)
+        self.assertEqual(1 - JChar("x"), -119)
+        self.assertEqual(JChar("x") - JChar(1), 119)
+        self.assertEqual(JChar("x") * 1, 120)
+        self.assertEqual(1 * JChar("x"), 120)
+        self.assertEqual(JChar("x") & 1, 0)
+        self.assertEqual(1 & JChar("x"), 0)
+        self.assertEqual(JChar("x") | 1, 121)
+        self.assertEqual(1 | JChar("x"), 121)