Panic when reading geojson with null geometries in UnmarshallJSON()

[GeoWonk]

I am writing a program using SA2 geometries from the Australian Statistical Standard Geography which I have converted into geojson with the below code snippet.

curl http://www.ausstats.abs.gov.au/ausstats/subscriber.nsf/0/A09309ACB3FA50B8CA257FED0013D420/\$File/1270055001_sa2_2016_aust_shape.zip -o ../shapefiles/sa2_2016.zip

unzip ../shapefiles/sa2_2016.zip
unzip ../shapefiles/ste_2016.zip
ogr2ogr -f Geojson /SA2_2016_AUST.geojson SA2_2016_AUST.shp

When I try to run my code

SA2b, _ := ioutil.ReadFile(filename)
SA2, _ := geojson.UnmarshalFeatureCollection(SA2b)

I receive the following panic error

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x66961f]

goroutine 1 [running]:
encoding/json.(*decodeState).unmarshal.func1(0xc4200c5c18)
	/usr/local/go/src/encoding/json/decode.go:175 +0xd4
panic(0x6e71c0, 0x8eabd0)
	/usr/local/go/src/runtime/panic.go:502 +0x229
github.com/paulmach/orb/geojson.(*Feature).UnmarshalJSON(0xc42069b0e0, 0xc4304cdd1f, 0x1d1, 0x783ae33, 0x0, 0x7f318f62d840)
	/media/fpmpdrive/fpmp/goyulo/src/github.com/paulmach/orb/geojson/feature.go:86 +0x21f
encoding/json.(*decodeState).object(0xc4200ba240, 0x6efdc0, 0xc4200dfa00, 0x196)
	/usr/local/go/src/encoding/json/decode.go:626 +0x1c9d
encoding/json.(*decodeState).value(0xc4200ba240, 0x6efdc0, 0xc4200dfa00, 0x196)
	/usr/local/go/src/encoding/json/decode.go:408 +0x2d3
encoding/json.(*decodeState).array(0xc4200ba240, 0x6c8120, 0xc42530c0a8, 0x197)
	/usr/local/go/src/encoding/json/decode.go:583 +0x1d0
encoding/json.(*decodeState).value(0xc4200ba240, 0x6c8120, 0xc42530c0a8, 0x197)
	/usr/local/go/src/encoding/json/decode.go:405 +0x266
encoding/json.(*decodeState).object(0xc4200ba240, 0x6e83e0, 0xc42530c080, 0x16)
	/usr/local/go/src/encoding/json/decode.go:776 +0x132d
encoding/json.(*decodeState).value(0xc4200ba240, 0x6e83e0, 0xc42530c080, 0x16)
	/usr/local/go/src/encoding/json/decode.go:408 +0x2d3
encoding/json.(*decodeState).unmarshal(0xc4200ba240, 0x6e83e0, 0xc42530c080, 0x0, 0x0)
	/usr/local/go/src/encoding/json/decode.go:189 +0x1e7
encoding/json.Unmarshal(0xc42bd76000, 0xbf92952, 0xbf92b52, 0x6e83e0, 0xc42530c080, 0xbf92952, 0xbf92b52)
	/usr/local/go/src/encoding/json/decode.go:108 +0x148
github.com/paulmach/orb/geojson.UnmarshalFeatureCollection(0xc42bd76000, 0xbf92952, 0xbf92b52, 0xbf92952, 0xbf92b52, 0x0)
	/media/fpmpdrive/fpmp/goyulo/src/github.com/paulmach/orb/geojson/feature_collection.go:58 +0x6e
main.main()
	/media/fpmpdrive/fpmp/goyulo/src/yuloserver/main.go:61 +0x225
exit status 2

This was apparently caused missing geometries in the file (for statistical geographical classifications with no true spatial elements) and subsequently a pointer error when UnmarshallJSON. I fixed this by removing features will missing geometries in Python and resaving the file.

This scenario may well happen again to other users, especially if other data providers include features without spatial elements. Do you think it wise to put a check in to see if the geometry is present, or provide a specific error message?

[paulmach]

Thanks for the report. I just pushed a commit to so that in this case geometry: nil it'll return an error just like geometry: {}. So that's consistent.

However, it probably doesn't make sense to return an error for this and just leave the geometry as nil. That way you can actually filter the bad stuff out. What do you think?

[GeoWonk]

My personal preference is to give a warning but return nil geometries for the relevant features so I can filter them out before later work, or implement error handling at that stage, without having to preprocess the data.
But in any case as long as the error being thrown makes it clear that the problem is with the data being silly rather than a problem with the function I think it's ok.

[changchingchen]

According to the RFC section 3.2 Feature Object, it says

A Feature object has a member with the name "geometry".  The value
of the geometry member SHALL be either a Geometry object as
defined above or, in the case that the Feature is unlocated, a
JSON null value.

It is actually valid when the geometry in a Feature is null. In this case, it seems that is should not return an error when unmarshalling a nil geometry in the UnmarshalJSON func https://github.com/paulmach/orb/blob/master/geojson/feature.go#L81

[paulmach]

@changchingchen makes sense. I have the fix here #58

I'll merge it and bump the version in a few days.

[paulmach]

I hope this is finally resolved with #58 and is part of the recently tagged v0.2