Guard web-logger uriList[2] reads so a missing month segment can't crash the handler by patrickrb · Pull Request #584 · patrickrb/FT8AF

patrickrb · 2026-07-13T21:20:21Z

Root cause

LogHttpServer is the always-on web-logbook HTTP server; it's constructed and start()ed unconditionally during MainViewModel init (MainViewModel.java:936), so it listens on DEFAULT_PORT for the whole app session and any browser on the same LAN (or the phone itself) can reach it.

Its request dispatcher, serve(), reads the trailing path segment uriList[2] (from getUri().split("/")) in three branches without the uriList.length >= 3 guard that its sibling branches — DELFOLLOW, DELQSL, DELQSLCALLSIGN — already have:

SHOWQSL — msg = HTML_STRING(showQSLByMonth(uriList[2])); sits outside the serve() try/catch. A bare GET /SHOWQSL splits to ["", "SHOWQSL"] (length 2), so uriList[2] throws an uncaught ArrayIndexOutOfBoundsException straight out of serve().
DOWNQSL / DOWNQSLNOQSL — the body branch correctly falls back to the default page when the segment is missing, but the Content-Disposition header line then dereferences uriList[2] unconditionally. That throws inside the try, so the client gets an opaque HTTP 500 instead of the intended default page.

Fix

Route every uriList[2] read through a new bounds-checked static helper uriSegment(uriList, index) (returns null when the segment is absent), and build the download filename through downloadFileName(month) (falls back to log.adi). This mirrors the guard the sibling branches already use.

Well-formed requests are unchanged — GET /SHOWQSL/202401 and GET /DOWNQSL/202401 produce byte-identical output/headers (downloadFileName("202401") == "log202401.adi", exactly what String.format("log%s.adi", uriList[2]) produced).
Malformed requests (missing trailing segment) now return the default page instead of throwing / returning a 500.

Testing

New pure-JVM LogHttpServerUriSegmentTest (6 cases) pins the guard: reproduces the exact "/SHOWQSL".split("/") → length-2 shape, asserts uriSegment(...) == null for the missing/out-of-range/negative/empty/null cases, and covers downloadFileName present/absent-month behaviour. Matches the existing LogHttpServerPageCountTest style (no Robolectric needed).
./gradlew :app:testDebugUnitTest — full unit-test suite green (BUILD SUCCESSFUL).

Risk

Low. Localized to LogHttpServer.serve() dispatch; no protocol/DSP/audio behavior touched, no change for valid requests. Java-only (no Kotlin/ktlint impact).

🤖 Generated with Claude Code

…ment can't crash the handler The always-on web-logbook HTTP server (LogHttpServer.serve, started at app launch) read the trailing path segment uriList[2] in three dispatch branches without the `uriList.length >= 3` guard its sibling branches (DELFOLLOW/DELQSL/DELQSLCALLSIGN) already use: - SHOWQSL (`showQSLByMonth(uriList[2])`) sat OUTSIDE the serve try/catch, so a bare `GET /SHOWQSL` — which splits to `["", "SHOWQSL"]` (length 2) — threw an uncaught ArrayIndexOutOfBoundsException out of serve(). - DOWNQSL / DOWNQSLNOQSL built the Content-Disposition header from uriList[2] unconditionally even when the body branch had already fallen back to the default page for a missing segment, throwing inside the try and returning an opaque HTTP 500 instead of the intended default page. Route every uriList[2] read through a new bounds-checked static helper `uriSegment(uriList, index)` (returns null when the segment is absent), and the download filename through `downloadFileName(month)` (falls back to log.adi). Behaviour is unchanged for well-formed requests; malformed ones now return the default page. Both helpers are pure and covered by LogHttpServerUriSegmentTest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

codecov · 2026-07-13T21:21:32Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 29.41%. Comparing base (647b12e) to head (4f5334f).

Additional details and impacted files

@@             Coverage Diff              @@
##                dev     #584      +/-   ##
============================================
+ Coverage     29.36%   29.41%   +0.04%     
  Complexity      197      197              
============================================
  Files           179      179              
  Lines         24068    24097      +29     
  Branches       3263     3268       +5     
============================================
+ Hits           7067     7087      +20     
- Misses        16780    16787       +7     
- Partials        221      223       +2

Flag	Coverage Δ
android	`14.62% <ø> (+0.10%)`	⬆️
native	`9.93% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.
see 4 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR hardens the always-on LogHttpServer request dispatcher so malformed web-logbook URLs (missing a trailing month segment) don’t crash the handler or return HTTP 500s, and adds unit tests to pin the new guard behavior.

Changes:

Added LogHttpServer.uriSegment(...) to safely read URI path segments by index.
Added LogHttpServer.downloadFileName(...) to generate a safe download filename when the month is missing.
Added a new pure-JVM unit test class validating segment bounds and filename fallback behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
ft8af/app/src/main/java/com/k1af/ft8af/html/LogHttpServer.java	Adds guarded URI segment access and uses it in `SHOWQSL` / `DOWNQSL*` paths to avoid `ArrayIndexOutOfBoundsException`.
ft8af/app/src/test/java/com/k1af/ft8af/html/LogHttpServerUriSegmentTest.java	Adds unit tests covering missing/out-of-range segment reads and download filename fallback.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    static String uriSegment(String[] uriList, int index) {
+        return uriList != null && index >= 0 && index < uriList.length ? uriList[index] : null;
+    }


            } else if (uri.equalsIgnoreCase("DOWNQSL")) {
-                if (uriList.length >= 3) {
-                    msg = downQSLByMonth(uriList[2], true);
+                String month = uriSegment(uriList, 2);
+                if (month != null) {
+                    msg = downQSLByMonth(month, true);
                } else {
                    msg = HtmlContext.DEFAULT_HTML();
                }
                response = newFixedLengthResponse(NanoHTTPD.Response.Status.OK, "text/plain", msg);
-                response.addHeader("Content-Disposition", String.format("attachment;filename=log%s.adi", uriList[2]));
+                response.addHeader("Content-Disposition", "attachment;filename=" + downloadFileName(month));



            } else if (uri.equalsIgnoreCase("DOWNQSLNOQSL")) {
-                if (uriList.length >= 3) {
-                    msg = downQSLByMonth(uriList[2], false);
+                String month = uriSegment(uriList, 2);
+                if (month != null) {
+                    msg = downQSLByMonth(month, false);
                } else {
                    msg = HtmlContext.DEFAULT_HTML();
                }
                response = newFixedLengthResponse(NanoHTTPD.Response.Status.OK, "text/plain", msg);
-                response.addHeader("Content-Disposition", String.format("attachment;filename=log%s.adi", uriList[2]));
+                response.addHeader("Content-Disposition", "attachment;filename=" + downloadFileName(month));


+    @Test
+    public void outOfRangeAndDegenerateInputs_areNull() {
+        String[] one = {"", "SHOWQSL"};
+        assertThat(LogHttpServer.uriSegment(one, 5)).isNull();
+        assertThat(LogHttpServer.uriSegment(one, -1)).isNull();
+        assertThat(LogHttpServer.uriSegment(new String[0], 0)).isNull();
+        assertThat(LogHttpServer.uriSegment(null, 2)).isNull();
+    }


patrickrb requested a review from Copilot July 14, 2026 14:18

Copilot started reviewing on behalf of patrickrb July 14, 2026 14:18 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Guard web-logger uriList[2] reads so a missing month segment can't crash the handler#584

Guard web-logger uriList[2] reads so a missing month segment can't crash the handler#584
patrickrb wants to merge 1 commit into
devfrom
optio/task-f6492afa-446d-400d-b9e4-b3057744c0df

patrickrb commented Jul 13, 2026

Uh oh!

codecov Bot commented Jul 13, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

patrickrb commented Jul 13, 2026

Root cause

Fix

Testing

Risk

Uh oh!

codecov Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Jul 13, 2026 •

edited

Loading