Skip to content

Commit

Permalink
IGNITE-20586 .NET: Fix JNI string conversion causing node crash (#10985)
Browse files Browse the repository at this point in the history
When working with `jstring` in JNI, we used `GetStringUTFChars`, which returns Java-specific "modified UTF-8" characters, then passed those characters to `Encoding.UTF8.GetString` in .NET. However, "modified UTF-8" is not true UTF-8, and cannot be correctly decoded by .NET standard library; in some cases it can even cause `OverflowException` and crash the node.

The fix is to use `GetStringChars`, which returns UTF-16 characters without any quirks. Both Java and .NET use UTF-16 to represent strings internally, so this approach is also simpler and faster.

The fix (and the bug) does not affect user data serialization - JNI strings are only used for logging, console output, instance names, and config paths.
  • Loading branch information
ptupitsyn authored Oct 10, 2023
1 parent ceb22d2 commit aa75904
Show file tree
Hide file tree
Showing 9 changed files with 205 additions and 79 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.ignite.platform;

import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import org.apache.ignite.cluster.ClusterNode;
import org.apache.ignite.compute.ComputeJob;
import org.apache.ignite.compute.ComputeJobAdapter;
import org.apache.ignite.compute.ComputeJobResult;
import org.apache.ignite.compute.ComputeTaskAdapter;
import org.apache.ignite.internal.util.typedef.F;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

/**
* Task to test Java console output.
*/
public class PlatformConsoleWriteTask extends ComputeTaskAdapter<byte[], Object> {
/** {@inheritDoc} */
@NotNull @Override public Map<? extends ComputeJob, ClusterNode> map(List<ClusterNode> subgrid,
@Nullable byte[] arg) {
return Collections.singletonMap(new Job(arg), F.first(subgrid));
}

/** {@inheritDoc} */
@Nullable @Override public String reduce(List<ComputeJobResult> results) {
return results.get(0).getData();
}

/**
* Job.
*/
private static class Job extends ComputeJobAdapter {
/** */
private final byte[] arg;

/**
* Ctor.
*
* @param arg arg.
*/
private Job(byte[] arg) {
this.arg = arg;
}

/** {@inheritDoc} */
@Nullable @Override public String execute() {
String str = new String(arg, StandardCharsets.UTF_16LE);
System.out.println(str);

return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,13 @@ public class PlatformTestUtils {
public static int majorJavaVersion() {
return U.majorJavaVersion(System.getProperty("java.version"));
}

/**
* Prints line to standard output.
*
* @param str String.
*/
public static void println(String str) {
System.out.println(str);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ public class BinarySelfTest {
new string(new[] {(char) 0xD800, '的', (char) 0xD800, (char) 0xD800, (char) 0xDC00, (char) 0xDFFF}),
"ascii0123456789",
"的的abcdкириллица",
new string(new[] {(char) 0xD801, (char) 0xDC37})
new string(new[] {(char) 0xD801, (char) 0xDC37}),
"Ḽơᶉëᶆ ȋṕšᶙṁ",
"A_\ud83e\udd26\ud83c\udffc\u200d\u2642\ufe0f_B" // A_🤦🏼‍♂️_B
};

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,17 @@ namespace Apache.Ignite.Core.Tests
using System.Text.RegularExpressions;
using Apache.Ignite.Core.Common;
using Apache.Ignite.Core.Communication.Tcp;
using Apache.Ignite.Core.Tests.Binary;
using NUnit.Framework;

/// <summary>
/// Tests that Java console output is redirected to .NET console.
/// </summary>
public class ConsoleRedirectTest
{
/** Thread name task name. */
private const string ConsoleWriteTask = "org.apache.ignite.platform.PlatformConsoleWriteTask";

/** */
private StringBuilder _outSb;

Expand Down Expand Up @@ -101,6 +105,35 @@ public void TestExceptionInWriterPropagatesToJavaAndBack()
Assert.AreEqual("foo", ex.Message);
}

[Test]
public void TestConsoleWriteSpecialStrings()
{
var ignite = Ignition.Start(TestUtils.GetTestConfiguration());

foreach (var val in BinarySelfTest.SpecialStrings)
{
MyStringWriter.LastValue = null;

// Send to Java as UTF-16 to avoid dealing with IGNITE_BINARY_MARSHALLER_USE_STRING_SERIALIZATION_VER_2
var bytes = Encoding.Unicode.GetBytes(MyStringWriter.Prefix + val);
ignite.GetCompute().ExecuteJavaTask<string>(ConsoleWriteTask, bytes);

var expectedStr = GetExpectedStr(val);
Assert.AreEqual(expectedStr, MyStringWriter.LastValue, message: val);
StringAssert.Contains(expectedStr, _outSb.ToString(), message: val);

// Test Env.NewString
MyStringWriter.LastValue = null;
TestUtilsJni.Println(MyStringWriter.Prefix + val);

Assert.AreEqual(expectedStr.Length, MyStringWriter.LastValue?.Length, message: val);
if (val != BinarySelfTest.SpecialStrings[0])
{
Assert.AreEqual(expectedStr, MyStringWriter.LastValue, message: val);
}
}
}

/// <summary>
/// Tests startup error in Java.
/// </summary>
Expand Down Expand Up @@ -239,10 +272,26 @@ public void Run()
}
#endif

private static string GetExpectedStr(string val)
{
if (val != BinarySelfTest.SpecialStrings[0])
{
return val;
}

// Some special strings are not equal to themselves after UTF16 roundtrip,
// even though they contain exactly the same bytes.
return Encoding.Unicode.GetString(Encoding.Unicode.GetBytes(val));
}

private class MyStringWriter : StringWriter
{
public const string Prefix = "[MyStringWriter]";

public static bool Throw { get; set; }

public static string LastValue { get; set; }

public MyStringWriter(StringBuilder sb) : base(sb)
{
// No-op.
Expand All @@ -256,6 +305,11 @@ public override void Write(string value)
}

base.Write(value);

if (!string.IsNullOrWhiteSpace(value) && value.StartsWith(Prefix))
{
LastValue = value.Substring(Prefix.Length);
}
}
}
}
Expand Down
19 changes: 12 additions & 7 deletions modules/platforms/dotnet/Apache.Ignite.Core.Tests/TestUtilsJni.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ public static unsafe void StartProcess(
var methodId = env.GetStaticMethodId(cls, "startProcess",
"(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V");

using (var fileRef = env.NewStringUtf(file))
using (var arg1Ref = env.NewStringUtf(arg1))
using (var arg2Ref = env.NewStringUtf(arg2))
using (var envRef = env.NewStringUtf(envVars))
using (var workDirRef = env.NewStringUtf(workDir))
using (var waitForOutputRef = env.NewStringUtf(waitForOutput))
using (var fileRef = env.NewString(file))
using (var arg1Ref = env.NewString(arg1))
using (var arg2Ref = env.NewString(arg2))
using (var envRef = env.NewString(envVars))
using (var workDirRef = env.NewString(workDir))
using (var waitForOutputRef = env.NewString(waitForOutput))
{
var methodArgs = stackalloc long[6];
methodArgs[0] = fileRef.Target.ToInt64();
Expand Down Expand Up @@ -128,14 +128,19 @@ public static int GetJavaMajorVersion()
return CallIntMethod(ClassPlatformTestUtils, "majorJavaVersion", "()I");
}

public static void Println(string str)
{
CallStringMethod(ClassPlatformTestUtils, "println", "(Ljava/lang/String;)V", str);
}

/** */
private static unsafe void CallStringMethod(string className, string methodName, string methodSig, string arg)
{
var env = Jvm.Get().AttachCurrentThread();
using (var cls = env.FindClass(className))
{
var methodId = env.GetStaticMethodId(cls, methodName, methodSig);
using (var gridNameRef = env.NewStringUtf(arg))
using (var gridNameRef = env.NewString(arg))
{
var args = stackalloc long[1];
args[0] = gridNameRef.Target.ToInt64();
Expand Down
21 changes: 0 additions & 21 deletions modules/platforms/dotnet/Apache.Ignite.Core/Impl/IgniteUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ namespace Apache.Ignite.Core.Impl
using Apache.Ignite.Core.Impl.Binary;
using Apache.Ignite.Core.Impl.Cluster;
using Apache.Ignite.Core.Impl.Common;
using Apache.Ignite.Core.Impl.Compute;
using BinaryReader = Apache.Ignite.Core.Impl.Binary.BinaryReader;

/// <summary>
Expand Down Expand Up @@ -128,26 +127,6 @@ private static void SetProperties(object target, IEnumerable<KeyValuePair<string
}
}

/// <summary>
/// Convert unmanaged char array to string.
/// </summary>
/// <param name="chars">Char array.</param>
/// <param name="charsLen">Char array length.</param>
/// <returns></returns>
public static unsafe string Utf8UnmanagedToString(sbyte* chars, int charsLen)
{
IntPtr ptr = new IntPtr(chars);

if (ptr == IntPtr.Zero)
return null;

byte[] arr = new byte[charsLen];

Marshal.Copy(ptr, arr, 0, arr.Length);

return Encoding.UTF8.GetString(arr);
}

/// <summary>
/// Convert string to unmanaged byte array.
/// </summary>
Expand Down
Loading

0 comments on commit aa75904

Please sign in to comment.