Objective-C API: Fix insertion of values greater than the max index a…

…llowed by the spec

https://bugs.webkit.org/show_bug.cgi?id=108264

Reviewed by Oliver Hunt.

Fixed a bug, added a test to the API tests, cleaned up some code.

* API/JSValue.h: Changed some of the documentation on setValue:atIndex: to indicate that
setting values at indices greater than UINT_MAX - 1 wont' affect the length of JS arrays.
* API/JSValue.mm:
(-[JSValue valueAtIndex:]): We weren't returning when we should have been.
(-[JSValue setValue:atIndex:]): Added a comment about why we do the early check for being larger than UINT_MAX.
(objectToValueWithoutCopy): Removed two redundant cases that were already checked previously.
* API/tests/testapi.mm:


Canonical link: https://commits.webkit.org/126713@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@141443 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/API/JSValue.h

@@ -159,9 +159,10 @@ NS_CLASS_AVAILABLE(10_9, NA)
 - (void)defineProperty:(NSString *)property descriptor:(id)descriptor;
 
 // Access an indexed property from the value. This method will return the
-// JavaScript value 'undefined' if no property exists at that index.
+// JavaScript value 'undefined' if no property exists at that index. 
 - (JSValue *)valueAtIndex:(NSUInteger)index;
-// Set an indexed property on the value.
+// Set an indexed property on the value. For JSValues that are JavaScript arrays, 
+// indices greater than UINT_MAX - 1 will not affect the length of the array.
 - (void)setValue:(id)value atIndex:(NSUInteger)index;
 
 // All JavaScript values are precisely one of these types.

Source/JavaScriptCore/API/JSValue.mm

@@ -274,8 +274,10 @@ - (void)defineProperty:(NSString *)property descriptor:(id)descriptor
 
 - (JSValue *)valueAtIndex:(NSUInteger)index
 {
+    // Properties that are higher than an unsigned value can hold are converted to a double then inserted as a normal property.
+    // Indices that are bigger than the max allowed index size (UINT_MAX - 1) will be handled internally in get().
     if (index != (unsigned)index)
-        [self valueForProperty:[[JSValue valueWithDouble:index inContext:_context] toString]];
+        return [self valueForProperty:[[JSValue valueWithDouble:index inContext:_context] toString]];
 
     JSValueRef exception = 0;
     JSObjectRef object = JSValueToObject(contextInternalContext(_context), m_value, &exception);
@@ -291,6 +293,8 @@ - (JSValue *)valueAtIndex:(NSUInteger)index
 
 - (void)setValue:(id)value atIndex:(NSUInteger)index
 {
+    // Properties that are higher than an unsigned value can hold are converted to a double, then inserted as a normal property.
+    // Indices that are bigger than the max allowed index size (UINT_MAX - 1) will be handled internally in putByIndex().
     if (index != (unsigned)index)
         return [self setValue:value forProperty:[[JSValue valueWithDouble:index inContext:_context] toString]];
 
@@ -864,12 +868,6 @@ inline bool isNSBoolean(id object)
         if ([object isKindOfClass:[JSValue class]])
             return (ObjcContainerConvertor::Task){ object, ((JSValue *)object)->m_value, ContainerNone };
 
-        if ([object isKindOfClass:[NSArray class]])
-            return (ObjcContainerConvertor::Task){ object, JSObjectMakeArray(contextRef, 0, NULL, 0), ContainerArray };
-
-        if ([object isKindOfClass:[NSDictionary class]])
-            return (ObjcContainerConvertor::Task){ object, JSObjectMake(contextRef, 0, 0), ContainerDictionary };
-
         if ([object isKindOfClass:[NSString class]]) {
             JSStringRef string = JSStringCreateWithCFString((CFStringRef)object);
             JSValueRef js = JSValueMakeString(contextRef, string);

Source/JavaScriptCore/API/tests/testapi.mm

@@ -256,6 +256,34 @@ void testObjectiveCAPI()
         checkResult(@"mulAddFunction", [result isObject] && [[result toString] isEqual:@"43,44,46"]);
     }
 
+    @autoreleasepool {
+        JSContext *context = [[[JSContext alloc] init] autorelease];        
+        JSValue *array = [JSValue valueWithNewArrayInContext:context];
+        checkResult(@"arrayLengthEmpty", [[array[@"length"] toNumber] unsignedIntegerValue] == 0);
+        JSValue *value1 = [JSValue valueWithInt32:42 inContext:context];
+        JSValue *value2 = [JSValue valueWithInt32:24 inContext:context];
+        NSUInteger lowIndex = 5;
+        NSUInteger maxLength = UINT_MAX;
+
+        [array setValue:value1 atIndex:lowIndex];
+        checkResult(@"array.length after put to low index", [[array[@"length"] toNumber] unsignedIntegerValue] == (lowIndex + 1));
+
+        [array setValue:value1 atIndex:(maxLength - 1)];
+        checkResult(@"array.length after put to maxLength - 1", [[array[@"length"] toNumber] unsignedIntegerValue] == maxLength);
+
+        [array setValue:value2 atIndex:maxLength];
+        checkResult(@"array.length after put to maxLength", [[array[@"length"] toNumber] unsignedIntegerValue] == maxLength);
+
+        [array setValue:value2 atIndex:(maxLength + 1)];
+        checkResult(@"array.length after put to maxLength + 1", [[array[@"length"] toNumber] unsignedIntegerValue] == maxLength);
+
+        checkResult(@"valueAtIndex:0 is undefined", [[array valueAtIndex:0] isUndefined]);
+        checkResult(@"valueAtIndex:lowIndex", [[array valueAtIndex:lowIndex] toInt32] == 42);
+        checkResult(@"valueAtIndex:maxLength - 1", [[array valueAtIndex:(maxLength - 1)] toInt32] == 42);
+        checkResult(@"valueAtIndex:maxLength", [[array valueAtIndex:maxLength] toInt32] == 24);
+        checkResult(@"valueAtIndex:maxLength + 1", [[array valueAtIndex:(maxLength + 1)] toInt32] == 24);
+    }
+
     @autoreleasepool {
         JSContext *context = [[[JSContext alloc] init] autorelease];
         JSValue *object = [JSValue valueWithNewObjectInContext:context];

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,20 @@
+2013-01-31  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        Objective-C API: Fix insertion of values greater than the max index allowed by the spec
+        https://bugs.webkit.org/show_bug.cgi?id=108264
+
+        Reviewed by Oliver Hunt.
+
+        Fixed a bug, added a test to the API tests, cleaned up some code.
+
+        * API/JSValue.h: Changed some of the documentation on setValue:atIndex: to indicate that 
+        setting values at indices greater than UINT_MAX - 1 wont' affect the length of JS arrays.
+        * API/JSValue.mm:
+        (-[JSValue valueAtIndex:]): We weren't returning when we should have been.
+        (-[JSValue setValue:atIndex:]): Added a comment about why we do the early check for being larger than UINT_MAX.
+        (objectToValueWithoutCopy): Removed two redundant cases that were already checked previously.
+        * API/tests/testapi.mm:
+
 2013-01-30  Andreas Kling  <akling@apple.com>
 
         Vector should consult allocator about ideal size when choosing capacity.