Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[MGLGeoJSONSource initWithIdentifier:geoJSONData] crashes if data isn't a feature collection #6986

Closed
jfirebaugh opened this issue Nov 9, 2016 · 6 comments
Assignees
Labels
crash iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS release blocker Blocks the next final release runtime styling
Milestone

Comments

@jfirebaugh
Copy link
Contributor

The following code crashes with the unhandled exception mapbox::util::bad_variant_access: in get<T>() on this line.

    MGLGeoJSONSource *geojsonSource = [[MGLGeoJSONSource alloc]
        initWithIdentifier:@"geojson"
               geoJSONData:[@"{\"type\": \"Point\", \"coordinates\": [0, 0]}" dataUsingEncoding:NSUTF8StringEncoding]
                   options:nil];
@jfirebaugh jfirebaugh added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS labels Nov 9, 2016
@nitrag
Copy link
Contributor

nitrag commented Nov 10, 2016

Same error mapbox::util::bad_variant_access: in get<T>() so I figured I'd comment here.

I'm also getting this bug when adding MGLCircleStyleLayer after a MGLGeoJSONSource without any markers (it is only a single line).

var options = [String : Any]()
        options[MGLGeoJSONClusterOption] = true
        options[MGLGeoJSONClusterRadiusOption] = 42
        options[MGLGeoJSONClusterMaximumZoomLevelOption] = 14
        options[MGLGeoJSONMaximumZoomLevelOption] = 20
        options[MGLGeoJSONToleranceOption] = 1
let jsonPath = Bundle.main.url(forResource: "example", withExtension: "geojson")!
let geoJsonSource = MGLGeoJSONSource(identifier: "testasdf", url: jsonPath, options: options)
self.mapView.style().add(geoJsonSource)
NSLog("Source has been created and added")

//comment the rest of this out and it will work
let circleLayer = MGLCircleStyleLayer(identifier: "circle-geojson", source: geoJsonSource)
circleLayer.circleColor = MGLStyleConstantValue(rawValue: UIColor.orange)
circleLayer.circleOpacity = MGLStyleConstantValue(rawValue: 0.8)
circleLayer.circleRadius = MGLStyleConstantValue(rawValue: 13)
self.mapView.style().add(circleLayer);

@nitrag
Copy link
Contributor

nitrag commented Nov 10, 2016

I take that back...

It seems to crash when I reuse options. If I set:
let geoJsonSource1 = MGLGeoJSONSource(identifier: "asdfasadfas", url: path, options: [String : Any]())

it works fine. However as like below, it will crash

func addGeoJSONSource(){

    var options = [String : Any]()
    options[MGLGeoJSONClusterOption] = true
    options[MGLGeoJSONClusterRadiusOption] = 42
    options[MGLGeoJSONClusterMaximumZoomLevelOption] = 14
    options[MGLGeoJSONMaximumZoomLevelOption] = 20
    options[MGLGeoJSONToleranceOption] = 1

    let path = Bundle.main.url(forResource: "example", withExtension: "geojson")!
--> let geoJsonSource1 = MGLGeoJSONSource(identifier: "asdfasadfas", url: path, options: options)
    self.mapView.style().add(geoJsonSource1)
    let lineTest = MGLLineStyleLayer(identifier: "line-geojson", source: geoJsonSource1)
    self.mapView.style().add(lineTest)
    NSLog("Source has been created and added")

    let jsonPath = Bundle.main.url(forResource: "mcdonalds", withExtension: "geojson")!
    let geoJsonSource = MGLGeoJSONSource(identifier: "markers", url: jsonPath, options: options)
    self.mapView.style().add(geoJsonSource)
    NSLog("Source has been created and added")

    let circleLayer = MGLCircleStyleLayer(identifier: "circle-geojson", source: geoJsonSource)
    circleLayer.circleColor = MGLStyleConstantValue(rawValue: UIColor.orange)
    circleLayer.circleOpacity = MGLStyleConstantValue(rawValue: 0.8)
    circleLayer.circleRadius = MGLStyleConstantValue(rawValue: 13)
    self.mapView.style().add(circleLayer);

    NSLog("Done adding GeoJSON");
}

`libc++abi.dylib: terminating with uncaught exception of type mapbox::util::bad_variant_access: in get<T>()`

@1ec5
Copy link
Contributor

1ec5 commented Nov 16, 2016

This line assumes it’s a feature collection without checking first. This initializer should check first and return nil if the data doesn’t contain a feature collection or couldn’t be parsed as GeoJSON for some reason. The initializer’s return type should be declared as nullable instancetype, which will result in a failable initializer in Swift. We also need to document the fact that a feature collection is required.

@1ec5 1ec5 added this to the ios-v3.4.0 milestone Nov 16, 2016
@1ec5 1ec5 added the crash label Nov 16, 2016
@jfirebaugh
Copy link
Contributor Author

Why is a feature collection required? Can we implement full GeoJSON support instead?

@1ec5 1ec5 added the release blocker Blocks the next final release label Nov 16, 2016
@1ec5
Copy link
Contributor

1ec5 commented Nov 16, 2016

Oh, I was unaware that the style specification allows GeoJSON layers to have any GeoJSON type at the root. MGLGeoJSONStyleLayer should definitely support all of GeoJSON, because any type could come from a preexisting layer in the style.

With that change, features doesn’t make as much sense as an array of MGLFeature instances. I suppose it needs to be feature instead, typed as a (nullable) MGLFeature instance that could be an MGLShapeCollectionFeature under the hood for the cases that don’t currently crash. It’ll be a bit less convenient for use cases such as #6793 (comment), unfortunately.

/cc @boundsj

@frederoni
Copy link
Contributor

Fixed in #7145

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
crash iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS release blocker Blocks the next final release runtime styling
Projects
None yet
Development

No branches or pull requests

5 participants