Skip to content

Commit 43b6b6c

Browse files
authored
GH-759: Get length of byte[] in TryCopyLastError (#760)
## What's Changed We should get the length of byte[] by `GetArrayLength`, not `strlen` which may cause invalid memory access. Closes #759.
1 parent acbc138 commit 43b6b6c

File tree

2 files changed

+152
-1
lines changed

2 files changed

+152
-1
lines changed

c/src/main/cpp/jni_wrapper.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,9 @@ void TryCopyLastError(JNIEnv* env, InnerPrivateData* private_data) {
205205
return;
206206
}
207207

208+
jsize error_bytes_len = env->GetArrayLength(arr);
208209
char* error_str = reinterpret_cast<char*>(error_bytes);
209-
private_data->last_error_ = std::string(error_str, std::strlen(error_str));
210+
private_data->last_error_ = std::string(error_str, error_bytes_len);
210211

211212
env->ReleaseByteArrayElements(arr, error_bytes, JNI_ABORT);
212213
}
Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.arrow.c;
18+
19+
import static org.assertj.core.api.Assertions.assertThat;
20+
import static org.assertj.core.api.Assertions.catchThrowableOfType;
21+
22+
import java.io.IOException;
23+
import java.io.PrintWriter;
24+
import java.io.StringWriter;
25+
import java.util.ArrayList;
26+
import java.util.Collections;
27+
import java.util.List;
28+
import java.util.Map;
29+
import java.util.Set;
30+
import java.util.function.Function;
31+
import java.util.stream.Collectors;
32+
import org.apache.arrow.memory.BufferAllocator;
33+
import org.apache.arrow.memory.RootAllocator;
34+
import org.apache.arrow.vector.VectorLoader;
35+
import org.apache.arrow.vector.VectorSchemaRoot;
36+
import org.apache.arrow.vector.dictionary.Dictionary;
37+
import org.apache.arrow.vector.dictionary.DictionaryProvider;
38+
import org.apache.arrow.vector.ipc.ArrowReader;
39+
import org.apache.arrow.vector.ipc.message.ArrowRecordBatch;
40+
import org.apache.arrow.vector.types.pojo.ArrowType;
41+
import org.apache.arrow.vector.types.pojo.Field;
42+
import org.apache.arrow.vector.types.pojo.Schema;
43+
import org.junit.jupiter.api.Test;
44+
45+
// Regression test for https://github.com/apache/arrow-java/issues/759
46+
final class ExceptionTest {
47+
@Test
48+
public void testException() throws IOException {
49+
final Schema schema =
50+
new Schema(Collections.singletonList(Field.nullable("ints", new ArrowType.Int(32, true))));
51+
final List<Object> batches = new ArrayList<>();
52+
53+
try (BufferAllocator allocator = new RootAllocator();
54+
VectorSchemaRoot root = VectorSchemaRoot.create(schema, allocator)) {
55+
56+
final String exceptionMessage = "This is a message for testing exception.";
57+
58+
RuntimeException exToThrow = new RuntimeException(exceptionMessage);
59+
batches.add(exToThrow);
60+
61+
StringWriter sw = new StringWriter();
62+
PrintWriter pw = new PrintWriter(sw);
63+
exToThrow.printStackTrace(pw);
64+
final String expectExceptionMessage = sw.toString();
65+
66+
ArrowReader source = new ExceptionMemoryArrowReader(allocator, schema, batches);
67+
68+
try (final ArrowArrayStream stream = ArrowArrayStream.allocateNew(allocator);
69+
final VectorSchemaRoot importRoot = VectorSchemaRoot.create(schema, allocator)) {
70+
final VectorLoader loader = new VectorLoader(importRoot);
71+
Data.exportArrayStream(allocator, source, stream);
72+
73+
try (final ArrowReader reader = Data.importArrayStream(allocator, stream)) {
74+
IOException jniException = catchThrowableOfType(IOException.class, reader::loadNextBatch);
75+
final String jniMessage = jniException.getMessage();
76+
assertThat(jniMessage.endsWith(expectExceptionMessage + "}"));
77+
}
78+
}
79+
}
80+
}
81+
82+
static class ExceptionMemoryArrowReader extends ArrowReader {
83+
private final Schema schema;
84+
private final List<Object> batches; // set ArrowRecordBatch or Exception
85+
private final DictionaryProvider provider;
86+
private int nextBatch;
87+
88+
ExceptionMemoryArrowReader(BufferAllocator allocator, Schema schema, List<Object> batches) {
89+
super(allocator);
90+
this.schema = schema;
91+
this.batches = batches;
92+
this.provider = new CDataDictionaryProvider();
93+
this.nextBatch = 0;
94+
}
95+
96+
@Override
97+
public Dictionary lookup(long id) {
98+
return provider.lookup(id);
99+
}
100+
101+
@Override
102+
public Set<Long> getDictionaryIds() {
103+
return provider.getDictionaryIds();
104+
}
105+
106+
@Override
107+
public Map<Long, Dictionary> getDictionaryVectors() {
108+
return getDictionaryIds().stream()
109+
.collect(Collectors.toMap(Function.identity(), this::lookup));
110+
}
111+
112+
@Override
113+
public boolean loadNextBatch() throws IOException {
114+
if (nextBatch < batches.size()) {
115+
Object object = batches.get(nextBatch++);
116+
if (object instanceof RuntimeException) {
117+
throw (RuntimeException) object;
118+
}
119+
VectorLoader loader = new VectorLoader(getVectorSchemaRoot());
120+
loader.load((ArrowRecordBatch) object);
121+
return true;
122+
}
123+
return false;
124+
}
125+
126+
@Override
127+
public long bytesRead() {
128+
return 0;
129+
}
130+
131+
@Override
132+
protected void closeReadSource() throws IOException {
133+
try {
134+
for (Object object : batches) {
135+
if (object instanceof ArrowRecordBatch) {
136+
ArrowRecordBatch batch = (ArrowRecordBatch) object;
137+
batch.close();
138+
}
139+
}
140+
} catch (Exception e) {
141+
throw new IOException(e);
142+
}
143+
}
144+
145+
@Override
146+
protected Schema readSchema() {
147+
return schema;
148+
}
149+
}
150+
}

0 commit comments

Comments
 (0)