diff --git a/app/src/main/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClient.java b/app/src/main/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClient.java index 887f73ab..45b85fc3 100644 --- a/app/src/main/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClient.java +++ b/app/src/main/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClient.java @@ -251,12 +251,19 @@ public byte[] resolve(@NonNull byte[] dnsQuery, @Nullable Runnable onFailedAttem long contentLength = responseBody.contentLength(); if (contentLength > MAX_DOH_RESPONSE_BYTES) { Log.w(TAG, "DoH response too large: " + contentLength + " bytes"); + // An oversized body is a wasted attempt like any other unusable + // response. Leaving it uncounted made the query worth a single + // failure however many attempts it burned, so an endpoint + // spraying junk took ten whole queries to trip the breaker + // while every other failure mode took three. + reportFailedAttempt(onFailedAttempt); continue; } byte[] dnsResponse = response.peekBody(MAX_DOH_RESPONSE_BYTES + 1L).bytes(); if (dnsResponse.length > MAX_DOH_RESPONSE_BYTES) { Log.w(TAG, "DoH response too large: " + dnsResponse.length + " bytes"); + reportFailedAttempt(onFailedAttempt); continue; } if (dnsResponse.length < 12) { diff --git a/app/src/test/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClientTest.java b/app/src/test/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClientTest.java index 44375f35..f77c00c9 100644 --- a/app/src/test/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClientTest.java +++ b/app/src/test/java/net/kollnig/missioncontrol/dns/DnsOverHttpsClientTest.java @@ -204,6 +204,37 @@ public void resolveDoesNotReportCanceledCallAsFailedAttempt() throws Exception { assertEquals(0, failures.get()); } + /** + * An oversized body is a wasted attempt and must be reported, so that an + * endpoint spraying junk trips the circuit breaker at the same rate as any + * other failing one — see issue #760. + */ + @Test + public void resolveReportsOversizedDnsResponseAsFailedAttempt() { + server.enqueue(dnsResponse(200, responseOfLength(65536))); + server.enqueue(dnsResponse(200, responseOfLength(65536))); + server.enqueue(dnsResponse(200, responseOfLength(65536))); + + AtomicInteger failures = new AtomicInteger(0); + assertNull(client().resolve(QUERY, failures::incrementAndGet)); + + assertEquals(3, failures.get()); + assertEquals(3, server.getRequestCount()); + } + + /** The chunked path (no declared Content-Length) reports the same way. */ + @Test + public void resolveReportsOversizedChunkedResponseThenRecovers() { + server.enqueue(chunkedDnsResponse(200, responseOfLength(65536))); + server.enqueue(dnsResponse(200, RESPONSE)); + + AtomicInteger failures = new AtomicInteger(0); + assertArrayEquals(RESPONSE, client().resolve(QUERY, failures::incrementAndGet)); + + assertEquals(1, failures.get()); + assertEquals(2, server.getRequestCount()); + } + @Test public void resolveRejectsOversizedDnsResponse() { DnsOverHttpsClient.setScreenOff(true);