From 4f5334f268d28a6a2a666400e8398bbf9f0e9cff Mon Sep 17 00:00:00 2001 From: Optio Agent Date: Mon, 13 Jul 2026 21:19:58 +0000 Subject: [PATCH] Guard web-logger uriList[2] path-segment reads so a missing month segment can't crash the handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../com/k1af/ft8af/html/LogHttpServer.java | 44 ++++++++++-- .../html/LogHttpServerUriSegmentTest.java | 70 +++++++++++++++++++ 2 files changed, 107 insertions(+), 7 deletions(-) create mode 100644 ft8af/app/src/test/java/com/k1af/ft8af/html/LogHttpServerUriSegmentTest.java diff --git a/ft8af/app/src/main/java/com/k1af/ft8af/html/LogHttpServer.java b/ft8af/app/src/main/java/com/k1af/ft8af/html/LogHttpServer.java index 00b64c9e..669576e6 100644 --- a/ft8af/app/src/main/java/com/k1af/ft8af/html/LogHttpServer.java +++ b/ft8af/app/src/main/java/com/k1af/ft8af/html/LogHttpServer.java @@ -86,6 +86,33 @@ static int normalizePageSize(int pageSize) { return pageSize > 0 ? pageSize : 100; } + /** + * The request path segment at {@code index} (from {@code getUri().split("/")}), + * or {@code null} when the request has no such segment. + * + *

Every {@code uriList[2]} read in {@link #serve} routes through this so a + * request that omits the trailing segment falls back to the default page instead + * of throwing {@link ArrayIndexOutOfBoundsException}. {@code GET /SHOWQSL} split to + * {@code ["", "SHOWQSL"]} (length 2), so the old bare {@code uriList[2]} threw — and + * for SHOWQSL that read sat outside the try/catch, so it escaped + * {@code serve} uncaught; for the DOWNQSL / DOWNQSLNOQSL {@code Content-Disposition} + * header it threw inside the try and returned an opaque HTTP 500 even though the + * body branch had already fallen back correctly. Mirrors the {@code uriList.length + * >= 3} guard the DELFOLLOW / DELQSL / DELQSLCALLSIGN branches already use. + */ + static String uriSegment(String[] uriList, int index) { + return uriList != null && index >= 0 && index < uriList.length ? uriList[index] : null; + } + + /** + * {@code Content-Disposition} filename for a per-month QSL download. Falls back to + * {@code log.adi} when the month segment is absent (see {@link #uriSegment}), rather + * than reading a missing path segment. + */ + static String downloadFileName(String month) { + return String.format("log%s.adi", month != null ? month : ""); + } + @Override public Response serve(IHTTPSession session) { String[] uriList = session.getUri().split("/"); @@ -146,7 +173,8 @@ public Response serve(IHTTPSession session) { } else if (uri.equalsIgnoreCase("SHOWALLQSL")) { msg = HTML_STRING(showAllQSL()); } else if (uri.equalsIgnoreCase("SHOWQSL")) { - msg = HTML_STRING(showQSLByMonth(uriList[2])); + String month = uriSegment(uriList, 2); + msg = month != null ? HTML_STRING(showQSLByMonth(month)) : HtmlContext.DEFAULT_HTML(); } else if (uri.equalsIgnoreCase("DELQSLCALLSIGN")) {//Delete a QSO callsign if (uriList.length >= 3) { deleteQSLCallSign(uriList[2].replace("_", "/")); @@ -164,22 +192,24 @@ public Response serve(IHTTPSession session) { response = newFixedLengthResponse(NanoHTTPD.Response.Status.OK, "text/plain", msg); response.addHeader("Content-Disposition", "attachment;filename=All_log.adi"); } 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)); } else { response = newFixedLengthResponse(msg); diff --git a/ft8af/app/src/test/java/com/k1af/ft8af/html/LogHttpServerUriSegmentTest.java b/ft8af/app/src/test/java/com/k1af/ft8af/html/LogHttpServerUriSegmentTest.java new file mode 100644 index 00000000..ee6efce2 --- /dev/null +++ b/ft8af/app/src/test/java/com/k1af/ft8af/html/LogHttpServerUriSegmentTest.java @@ -0,0 +1,70 @@ +package com.k1af.ft8af.html; + +import static com.google.common.truth.Truth.assertThat; + +import org.junit.Test; + +/** + * Pure-logic coverage for {@link LogHttpServer#uriSegment(String[], int)} and + * {@link LogHttpServer#downloadFileName(String)}, the guards that keep the + * web-logbook {@code serve} dispatch from reading a missing {@code uriList[2]} + * path segment. + * + *

Regression: {@code GET /SHOWQSL} (no trailing month) split to + * {@code ["", "SHOWQSL"]} and the bare {@code uriList[2]} read threw + * {@link ArrayIndexOutOfBoundsException}. For SHOWQSL that read was outside the + * {@code serve} try/catch, so it escaped the handler entirely; the + * DOWNQSL / DOWNQSLNOQSL {@code Content-Disposition} header read threw inside the + * try and returned an opaque HTTP 500 even though the body branch had already + * fallen back to the default page. The three guarded sibling branches + * (DELFOLLOW / DELQSL / DELQSLCALLSIGN) already used {@code uriList.length >= 3}. + * + *

These tests reproduce the exact {@code getUri().split("/")} shape and pin the + * guarded behaviour. No Android types are touched, so no Robolectric runner is + * needed. + */ +public class LogHttpServerUriSegmentTest { + + @Test + public void missingTrailingSegment_isNull() { + // "/SHOWQSL".split("/") == ["", "SHOWQSL"] (length 2), so index 2 is absent. + String[] uriList = "/SHOWQSL".split("/"); + assertThat(uriList).hasLength(2); + assertThat(LogHttpServer.uriSegment(uriList, 2)).isNull(); + } + + @Test + public void presentTrailingSegment_isReturned() { + String[] uriList = "/SHOWQSL/202401".split("/"); + assertThat(LogHttpServer.uriSegment(uriList, 2)).isEqualTo("202401"); + } + + @Test + public void downloadMissingMonth_isNull() { + // The DOWNQSL Content-Disposition header used to dereference this unconditionally. + assertThat(LogHttpServer.uriSegment("/DOWNQSL".split("/"), 2)).isNull(); + assertThat(LogHttpServer.uriSegment("/DOWNQSLNOQSL".split("/"), 2)).isNull(); + } + + @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(); + } + + @Test + public void downloadFileName_usesMonthWhenPresent() { + // Identical to the old String.format("log%s.adi", uriList[2]) when the month exists. + assertThat(LogHttpServer.downloadFileName("202401")).isEqualTo("log202401.adi"); + assertThat(LogHttpServer.downloadFileName("")).isEqualTo("log.adi"); + } + + @Test + public void downloadFileName_fallsBackWhenMonthMissing() { + // A bare GET /DOWNQSL must produce a sane filename, not crash the header build. + assertThat(LogHttpServer.downloadFileName(null)).isEqualTo("log.adi"); + } +}