Conversation
Updated URLService to use NetworkClientSecure for HTTP connections to reduce memory footprint for TLS connections.
📝 WalkthroughWalkthroughESP32 map downloads now validate connectivity and the URL before creating ChangesMap download transport
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/graphics/map/URLService.cpp`:
- Around line 42-43: Update the NetworkClientSecure setup in URLService to avoid
setInsecure() for tile downloads. Configure validation with a trusted CA bundle
or certificate pinning, and if neither is possible, constrain and document the
exception specifically to public map-tile requests rather than accepting
arbitrary certificates.
- Around line 42-46: In the URL setup flow, reorder the local declarations so
NetworkClientSecure secureClient is declared before HTTPClient http. Preserve
the existing initialization and http.begin behavior, ensuring secureClient
remains alive until HTTPClient is destroyed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cd736965-2662-424d-8fc9-217936755b5a
📒 Files selected for processing (1)
source/graphics/map/URLService.cpp
Moved HTTPClient instantiation before NetworkClientSecure
mverch67
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@source/graphics/map/TileProvider.cpp`:
- Line 7: Update the default Google tile URL in the tile-provider configuration
to use HTTPS instead of plaintext HTTP. If HTTPS is unsupported on specific
firmware or hardware targets, restrict the HTTP fallback to those targets via an
explicit build/platform condition while retaining HTTPS for secure builds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 56d1ffae-319c-4b74-b939-706d8f1d24b1
📒 Files selected for processing (1)
source/graphics/map/TileProvider.cpp
|
|
||
| std::vector<std::tuple<std::string, std::string>> TileProvider::urlTemplates = { | ||
| {"URL: Google Maps", "https://mt0.google.com/vt?lyrs=m&x={x}&s=&y={y}&z={z}"}}; | ||
| {"URL: Google Maps", "http://mt0.google.com/vt?lyrs=m&x={x}&s=&y={y}&z={z}"}}; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file context =="
if [ -f source/graphics/map/TileProvider.cpp ]; then
cat -n source/graphics/map/TileProvider.cpp | sed -n '1,80p'
else
echo "source/graphics/map/TileProvider.cpp not found"
fi
echo
echo "== tile provider/url service related references =="
rg -n "TileProvider|URLService|Google Maps|google\.com|lyrs=m|x=\{x\}|y=\{y\}|z=\{z\}" -S . --glob '!build/**' --glob '!bin/**' --glob '!dist/**' || true
echo
echo "== git diff summary and line 7 context =="
git diff --stat HEAD^..HEAD 2>/dev/null || git diff --stat HEAD 2>/dev/null || true
git diff --source/graphics/map/TileProvider.cpp 2>/dev/null | sed -n '1,120p' || trueRepository: meshtastic/device-ui
Length of output: 9872
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== source/graphics/TFT/TFTView_320x240.cpp map initialization =="
sed -n '2520,2570p' source/graphics/TFT/TFTView_320x240.cpp | cat -n
echo
echo "== source/graphics/TFT/TFTView_320x240.cpp map dropdown handling =="
sed -n '2650,2690p' source/graphics/TFT/TFTView_320x240.cpp | cat -n
echo
echo "== MapTileSettings usages =="
rg -n "tileProviderId|MapTileSettings|setTileProvider|getTileProvider" source include generated -S
echo
echo "== Google Maps URL settings in maps folder (if any) =="
fd -a '\.url$' . 2>/dev/null | while read -r f; do
rel="${f#$(pwd)/}"
printf '%s\n' "$rel"
cat -n "$rel" | sed -n '1,20p'
doneRepository: meshtastic/device-ui
Length of output: 17788
🌐 Web query:
Google Maps Static API vt map tiles https://mt0.google.com/vt TLS SSL certificate error
💡 Result:
If you are encountering SSL/TLS certificate errors or connection issues when attempting to access Google Maps tile servers (such as mt0.google.com), it is important to understand that these URLs are not part of the official, supported Google Maps Platform APIs [1]. Direct access to mt0.google.com for tile retrieval is generally unauthorized and violates the Google Maps Platform Terms of Service [1]. Because these endpoints are not intended for public developer use, they do not guarantee stable SSL/TLS configurations, and their behavior (including certificate chain validity) is subject to change without notice [1]. If you are experiencing TLS/SSL handshake errors, such as a "Trust anchor for certification path not found" or similar certificate validation issues, it is often because these unofficial endpoints may not provide the full certificate chain required by specific client environments [2]. To resolve these issues and ensure compliance, you should transition to the official Google Maps Platform services: 1. Official Alternatives: Use the officially documented Maps Static API or the Map Tiles API [3][1]. These services are designed for developer use, provide secure HTTPS access, and are supported by Google’s infrastructure [3][1]. 2. URL Formatting: Ensure all API requests are properly constructed [4]. Connection issues over HTTPS—often manifesting as 400 (Bad Request) or 403 (Forbidden) errors—are frequently caused by improper URL encoding (e.g., unencoded characters in path parameters) or missing/invalid API keys [5][6][7][4]. 3. Client Configuration: If you are using a custom application or a proxy, ensure your client environment (such as Java/Android key stores) is correctly configured to trust standard, reputable Certificate Authorities [2][8]. If you are currently using an unofficial URL structure like mt0.google.com/vt/..., you should replace it with the appropriate, officially supported endpoint for your use case via the Google Cloud Console [1].
Citations:
- 1: https://stackoverflow.com/questions/58846393/how-to-apply-api-key-to-google-maps-tile-server-url
- 2: https://www.traccar.org/forums/topic/manager-custom-url-issue/
- 3: https://developers.google.com/maps/documentation/maps-static/static-web-api-best-practices
- 4: https://developers.google.com/maps/documentation/maps-static/error-messages
- 5: https://stackoverflow.com/questions/79085886/google-maps-static-api-work-in-http-not-in-https-why
- 6: https://stackoverflow.com/questions/79414705/google-maps-static-api-request-isnt-working-over-https-and-http2-h2
- 7: https://stackoverflow.com/questions/63600822/google-maps-static-api-request-isnt-working-over-https
- 8: https://theses.hal.science/tel-01649735/file/DDOC_T_2017_0029_SHBAIR.pdf
🌐 Web query:
Google Maps vt map tiles http works over HTTPS official documentation
💡 Result:
Google Maps Platform services, including the Map Tiles API and Maps Static API, require the use of HTTPS for secure communication [1][2]. Official documentation specifies that HTTPS is mandatory for all requests that use API keys or involve user data to ensure security [1][2]. Requests made over plain HTTP that transmit sensitive information may be rejected [1][2]. If you encounter issues where requests fail over HTTPS but succeed over HTTP, it is typically not an indication that the service does not support HTTPS. Instead, such issues often stem from improper URL encoding—particularly with complex parameters (like encoded paths or markers)—which can cause servers to interpret the URL as malformed or illegal [3]. Ensuring that all URL parameters are correctly URL-encoded is the standard resolution for these errors [3]. Additionally, for web applications served over HTTPS, it is required to load static map images over HTTPS to prevent browser security warnings [4].
Citations:
- 1: https://developers.google.cn/maps/documentation/tile/web-api-best-practices
- 2: https://developers.google.com/maps/documentation/maps-static/static-web-api-best-practices
- 3: https://stackoverflow.com/questions/79414705/google-maps-static-api-request-isnt-working-over-https-and-http2-h2
- 4: https://developers.google.com/maps/documentation/maps-static/start
Do not use plaintext HTTP as the default Google tile URL.
The default template applies to every device using this source build unless an SD card provider overrides it. This exposes tile coordinates in plain text and allows network observers or on-path attackers to see or modify downloaded tiles. If TLS is unavailable on some firmware/hardware, gate the HTTP URL to those targets only and keep HTTPS for secure builds; otherwise use a supported HTTPS map API.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@source/graphics/map/TileProvider.cpp` at line 7, Update the default Google
tile URL in the tile-provider configuration to use HTTPS instead of plaintext
HTTP. If HTTPS is unsupported on specific firmware or hardware targets, restrict
the HTTP fallback to those targets via an explicit build/platform condition
while retaining HTTPS for secure builds.
|
This should have a warning somewhere. I think most of us expect https everywhere by now. |
2 participants
fixes #360 where TLS handshake runs out of memory.
Summary by CodeRabbit