Skip to content

Commit edd18dd

Browse files
kareskares
committedFeb 6, 2018
[ji] avoid NPE when Java collection is not clone-able (the "usual" way)
GH-4721
·
9.4.12.09.1.16.0
1 parent 5cdbe74 commit edd18dd

File tree

5 files changed

+96
-6
lines changed

5 files changed

+96
-6
lines changed
 

‎core/src/main/java/org/jruby/javasupport/ext/JavaUtil.java

+8-6
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,8 @@ public static IRubyObject op_minus(final ThreadContext context, final IRubyObjec
226226
@JRubyMethod
227227
public static IRubyObject dup(final ThreadContext context, final IRubyObject self) {
228228
java.util.Collection coll = unwrapIfJavaObject(self);
229-
final JavaProxy dup = (JavaProxy) self.dup();
230-
if ( coll == dup.getObject() ) { // not Cloneable
229+
final JavaProxy dup = (JavaProxy) ((RubyBasicObject) self).dup();
230+
if ( coll == dup.getObject() && ! (coll instanceof Cloneable) ) {
231231
dup.setObject( tryNewEqualInstance(coll) );
232232
}
233233
return dup;
@@ -236,8 +236,8 @@ public static IRubyObject dup(final ThreadContext context, final IRubyObject sel
236236
@JRubyMethod
237237
public static IRubyObject clone(final ThreadContext context, final IRubyObject self) {
238238
java.util.Collection coll = unwrapIfJavaObject(self);
239-
final JavaProxy dup = (JavaProxy) self.rbClone();
240-
if ( coll == dup.getObject() ) { // not Cloneable
239+
final JavaProxy dup = (JavaProxy) ((RubyBasicObject) self).rbClone();
240+
if ( coll == dup.getObject() && ! (coll instanceof Cloneable) ) {
241241
dup.setObject( tryNewEqualInstance(coll) );
242242
}
243243
return dup;
@@ -608,8 +608,10 @@ private static java.util.Collection tryNewEqualInstance(final java.util.Collecti
608608
}
609609
}
610610
}
611-
if ( CAN_SET_ACCESSIBLE ) best.setAccessible(true);
612-
return (java.util.Collection) best.newInstance(coll);
611+
if ( best != null ) {
612+
if ( CAN_SET_ACCESSIBLE ) best.setAccessible(true);
613+
return (java.util.Collection) best.newInstance(coll);
614+
}
613615
}
614616
catch (IllegalAccessException e) {
615617
// fallback on getConstructor();

‎spec/java_integration/extensions/collection_spec.rb

+22
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,19 @@
139139
expect( arr.dup ).to be_a java.util.concurrent.LinkedBlockingQueue
140140
expect( arr.dup.poll ).to eq 42
141141
expect( arr.to_a ).to eql [42]
142+
143+
# immutable and Cloneable
144+
arr = Java::java_integration::fixtures::coll::CloneableImmutableList.new
145+
expect( arr.dup ).to equal arr
146+
147+
# non-Cloneable with public <init>()
148+
arr = Java::java_integration::fixtures::coll::NonCloneableList.new; arr.add(42)
149+
expect( arr.dup ).to eql arr
150+
expect( arr.dup ).to_not equal arr
151+
152+
# immutable and non-Cloneable
153+
# arr = Java::java_integration::fixtures::coll::NonCloneableImmutableList.new
154+
# expect( arr.dup ).to equal arr
142155
end
143156

144157
it 'clones' do
@@ -157,6 +170,15 @@
157170
expect( set.clone ).to be_a java.util.concurrent.CopyOnWriteArraySet
158171
set.clone.add '1'
159172
expect( set.to_a ).to eql ['0']
173+
174+
# immutable and Cloneable
175+
arr = Java::java_integration::fixtures::coll::CloneableImmutableList.new
176+
expect( arr.clone ).to equal arr
177+
178+
# non-Cloneable with public <init>()
179+
arr = Java::java_integration::fixtures::coll::NonCloneableList.new; arr.add(42)
180+
expect( arr.clone ).to eql arr
181+
expect( arr.clone ).to_not equal arr
160182
end
161183

162184
it '#include?' do

‎spec/java_integration/fixtures/coll/CloneableImmutableList.java

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package java_integration.fixtures.coll;
2+
3+
import java.util.*;
4+
5+
public final class CloneableImmutableList extends AbstractList<Object> implements Cloneable {
6+
private final List<Object> storage = Arrays.asList(1, 2, 3);
7+
8+
@Override
9+
public int size() {
10+
return storage.size();
11+
}
12+
13+
@Override
14+
public Object get(int index) {
15+
return storage.get(index);
16+
}
17+
18+
@Override
19+
public Object clone() {
20+
return this;
21+
}
22+
}

‎spec/java_integration/fixtures/coll/NonCloneableImmutableList.java

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package java_integration.fixtures.coll;
2+
3+
import java.util.*;
4+
5+
public final class NonCloneableImmutableList extends AbstractList<Object> {
6+
private final List<Object> storage = Arrays.asList(1, 2, 3);
7+
8+
@Override
9+
public int size() {
10+
return storage.size();
11+
}
12+
13+
@Override
14+
public Object get(int index) {
15+
return storage.get(index);
16+
}
17+
}

‎spec/java_integration/fixtures/coll/NonCloneableList.java

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package java_integration.fixtures.coll;
2+
3+
import java.util.*;
4+
5+
public class NonCloneableList extends AbstractList<Object> {
6+
7+
final List<Object> storage = new ArrayList();
8+
9+
public NonCloneableList() {} // newInstance + addAll works for dup
10+
11+
@Override
12+
public int size() {
13+
return storage.size();
14+
}
15+
16+
@Override
17+
public Object get(int index) {
18+
return storage.get(index);
19+
}
20+
21+
@Override
22+
public boolean add(Object e) {
23+
storage.add(e);
24+
return true;
25+
}
26+
27+
}

0 commit comments

Comments
 (0)
Please sign in to comment.