[ios, macos] Use std::vector for MGLMultiPoint coordinate storage (#6890)

Refactor the previous C array based implementation to use C++ std::vector. This is done to avoid potential safety and security issues with the previous approach.

platform/darwin/src/MGLMultiPoint.h

@@ -48,17 +48,26 @@ NS_ASSUME_NONNULL_BEGIN
  cause the shape to be redrawn if it is currently visible on the map.
  
  @param range The range of points to update. The `location` field indicates the
- first point you are replacing, with `0` being the first point, `1` being
- the second point, and so on. The `length` field indicates the number of
- points to update. You can append to an existing array of coordinates
- by specifying a range with a `location` at the end of the existing array 
- and/or a `length` which extends past the end of the existing array. The array 
- in _`coords`_ must be equal in number to the length of the range.
+    first point you are replacing, with `0` being the first point, `1` being
+    the second point, and so on. The `length` field indicates the number of
+    points to update. The array in _`coords`_ must be equal in number to the 
+    length of the range. If you want to append to the existing coordinates
+    array use `-[MGLMultiPoint appendCoordinates:count:]`.
  @param coords The array of coordinates defining the shape. The data in this
- array is copied to the object.
+    array is copied to the object.
  */
 - (void)replaceCoordinatesInRange:(NSRange)range withCoordinates:(CLLocationCoordinate2D *)coords;
 
+/**
+ Appends one or more coordinates for the shape, which will instantaneously
+ cause the shape to be redrawn if it is currently visible on the map.
+ 
+ @param coords The array of coordinates to add to the shape. The data in this
+    array is copied to the new object.
+ @param count The number of items in the `coords` array.
+ */
+- (void)appendCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count;
+
 @end
 
 NS_ASSUME_NONNULL_END

platform/darwin/src/MGLMultiPoint.mm

@@ -14,8 +14,8 @@ mbgl::Color MGLColorObjectFromCGColorRef(CGColorRef cgColor)
 
 @implementation MGLMultiPoint
 {
-    size_t _count;
     MGLCoordinateBounds _bounds;
+    std::vector<CLLocationCoordinate2D> _coordinates;
 }
 
 - (instancetype)initWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count
@@ -24,45 +24,23 @@ mbgl::Color MGLColorObjectFromCGColorRef(CGColorRef cgColor)
 
     if (self)
     {
-        [self setupWithCoordinates:coords count:count];
+        NSAssert(count > 0, @"A multipoint must have coordinates");
+        _coordinates = { coords, coords + count };
+        [self computeBounds];
     }
 
     return self;
 }
 
-- (void)setupWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count
-{
-    if (_coordinates) free(_coordinates);
-
-    _count = count;
-    _coordinates = (CLLocationCoordinate2D *)malloc(_count * sizeof(CLLocationCoordinate2D));
-
-    mbgl::LatLngBounds bounds = mbgl::LatLngBounds::empty();
-
-    for (NSUInteger i = 0; i < _count; i++)
-    {
-        _coordinates[i] = coords[i];
-        bounds.extend(mbgl::LatLng(coords[i].latitude, coords[i].longitude));
-    }
-
-    _bounds = MGLCoordinateBoundsFromLatLngBounds(bounds);
-}
-
-- (void)dealloc
-{
-    free(_coordinates);
-}
-
 - (CLLocationCoordinate2D)coordinate
 {
-    assert(_count > 0);
-
-    return CLLocationCoordinate2DMake(_coordinates[0].latitude, _coordinates[0].longitude);
+    NSAssert([self pointCount] > 0, @"A multipoint must have coordinates");
+    return _coordinates.at(0);
 }
 
 - (NSUInteger)pointCount
 {
-    return _count;
+    return _coordinates.size();
 }
 
 + (NS_SET_OF(NSString *) *)keyPathsForValuesAffectingPointCount
@@ -70,61 +48,61 @@ mbgl::Color MGLColorObjectFromCGColorRef(CGColorRef cgColor)
     return [NSSet setWithObjects:@"coordinates", nil];
 }
 
+- (CLLocationCoordinate2D *)coordinates
+{
+    return _coordinates.data();
+}
+
 - (void)getCoordinates:(CLLocationCoordinate2D *)coords range:(NSRange)range
 {
-    if (range.location + range.length > _count)
+    if (range.location + range.length > [self pointCount])
     {
         [NSException raise:NSRangeException
                     format:@"Invalid coordinate range %@ extends beyond current coordinate count of %zu",
-                        NSStringFromRange(range), _count];
+                        NSStringFromRange(range), [self pointCount]];
     }
 
-    NSUInteger index = 0;
+    std::copy(_coordinates.begin() + range.location, _coordinates.begin() + NSMaxRange(range), coords);
+}
 
-    for (NSUInteger i = range.location; i < range.location + range.length; i++)
-    {
-        coords[index] = _coordinates[i];
-        index++;
-    }
+- (void)appendCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count
+{
+    [self willChangeValueForKey:@"coordinates"];
+    _coordinates.insert(_coordinates.end(), count, *coords);
+    [self computeBounds];
+    [self didChangeValueForKey:@"coordinates"];
 }
 
 - (void)replaceCoordinatesInRange:(NSRange)range withCoordinates:(CLLocationCoordinate2D *)coords
 {
-    if ((coords >= _coordinates && coords < _coordinates + _count) ||
-        (coords + range.length >= _coordinates && coords + range.length < _coordinates + _count))
+    if (range.length == 0)
     {
-        [NSException raise:NSRangeException format:@"Reuse of existing coordinates array %p not supported", coords];
+        return;
     }
-    else if (range.length == 0)
-    {
-        [NSException raise:NSRangeException format:@"Empty coordinate range %@", NSStringFromRange(range)];
-    }
-    else if (range.location > _count)
+    
+    if (NSMaxRange(range) > _coordinates.size())
     {
         [NSException raise:NSRangeException
                     format:@"Invalid range %@ for existing coordinate count %zu",
-                        NSStringFromRange(range), _count];
+         NSStringFromRange(range), [self pointCount]];
     }
 
     [self willChangeValueForKey:@"coordinates"];
-    if (NSMaxRange(range) <= _count)
-    {
-        // replacing existing coordinate(s)
-        memcpy(_coordinates + range.location, coords, range.length * sizeof(CLLocationCoordinate2D));
-    }
-    else
-    {
-        // appending new coordinate(s)
-        NSUInteger newCount = NSMaxRange(range);
-        CLLocationCoordinate2D *newCoordinates = (CLLocationCoordinate2D *)malloc(newCount * sizeof(CLLocationCoordinate2D));
-        memcpy(newCoordinates, _coordinates, fmin(_count, range.location) * sizeof(CLLocationCoordinate2D));
-        memcpy(newCoordinates + range.location, coords, range.length * sizeof(CLLocationCoordinate2D));
-        [self setupWithCoordinates:newCoordinates count:newCount];
-        free(newCoordinates);
-    }
+    std::copy(coords, coords + range.length, _coordinates.begin() + range.location);
+    [self computeBounds];
     [self didChangeValueForKey:@"coordinates"];
 }
 
+- (void)computeBounds
+{
+    mbgl::LatLngBounds bounds = mbgl::LatLngBounds::empty();
+    for (auto coordinate : _coordinates)
+    {
+        bounds.extend(mbgl::LatLng(coordinate.latitude, coordinate.longitude));
+    }
+    _bounds = MGLCoordinateBoundsFromLatLngBounds(bounds);
+}
+
 - (MGLCoordinateBounds)overlayBounds
 {
     return _bounds;
@@ -144,7 +122,7 @@ mbgl::Color MGLColorObjectFromCGColorRef(CGColorRef cgColor)
 - (NSString *)description
 {
     return [NSString stringWithFormat:@"<%@: %p; count = %lu; bounds = %@>",
-               NSStringFromClass([self class]), (void *)self, (unsigned long)_count, MGLStringFromCoordinateBounds(_bounds)];
+               NSStringFromClass([self class]), (void *)self, (unsigned long)[self pointCount], MGLStringFromCoordinateBounds(_bounds)];
 }
 
 @end

platform/darwin/src/MGLPointCollection.mm

@@ -8,8 +8,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 @implementation MGLPointCollection
 {
-    size_t _count;
     MGLCoordinateBounds _bounds;
+    std::vector<CLLocationCoordinate2D> _coordinates;
 }
 
 + (instancetype)pointCollectionWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count
@@ -22,17 +22,12 @@ NS_ASSUME_NONNULL_BEGIN
     self = [super init];
     if (self)
     {
-        _count = count;
-        _coordinates = (CLLocationCoordinate2D *)malloc(_count * sizeof(CLLocationCoordinate2D));
-        
+        _coordinates = { coords, coords + count };
         mbgl::LatLngBounds bounds = mbgl::LatLngBounds::empty();
-        
-        for (NSUInteger i = 0; i < count; i++)
+        for (auto coordinate : _coordinates)
         {
-            _coordinates[i] = coords[i];
-            bounds.extend(mbgl::LatLng(coords[i].latitude, coords[i].longitude));
+            bounds.extend(mbgl::LatLng(coordinate.latitude, coordinate.longitude));
         }
-        
         _bounds = MGLCoordinateBoundsFromLatLngBounds(bounds);
     }
     return self;
@@ -40,32 +35,30 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (NSUInteger)pointCount
 {
-    return _count;
+    return _coordinates.size();
+}
+
+- (CLLocationCoordinate2D *)coordinates
+{
+    return _coordinates.data();
 }
 
 - (CLLocationCoordinate2D)coordinate
 {
-    assert(_count > 0);
-    
-    return CLLocationCoordinate2DMake(_coordinates[0].latitude, _coordinates[0].longitude);
+    NSAssert([self pointCount] > 0, @"A multipoint must have coordinates");
+    return _coordinates.at(0);
 }
 
 - (void)getCoordinates:(CLLocationCoordinate2D *)coords range:(NSRange)range
 {
-    if (range.location + range.length > _count)
+    if (range.location + range.length > [self pointCount])
     {
         [NSException raise:NSRangeException
                     format:@"Invalid coordinate range %@ extends beyond current coordinate count of %zu",
-         NSStringFromRange(range), _count];
-    }
-    
-    NSUInteger index = 0;
-    
-    for (NSUInteger i = range.location; i < range.location + range.length; i++)
-    {
-        coords[index] = _coordinates[i];
-        index++;
+         NSStringFromRange(range), [self pointCount]];
     }
+
+    std::copy(_coordinates.begin() + range.location, _coordinates.begin() + NSMaxRange(range), coords);
 }
 
 - (MGLCoordinateBounds)overlayBounds
@@ -104,7 +97,7 @@ NS_ASSUME_NONNULL_BEGIN
 - (NSString *)description
 {
     return [NSString stringWithFormat:@"<%@: %p; count = %lu>",
-            NSStringFromClass([self class]), (void *)self, (unsigned long)_count];
+            NSStringFromClass([self class]), (void *)self, (unsigned long)[self pointCount]];
 }
 
 @end