[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.

1 files changed, 39 insertions, 61 deletions

diff --git a/platform/darwin/src/MGLMultiPoint.mm b/platform/darwin/src/MGLMultiPoint.mm
index 17a61ed081..66ce27614a 100644
--- a/platform/darwin/src/MGLMultiPoint.mm
+++ b/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,59 +48,59 @@ 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))
-    {
-        [NSException raise:NSRangeException format:@"Reuse of existing coordinates array %p not supported", coords];
-    }
-    else if (range.length == 0)
+    if (range.length == 0)
     {
-        [NSException raise:NSRangeException format:@"Empty coordinate range %@", NSStringFromRange(range)];
+        return;
     }
-    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
+    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)
     {
-        // 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);
+        bounds.extend(mbgl::LatLng(coordinate.latitude, coordinate.longitude));
     }
-    [self didChangeValueForKey:@"coordinates"];
+    _bounds = MGLCoordinateBoundsFromLatLngBounds(bounds);
 }
 
 - (MGLCoordinateBounds)overlayBounds
@@ -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