From d55733c76ee1db702529f38f602548ffe48a4ab1 Mon Sep 17 00:00:00 2001
From: Adam Greene <adam.greene@gmail.com>
Date: Fri, 1 May 2015 11:54:29 -0700
Subject: return nil if array index indicie is out of bounds

ruby arrays don't throw an exception; they return nil.  Lets do the
same!

this fix also includes the ability to use negative array indicies
---
 .../com/google/protobuf/jruby/RubyMessage.java     | 14 ++++++-------
 .../google/protobuf/jruby/RubyRepeatedField.java   | 24 ++++++++++++++++------
 2 files changed, 25 insertions(+), 13 deletions(-)

(limited to 'ruby/src/main')

diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java
index 04bc0b76..20e825e2 100644
--- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java
+++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java
@@ -246,16 +246,15 @@ public class RubyMessage extends RubyObject {
     public IRubyObject dup(ThreadContext context) {
         RubyMessage dup = (RubyMessage) metaClass.newInstance(context, Block.NULL_BLOCK);
         IRubyObject value;
-        for (Descriptors.FieldDescriptor fieldDescriptor : builder.getAllFields().keySet()) {
+        for (Descriptors.FieldDescriptor fieldDescriptor : this.descriptor.getFields()) {
             if (fieldDescriptor.isRepeated()) {
-                dup.repeatedFields.put(fieldDescriptor, getRepeatedField(context, fieldDescriptor));
-            } else if (builder.hasField(fieldDescriptor)) {
-                dup.fields.put(fieldDescriptor, wrapField(context, fieldDescriptor, builder.getField(fieldDescriptor)));
+                dup.addRepeatedField(fieldDescriptor, this.getRepeatedField(context, fieldDescriptor));
+            } else if (fields.containsKey(fieldDescriptor)) {
+                dup.fields.put(fieldDescriptor, fields.get(fieldDescriptor));
+            } else if (this.builder.hasField(fieldDescriptor)) {
+                dup.fields.put(fieldDescriptor, wrapField(context, fieldDescriptor, this.builder.getField(fieldDescriptor)));
             }
         }
-        for (Descriptors.FieldDescriptor fieldDescriptor : fields.keySet()) {
-            dup.fields.put(fieldDescriptor, fields.get(fieldDescriptor));
-        }
         for (Descriptors.FieldDescriptor fieldDescriptor : maps.keySet()) {
             dup.maps.put(fieldDescriptor, maps.get(fieldDescriptor));
         }
@@ -411,6 +410,7 @@ public class RubyMessage extends RubyObject {
         for (int i = 0; i < count; i++) {
             ret.push(context, wrapField(context, fieldDescriptor, this.builder.getRepeatedField(fieldDescriptor, i)));
         }
+        addRepeatedField(fieldDescriptor, ret);
         return ret;
     }
 
diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java
index 9788317a..84bf8956 100644
--- a/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java
+++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java
@@ -108,8 +108,9 @@ public class RubyRepeatedField extends RubyObject {
      */
     @JRubyMethod(name = "[]=")
     public IRubyObject indexSet(ThreadContext context, IRubyObject index, IRubyObject value) {
+        int arrIndex = normalizeArrayIndex(index);
         Utils.checkType(context, fieldType, value, (RubyModule) typeClass);
-        this.storage.set(RubyNumeric.num2int(index), value);
+        this.storage.set(arrIndex, value);
         return context.runtime.getNil();
     }
 
@@ -117,12 +118,15 @@ public class RubyRepeatedField extends RubyObject {
      * call-seq:
      *     RepeatedField.[](index) => value
      *
-     * Accesses the element at the given index. Throws an exception on out-of-bounds
-     * errors.
+     * Accesses the element at the given index. Returns nil on out-of-bounds
      */
     @JRubyMethod(name = "[]")
     public IRubyObject index(ThreadContext context, IRubyObject index) {
-        return this.storage.eltInternal(RubyNumeric.num2int(index));
+        int arrIndex = normalizeArrayIndex(index);
+        if (arrIndex < 0 || arrIndex >= this.storage.size()) {
+            return context.runtime.getNil();
+        }
+        return this.storage.eltInternal(arrIndex);
     }
 
     /*
@@ -134,8 +138,7 @@ public class RubyRepeatedField extends RubyObject {
     @JRubyMethod(rest = true)
     public IRubyObject insert(ThreadContext context, IRubyObject[] args) {
         for (int i = 0; i < args.length; i++) {
-            Utils.checkType(context, fieldType, args[i], (RubyModule) typeClass);
-            this.storage.add(args[i]);
+            push(context, args[i]);
         }
         return context.runtime.getNil();
     }
@@ -385,6 +388,15 @@ public class RubyRepeatedField extends RubyObject {
         }
     }
 
+    private int normalizeArrayIndex(IRubyObject index) {
+        int arrIndex = RubyNumeric.num2int(index);
+        int arrSize = this.storage.size();
+        if (arrIndex < 0 && arrSize > 0) {
+            arrIndex = arrSize + arrIndex;
+        }
+        return arrIndex;
+    }
+
     private RubyArray storage;
     private Descriptors.FieldDescriptor.Type fieldType;
     private IRubyObject typeClass;
-- 
cgit v1.2.3