diff --git a/CHANGELOG.md b/CHANGELOG.md index 6193e0d..34f8f72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ - Security: reject cross-origin API redirects and redact API keys echoed in errors while preserving typed error causes. - CLI: sanitize parser diagnostics so invalid arguments cannot emit terminal control sequences. +- Route: sample correctly across the international date line, keep antipodal distances finite, and reject malformed upstream polylines before searching. +- Client: reject non-finite coordinates, radii, and ratings before API calls, with normal validation errors and CLI exit code 2. - Build: require Go 1.26.8 for upstream compiler and runtime fixes; update GoReleaser to 2.18.1, Node to 26.8.2, deadcode to 0.50.0, and govulncheck to 1.8.0 with matching release-verifier pins. - CI: pin Actions by commit, test Windows with Go 1.27.1 and macOS with the race detector, verify documentation metadata, reject formatting drift, and avoid duplicate branch-push audits. diff --git a/docs/client-reference.md b/docs/client-reference.md index e27a13f..69935ed 100644 --- a/docs/client-reference.md +++ b/docs/client-reference.md @@ -6,6 +6,7 @@ These request details are easy to miss when moving between the CLI and library: - `Filters.Types` maps to Google's singular `includedType`; only the first value is sent. - Search price levels use integers from `0` (free) through `4` (very expensive). +- Coordinates, radii, and minimum ratings must be finite numbers; invalid values fail validation before any API calls. - Details include reviews or photos only when `IncludeReviews` or `IncludePhotos` is set. - `PhotoMediaRequest` requires `MaxWidthPx` or `MaxHeightPx`; each supplied dimension must be between 1 and 4800. - Search and nearby responses include `NextPageToken` when Google returns one. Place summaries include business status when it is present upstream. diff --git a/docs/route.md b/docs/route.md index e38d848..719b210 100644 --- a/docs/route.md +++ b/docs/route.md @@ -32,3 +32,4 @@ response, err := client.Route(ctx, goplaces.RouteRequest{ - Requires the Google Routes API to be enabled. - Waypoints are sampled evenly along the route polyline. +- Longitude interpolation follows the short crossing of the international date line. Malformed or out-of-range polyline coordinates are rejected before place searches. diff --git a/internal/places/directions.go b/internal/places/directions.go index 2ddc7ca..c271526 100644 --- a/internal/places/directions.go +++ b/internal/places/directions.go @@ -206,11 +206,8 @@ func validateDirectionsLocation(label, placeID string, location *LatLng, text st } if location != nil { provided++ - if location.Lat < -90 || location.Lat > 90 { - return ValidationError{Field: label + ".lat", Message: "must be -90..90"} - } - if location.Lng < -180 || location.Lng > 180 { - return ValidationError{Field: label + ".lng", Message: "must be -180..180"} + if err := validateCoordinates(location.Lat, location.Lng, label); err != nil { + return err } } if strings.TrimSpace(text) != "" { diff --git a/internal/places/route.go b/internal/places/route.go index e417813..ee04b8c 100644 --- a/internal/places/route.go +++ b/internal/places/route.go @@ -206,11 +206,8 @@ func validateRouteRequest(req RouteRequest) error { if req.Limit < 1 || req.Limit > maxSearchLimit { return ValidationError{Field: validationFieldLimit, Message: fmt.Sprintf("must be 1-%d", maxSearchLimit)} } - if req.RadiusM <= 0 { - return ValidationError{Field: validationFieldRadiusM, Message: "must be > 0"} - } - if req.RadiusM > maxCircleRadiusM { - return ValidationError{Field: validationFieldRadiusM, Message: fmt.Sprintf("must be <= %d", maxCircleRadiusM)} + if err := validateRadius(req.RadiusM, validationFieldRadiusM); err != nil { + return err } if req.MaxWaypoints < 1 || req.MaxWaypoints > maxRouteWaypoints { return ValidationError{Field: "max_waypoints", Message: fmt.Sprintf("must be 1-%d", maxRouteWaypoints)} diff --git a/internal/places/route_geometry.go b/internal/places/route_geometry.go index 96e591e..2d89dde 100644 --- a/internal/places/route_geometry.go +++ b/internal/places/route_geometry.go @@ -12,44 +12,30 @@ const ( routePolylinePrecision = 1e5 ) +var errInvalidPolyline = errors.New("goplaces: invalid polyline") + func decodePolyline(encoded string) ([]LatLng, error) { if strings.TrimSpace(encoded) == "" { return nil, errors.New("goplaces: empty polyline") } points := make([]LatLng, 0, len(encoded)/4) - var lat, lng int + var lat, lng int64 for i := 0; i < len(encoded); { - var delta int - var shift uint - for { - if i >= len(encoded) { - return nil, errors.New("goplaces: invalid polyline") - } - b := int(encoded[i]) - 63 - i++ - delta |= (b & 0x1f) << shift - shift += 5 - if b < 0x20 { - break - } + deltaLat, next, err := decodePolylineDelta(encoded, i) + if err != nil { + return nil, err + } + deltaLng, next, err := decodePolylineDelta(encoded, next) + if err != nil { + return nil, err } - lat += (delta >> 1) ^ (-(delta & 1)) - - delta = 0 - shift = 0 - for { - if i >= len(encoded) { - return nil, errors.New("goplaces: invalid polyline") - } - b := int(encoded[i]) - 63 - i++ - delta |= (b & 0x1f) << shift - shift += 5 - if b < 0x20 { - break - } + i = next + lat += deltaLat + lng += deltaLng + if lat < -90*routePolylinePrecision || lat > 90*routePolylinePrecision || + lng < -180*routePolylinePrecision || lng > 180*routePolylinePrecision { + return nil, errInvalidPolyline } - lng += (delta >> 1) ^ (-(delta & 1)) points = append(points, LatLng{ Lat: float64(lat) / routePolylinePrecision, @@ -59,6 +45,25 @@ func decodePolyline(encoded string) ([]LatLng, error) { return points, nil } +func decodePolylineDelta(encoded string, index int) (int64, int, error) { + var value uint32 + for shift := uint(0); ; shift += 5 { + if index >= len(encoded) || encoded[index] < 63 || encoded[index] > 126 { + return 0, index, errInvalidPolyline + } + chunk := uint32(encoded[index] - 63) + index++ + // The seventh group has only two bits left in a 32-bit coordinate. + if shift == 30 && chunk > 3 { + return 0, index, errInvalidPolyline + } + value |= (chunk & 0x1f) << shift + if chunk < 0x20 { + return int64(value>>1) ^ -int64(value&1), index, nil + } + } +} + func sampleWaypoints(points []LatLng, maxWaypoints int) []LatLng { if len(points) == 0 || maxWaypoints <= 0 { return nil @@ -139,9 +144,10 @@ func pointAtCumulative(points []LatLng, cumulative []float64, target float64) La return next } fraction := (target - cumulative[index-1]) / segment + longitudeDelta := math.Remainder(next.Lng-prev.Lng, 360) return LatLng{ Lat: prev.Lat + (next.Lat-prev.Lat)*fraction, - Lng: prev.Lng + (next.Lng-prev.Lng)*fraction, + Lng: math.Remainder(prev.Lng+longitudeDelta*fraction, 360), } } @@ -157,7 +163,7 @@ func uniqueWaypoints(points []LatLng) []LatLng { func samePoint(a, b LatLng) bool { const epsilon = 1e-6 - return math.Abs(a.Lat-b.Lat) < epsilon && math.Abs(a.Lng-b.Lng) < epsilon + return math.Abs(a.Lat-b.Lat) < epsilon && math.Abs(math.Remainder(a.Lng-b.Lng, 360)) < epsilon } func distanceMeters(a, b LatLng) float64 { @@ -169,5 +175,6 @@ func distanceMeters(a, b LatLng) float64 { sinDLat := math.Sin(dlat / 2) sinDLng := math.Sin(dlng / 2) value := sinDLat*sinDLat + math.Cos(lat1)*math.Cos(lat2)*sinDLng*sinDLng - return 2 * earthRadiusMeters * math.Asin(math.Sqrt(value)) + // Roundoff near antipodal points can push the haversine above one. + return 2 * earthRadiusMeters * math.Asin(math.Sqrt(min(1, value))) } diff --git a/internal/places/route_geometry_test.go b/internal/places/route_geometry_test.go new file mode 100644 index 0000000..adaaf90 --- /dev/null +++ b/internal/places/route_geometry_test.go @@ -0,0 +1,71 @@ +package places + +import ( + "errors" + "fmt" + "math" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" +) + +func TestDecodePolylineRejectsInvalidCoordinates(t *testing.T) { + for _, encoded := range []string{"!!", "\x7f?", strings.Repeat("~", 14), strings.Repeat("_", 8) + "??", "acidP?", "`cidP?", "?agsia@", "?`gsia@"} { + if _, err := decodePolyline(encoded); err == nil { + t.Errorf("accepted invalid polyline %q", encoded) + } + } +} + +func TestSampleWaypointsAcrossDateLine(t *testing.T) { + for _, direction := range []float64{1, -1} { + points := []LatLng{{Lng: direction * 179}, {Lng: direction * 179.5}, {Lng: direction * -179.5}, {Lng: direction * -179}} + for _, count := range []int{1, 3} { + sampled := sampleWaypoints(points, count) + if len(sampled) != count { + t.Fatalf("sample count: %d", len(sampled)) + } + midpoint := sampled[len(sampled)/2] + if math.Abs(math.Abs(midpoint.Lng)-180) > 1e-6 || midpoint.Lat != 0 { + t.Errorf("date-line midpoint: %+v", midpoint) + } + } + } + if got := uniqueWaypoints([]LatLng{{Lng: 180}, {Lng: -180}}); len(got) != 1 { + t.Errorf("duplicate date-line points: %+v", got) + } +} + +func TestSampleWaypointsAntipodal(t *testing.T) { + points := []LatLng{{Lat: -58.77137, Lng: 17.12345}, {Lat: 58.77137, Lng: -162.87655}} + distance := distanceMeters(points[0], points[1]) + if math.IsNaN(distance) || math.Abs(distance-earthRadiusMeters*math.Pi) > 0.1 { + t.Fatalf("antipodal distance: %v", distance) + } + sampled := sampleWaypoints(points, 1) + if len(sampled) != 1 || math.IsNaN(sampled[0].Lat) || math.IsNaN(sampled[0].Lng) { + t.Fatalf("antipodal waypoint: %+v", sampled) + } +} + +func TestRouteRejectsMalformedPolylineBeforeSearch(t *testing.T) { + var searches atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != routesPath { + searches.Add(1) + } + _, _ = fmt.Fprint(w, `{"routes":[{"polyline":{"encodedPolyline":"!!"}}],"places":[]}`) + })) + defer server.Close() + client := NewClient(Options{APIKey: "synthetic-key", BaseURL: server.URL, RoutesBaseURL: server.URL}) + _, err := client.Route(t.Context(), RouteRequest{Query: "coffee", From: "A", To: "B"}) + var validation ValidationError + if err == nil || errors.As(err, &validation) || !strings.Contains(err.Error(), "invalid polyline") { + t.Fatalf("expected upstream polyline error, got %v", err) + } + if searches.Load() != 0 { + t.Fatal("malformed polyline triggered place searches") + } +} diff --git a/internal/places/search.go b/internal/places/search.go index 2f6a6f6..d26d92a 100644 --- a/internal/places/search.go +++ b/internal/places/search.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "math" "net/http" "strings" ) @@ -107,7 +108,7 @@ func validateSearchRequest(req SearchRequest) error { if req.Filters != nil { if req.Filters.MinRating != nil { - if *req.Filters.MinRating < 0 || *req.Filters.MinRating > 5 { + if math.IsNaN(*req.Filters.MinRating) || *req.Filters.MinRating < 0 || *req.Filters.MinRating > 5 { return ValidationError{Field: "filters.min_rating", Message: "must be 0-5"} } } diff --git a/internal/places/validation.go b/internal/places/validation.go index 7f899c3..9a9543a 100644 --- a/internal/places/validation.go +++ b/internal/places/validation.go @@ -1,6 +1,9 @@ package places -import "fmt" +import ( + "fmt" + "math" +) const ( payloadFieldAddress = "address" @@ -22,16 +25,27 @@ func validateCircle(bias *LocationBias, fieldPrefix string) error { if bias == nil { return nil } - if bias.RadiusM <= 0 { - return ValidationError{Field: fieldPrefix + ".radius_m", Message: "must be > 0"} + if err := validateRadius(bias.RadiusM, fieldPrefix+".radius_m"); err != nil { + return err + } + return validateCoordinates(bias.Lat, bias.Lng, fieldPrefix) +} + +func validateRadius(radius float64, field string) error { + if math.IsNaN(radius) || radius <= 0 { + return ValidationError{Field: field, Message: "must be > 0"} } - if bias.RadiusM > maxCircleRadiusM { - return ValidationError{Field: fieldPrefix + ".radius_m", Message: fmt.Sprintf("must be <= %d", maxCircleRadiusM)} + if radius > maxCircleRadiusM { + return ValidationError{Field: field, Message: fmt.Sprintf("must be <= %d", maxCircleRadiusM)} } - if bias.Lat < -90 || bias.Lat > 90 { + return nil +} + +func validateCoordinates(lat, lng float64, fieldPrefix string) error { + if math.IsNaN(lat) || lat < -90 || lat > 90 { return ValidationError{Field: fieldPrefix + ".lat", Message: "must be -90..90"} } - if bias.Lng < -180 || bias.Lng > 180 { + if math.IsNaN(lng) || lng < -180 || lng > 180 { return ValidationError{Field: fieldPrefix + ".lng", Message: "must be -180..180"} } return nil diff --git a/internal/places/validation_test.go b/internal/places/validation_test.go new file mode 100644 index 0000000..b18c082 --- /dev/null +++ b/internal/places/validation_test.go @@ -0,0 +1,78 @@ +package places + +import ( + "errors" + "fmt" + "math" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" +) + +func TestRequestsRejectNonFiniteNumbersBeforeHTTP(t *testing.T) { + var calls atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + calls.Add(1) + _, _ = fmt.Fprint(w, `{"routes":[{"polyline":{"encodedPolyline":"??"}}],"places":[]}`) + })) + defer server.Close() + client := NewClient(Options{APIKey: "synthetic-key", BaseURL: server.URL, RoutesBaseURL: server.URL, DirectionsBaseURL: server.URL}) + ctx := t.Context() + cases := []struct { + name, field string + run func(float64) error + }{ + {"search latitude", "location_bias.lat", func(v float64) error { + _, err := client.Search(ctx, SearchRequest{Query: "coffee", LocationBias: &LocationBias{Lat: v, RadiusM: 1}}) + return err + }}, + {"search longitude", "location_bias.lng", func(v float64) error { + _, err := client.Search(ctx, SearchRequest{Query: "coffee", LocationBias: &LocationBias{Lng: v, RadiusM: 1}}) + return err + }}, + {"search rating", "filters.min_rating", func(v float64) error { + _, err := client.Search(ctx, SearchRequest{Query: "coffee", Filters: &Filters{MinRating: &v}}) + return err + }}, + {"autocomplete radius", "location_bias.radius_m", func(v float64) error { + _, err := client.Autocomplete(ctx, AutocompleteRequest{Input: "cof", LocationBias: &LocationBias{RadiusM: v}}) + return err + }}, + {"nearby latitude", "location_restriction.lat", func(v float64) error { + _, err := client.NearbySearch(ctx, NearbySearchRequest{LocationRestriction: &LocationBias{Lat: v, RadiusM: 1}}) + return err + }}, + {"nearby radius", "location_restriction.radius_m", func(v float64) error { + _, err := client.NearbySearch(ctx, NearbySearchRequest{LocationRestriction: &LocationBias{RadiusM: v}}) + return err + }}, + {"directions origin", "from.lat", func(v float64) error { + _, err := client.Directions(ctx, DirectionsRequest{FromLocation: &LatLng{Lat: v}, To: "B"}) + return err + }}, + {"directions destination", "to.lng", func(v float64) error { + _, err := client.Directions(ctx, DirectionsRequest{From: "A", ToLocation: &LatLng{Lng: v}}) + return err + }}, + {"route radius", "radius_m", func(v float64) error { + _, err := client.Route(ctx, RouteRequest{Query: "coffee", From: "A", To: "B", RadiusM: v}) + return err + }}, + } + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + for _, value := range []float64{math.NaN(), math.Inf(1), math.Inf(-1)} { + before := calls.Load() + err := tt.run(value) + var validation ValidationError + if !errors.As(err, &validation) || validation.Field != tt.field { + t.Errorf("value %v: want validation field %s, got %v", value, tt.field, err) + } + if calls.Load() != before { + t.Errorf("value %v reached HTTP", value) + } + } + }) + } +}