From 8b7d3c9ee4c239178e8f77927e93cfc673d53ef3 Mon Sep 17 00:00:00 2001 From: Abhinav Dangeti Date: Mon, 17 Mar 2025 14:02:16 -0600 Subject: [PATCH 1/2] MB-65807: Graceful handling of non-conforming polygon data Geo's "extract3DCoordinates" does not validation input data which can cause mal-formed polygon data to cause a panic: panic({0x100a9e0c0?, 0x140001163a8?}) /opt/homebrew/opt/go/libexec/src/runtime/panic.go:770 +0x124 reflect.Value.Index({0x100a9ffe0?, 0x14000118c00?, 0x14000062c58?}, 0x1008913c8?) /opt/homebrew/opt/go/libexec/src/reflect/value.go:1447 +0x170 github.com/blevesearch/bleve/v2/geo.extract3DCoordinates({0x100a9ffe0?, 0x14000118c00?}) /Users/abhinav.dangeti/Documents/go/src/github.com/blevesearch/bleve/geo/parse.go:245 +0x12c github.com/blevesearch/bleve/v2/geo.ExtractGeoShapeCoordinates({0x100a9ffe0?, 0x14000118c00?}, {0x100a24659, 0x7}) See: https://pkg.go.dev/reflect#Value.IsValid --- geo/parse.go | 21 ++++++---- geo/parse_test.go | 101 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 113 insertions(+), 9 deletions(-) diff --git a/geo/parse.go b/geo/parse.go index 34f731a9e..ed1d935be 100644 --- a/geo/parse.go +++ b/geo/parse.go @@ -236,14 +236,19 @@ func extract2DCoordinates(thing interface{}) [][]float64 { func extract3DCoordinates(thing interface{}) (c [][][]float64) { coords := reflect.ValueOf(thing) - for i := 0; i < coords.Len(); i++ { - vals := coords.Index(i) - - edges := vals.Interface() - if es, ok := edges.([]interface{}); ok { - loop := extract2DCoordinates(es) - if len(loop) > 0 { - c = append(c, loop) + if !coords.IsValid() { + return nil + } + + if coords.Kind() == reflect.Slice { + for i := 0; i < coords.Len(); i++ { + vals := coords.Index(i) + edges := vals.Interface() + if es, ok := edges.([]interface{}); ok { + loop := extract2DCoordinates(es) + if len(loop) > 0 { + c = append(c, loop) + } } } } diff --git a/geo/parse_test.go b/geo/parse_test.go index 56145564c..2109c0ac1 100644 --- a/geo/parse_test.go +++ b/geo/parse_test.go @@ -15,12 +15,12 @@ package geo import ( + "encoding/json" "reflect" "testing" ) func TestExtractGeoPoint(t *testing.T) { - tests := []struct { in interface{} lon float64 @@ -353,3 +353,102 @@ func TestExtractGeoShape(t *testing.T) { } } } + +func TestExtractGeoShapeCoordinates(t *testing.T) { + tests := []struct { + x []byte + typ string + expectOK bool + }{ + { + x: []byte(`[ + [ + [77.58894681930542,12.976498523818783], + [77.58677959442139,12.974533005048169], + [77.58894681930542,12.976498523818783] + ] + ]`), + typ: PolygonType, + expectOK: true, + }, + { // Invalid construct, but handled + x: []byte(`[ + [ + {"lon":77.58894681930542,"lat":12.976498523818783}, + {"lon":77.58677959442139,"lat":12.974533005048169}, + {"lon":77.58894681930542,"lat":12.976498523818783} + ] + ]`), + typ: "polygon", + expectOK: false, + }, + { // Invalid construct causes panic (within extract3DCoordinates), fix MB-65807 + x: []byte(`{ + "coordinates": [ + [77.58894681930542,12.976498523818783], + [77.58677959442139,12.974533005048169], + [77.58894681930542,12.976498523818783] + ] + }`), + typ: "polygon", + expectOK: false, + }, + { + x: []byte(`[ + [ + [ + [-0.163421630859375,51.531600743186644], + [-0.15277862548828125,51.52455221546295], + [-0.15895843505859375,51.53693981046689], + [-0.163421630859375,51.531600743186644] + ] + ], + [ + [ + [-0.1902008056640625,51.5091698216777], + [-0.1599884033203125,51.51322956905176], + [-0.1902008056640625,51.5091698216777] + ] + ] + ]`), + typ: MultiPolygonType, + expectOK: true, + }, + { // Invalid construct causes panic (within extract3DCoordinates), fix MB-65807 + x: []byte(`[ + { + "coordinates": [ + [-0.163421630859375,51.531600743186644], + [-0.15277862548828125,51.52455221546295], + [-0.15895843505859375,51.53693981046689], + [-0.163421630859375,51.531600743186644] + ] + }, + { + "coordinates": [ + [-0.1902008056640625,51.5091698216777], + [-0.1599884033203125,51.51322956905176], + [-0.1902008056640625,51.5091698216777] + ] + } + ]`), + typ: MultiPolygonType, + expectOK: false, + }, + } + + for i := range tests { + var x interface{} + if err := json.Unmarshal(tests[i].x, &x); err != nil { + t.Fatalf("[%d] JSON err: %v", i+1, err) + } + _, typ, ok := ExtractGeoShapeCoordinates(x, tests[i].typ) + if ok != tests[i].expectOK { + t.Errorf("[%d] expected ok %t, got %t", i+1, tests[i].expectOK, ok) + } + + if ok && typ != tests[i].typ { + t.Errorf("[%d] expected type %s, got %s", i+1, tests[i].typ, typ) + } + } +} From 0b00f76efe8329876515ac8877d68d6e671999f1 Mon Sep 17 00:00:00 2001 From: Abhinav Dangeti Date: Mon, 17 Mar 2025 14:37:26 -0600 Subject: [PATCH 2/2] Fix test --- geo/parse_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/geo/parse_test.go b/geo/parse_test.go index 2109c0ac1..28acc6d8b 100644 --- a/geo/parse_test.go +++ b/geo/parse_test.go @@ -379,7 +379,7 @@ func TestExtractGeoShapeCoordinates(t *testing.T) { {"lon":77.58894681930542,"lat":12.976498523818783} ] ]`), - typ: "polygon", + typ: PolygonType, expectOK: false, }, { // Invalid construct causes panic (within extract3DCoordinates), fix MB-65807 @@ -390,7 +390,7 @@ func TestExtractGeoShapeCoordinates(t *testing.T) { [77.58894681930542,12.976498523818783] ] }`), - typ: "polygon", + typ: PolygonType, expectOK: false, }, { @@ -442,6 +442,7 @@ func TestExtractGeoShapeCoordinates(t *testing.T) { if err := json.Unmarshal(tests[i].x, &x); err != nil { t.Fatalf("[%d] JSON err: %v", i+1, err) } + _, typ, ok := ExtractGeoShapeCoordinates(x, tests[i].typ) if ok != tests[i].expectOK { t.Errorf("[%d] expected ok %t, got %t", i+1, tests[i].expectOK, ok)