Skip to content

[Detail Bug] Geocoding requests crash or allow query-parameter injection when location hint fields contain special characters #72

Description

@detail-app

Detail Bug Report

https://app.detail.dev/org_befd6425-a158-4e24-9d4d-1e5c08769515/bugs/bug_be806a16-75f7-4961-8676-4c27f0e705b2

Summary

  • Context: GeocodeInput is a record used to build geocoding requests for the Apple Maps Server API, converting its fields into a URL query string.
  • Bug: The location-related fields (searchLocation, searchRegion, and userLocation) are appended to the query string without URL encoding.
  • Actual vs. expected: These fields are appended as raw strings, whereas they should be URL-encoded to handle special characters like spaces and ampersands.
  • Impact: If these fields contain spaces, the application crashes with an IllegalArgumentException when attempting to create a java.net.URI. If they contain ampersands or equals signs, it can lead to query parameter injection, potentially breaking the request logic.

Code with Bug

// src/main/java/com/williamcallahan/applemaps/domain/request/GeocodeInput.java

    public String toQueryString() {
        List<String> parameters = new ArrayList<>();
        parameters.add(formatParameter(PARAMETER_QUERY, encode(address)));
        if (!limitToCountries.isEmpty()) {
            parameters.add(formatParameter(PARAMETER_LIMIT_TO_COUNTRIES, joinEncoded(limitToCountries)));
        }
        language.ifPresent(value -> parameters.add(formatParameter(PARAMETER_LANGUAGE, encode(value))));
        searchLocation.ifPresent(value -> parameters.add(formatParameter(PARAMETER_SEARCH_LOCATION, value.toQueryString()))); // <-- BUG 🔴 not URL-encoded
        searchRegion.ifPresent(value -> parameters.add(formatParameter(PARAMETER_SEARCH_REGION, value.toQueryString())));   // <-- BUG 🔴 not URL-encoded
        userLocation.ifPresent(value -> parameters.add(formatParameter(PARAMETER_USER_LOCATION, value.toQueryString())));     // <-- BUG 🔴 not URL-encoded
        return QUERY_PREFIX + String.join(PARAMETER_SEPARATOR, parameters);
    }

Explanation

GeocodeInput.toQueryString() correctly encodes some user-provided fields (e.g., address, language) via encode(), but directly concatenates searchLocation, searchRegion, and userLocation using value.toQueryString().

Because toQueryString() returns a raw string, spaces produce an invalid URI query (triggering IllegalArgumentException when building a java.net.URI), and reserved characters like & / = can alter the overall query string by injecting additional parameters.

Codebase Inconsistency

Within the same method, other fields are URL-encoded:

parameters.add(formatParameter(PARAMETER_QUERY, encode(address)));
language.ifPresent(value -> parameters.add(formatParameter(PARAMETER_LANGUAGE, encode(value))));

But the location-hint parameters are not.

Failing Test

import org.junit.jupiter.api.Test;

class GeocodeInputTest {

    @Test
    void toQueryStringProducesUriCompatibleString() {
        GeocodeInput geocodeInput = GeocodeInput.builder("test")
            .searchLocation(new SearchLocation("37.33, -122.03")) // Note the space after the comma
            .build();

        String queryString = geocodeInput.toQueryString();
        // This throws java.lang.IllegalArgumentException: Illegal character in query at index ...
        java.net.URI.create("https://maps-api.apple.com/v1/geocode" + queryString);
    }
}

Recommended Fix

Wrap the call to value.toQueryString() with encode() for all location-hint parameters:

searchLocation.ifPresent(value -> parameters.add(formatParameter(PARAMETER_SEARCH_LOCATION, encode(value.toQueryString()))));
searchRegion.ifPresent(value -> parameters.add(formatParameter(PARAMETER_SEARCH_REGION, encode(value.toQueryString()))));
userLocation.ifPresent(value -> parameters.add(formatParameter(PARAMETER_USER_LOCATION, encode(value.toQueryString()))));

History

This bug was introduced in commit 84b4c69. This commit added builder-based request inputs and centralized query serialization, but it failed to URL-encode the values returned by toQueryString() for location-based hint parameters.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions