[Detail Bug] HttpAppleMapsGateway: Missing URL encoding of placeId and language in lookupPlace and reverseGeocode

[detail-app]

Detail Bug Report

https://app.detail.dev/org_befd6425-a158-4e24-9d4d-1e5c08769515/bugs/bug_07a054c1-74ed-4fa2-9e1e-88ebc42fe0a7

Summary

• Context: HttpAppleMapsGateway is the HTTP adapter responsible for communicating with the Apple Maps Server API.
• Bug: The lookupPlace and reverseGeocode methods in HttpAppleMapsGateway fail to URL-encode their parameters (placeId and language) before constructing the request URI.
• Actual vs. expected: The current implementation appends raw strings directly to the URI, causing java.lang.IllegalArgumentException if they contain illegal characters (like spaces) or allowing parameter injection; it is expected that all user-provided strings used in URIs are properly URL-encoded.
• Impact: The application will crash with an unhandled IllegalArgumentException when searching for places with certain IDs or using certain language tags, and it is vulnerable to query parameter injection.

Code with bug

// src/main/java/com/williamcallahan/applemaps/adapters/mapsserver/HttpAppleMapsGateway.java

    @Override
    public PlaceResults reverseGeocode(double latitude, double longitude, String language) {
        StringBuilder query = new StringBuilder();
        query.append(QUERY_PREFIX)
            .append(PARAMETER_LOCATION)
            .append("=")
            .append(latitude)
            .append(LOCATION_SEPARATOR)
            .append(longitude);
        String resolvedLanguage = Objects.requireNonNull(language, "language");
        if (!resolvedLanguage.isBlank()) {
            query.append(PARAMETER_SEPARATOR)
                .append(PARAMETER_LANGUAGE)
                .append("=")
                .append(resolvedLanguage); // <-- BUG 🔴 resolvedLanguage is not URL-encoded
        }
        return invokeApi("reverseGeocode", buildUri(REVERSE_GEOCODE_PATH, query.toString()), PlaceResults.class);
    }

    @Override
    public Place lookupPlace(String placeId, String language) {
        Objects.requireNonNull(placeId, "placeId");
        String resolvedLanguage = Objects.requireNonNull(language, "language");
        String queryString = "";
        if (!resolvedLanguage.isBlank()) {
            queryString = QUERY_PREFIX + PARAMETER_LANGUAGE + "=" + resolvedLanguage; // <-- BUG 🔴 resolvedLanguage is not URL-encoded
        }
        // <-- BUG 🔴 placeId is not URL-encoded and used in path
        return invokeApi("place", URI.create(API_SERVER + PLACE_PATH + "/" + placeId + queryString), Place.class);
    }

Failing test

import com.williamcallahan.applemaps.adapters.mapsserver.HttpAppleMapsGateway;
import org.junit.jupiter.api.Test;
import java.time.Duration;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class HttpAppleMapsGatewayEncodingTest {
    @Test
    public void testLookupPlaceWithSpaceInId() {
        HttpAppleMapsGateway gateway = new HttpAppleMapsGateway("token", Duration.ofSeconds(5));
        // This throws IllegalArgumentException because the space is not encoded before calling URI.create
        assertThrows(IllegalArgumentException.class, () -> {
            gateway.lookupPlace("place id with space", "en-US");
        });
    }

    @Test
    public void testReverseGeocodeWithSpaceInLanguage() {
        HttpAppleMapsGateway gateway = new HttpAppleMapsGateway("token", Duration.ofSeconds(5));
        // This throws IllegalArgumentException because the space is not encoded before calling URI.create
        assertThrows(IllegalArgumentException.class, () -> {
            gateway.reverseGeocode(37.33182, -122.03118, "en US");
        });
    }
}

Test output:

HttpAppleMapsGatewayEncodingTest > testLookupPlaceWithSpaceInId() PASSED
HttpAppleMapsGatewayEncodingTest > testReverseGeocodeWithSpaceInLanguage() PASSED

Recommended fix

Use URLEncoder.encode() for the language parameter in both methods, and for the placeId parameter in lookupPlace. Note that for placeId in the path, it should technically use path encoding (where space is %20), whereas URLEncoder uses query encoding (where space is +), so care should be taken to use the correct encoding scheme or a URI builder.

    @Override
    public Place lookupPlace(String placeId, String language) {
        Objects.requireNonNull(placeId, "placeId");
        String resolvedLanguage = Objects.requireNonNull(language, "language");
        String encodedId = URLEncoder.encode(placeId, StandardCharsets.UTF_8).replace("+", "%20"); // <-- FIX 🟢 path-segment encoding
        String queryString = "";
        if (!resolvedLanguage.isBlank()) {
            queryString = QUERY_PREFIX + PARAMETER_LANGUAGE + "=" + URLEncoder.encode(resolvedLanguage, StandardCharsets.UTF_8); // <-- FIX 🟢 encode query param
        }
        return invokeApi("place", URI.create(API_SERVER + PLACE_PATH + "/" + encodedId + queryString), Place.class);
    }