Skip to content

Commit 6db6826

Browse files
authored
fix(android): eliminate OOM in response handling (#168)
* fix(android): eliminate OOM in response handling by streaming body parsing Replace full-body buffering in CompressedResponseSizeInterceptor and toWritableMap with streaming alternatives: CountingResponseBody counts bytes lazily as the body is consumed, and toWritableMap uses android.util.JsonReader to parse JSON token-by-token without an intermediate String. Non-JSON bodies are capped at 512K chars with the remainder drained. Content-Type and 1 KB sniffing with UTF-8 BOM stripping determine the parse path. Adds tests for all new pure-JVM helpers. * fix(android): guard against short reads in stripBom and sniffIsJson * fix(android): preserve scalar JSON roots in streaming toWritableMap * fix(android): measure body transfer time for all responses, not just chunked * fix(android): capture metadata reference at intercept time to avoid null lookup after removal * fix(android): address code review feedback on timing, source caching, and numeric overflow safety * fix(android): drain on parse failure, thread-safe metrics map, explicit UTF-8 in tests * fix(android): drain stream in finally block and fallback compressedSize for cached responses * fix(android): skip CountingResponseBody when no metrics, return null data on JSON parse failure * fix(android): guard non-finite doubles and reduce OOM test size to 64MB * perf(android): defer drain buffer allocation to only when bytes remain to consume * fix(android): remove metadata to prevent leaks on sync requests and failures * fix(test): correct KDoc comment to match actual 64 MB test size
1 parent f058e28 commit 6db6826

9 files changed

Lines changed: 999 additions & 203 deletions

File tree

android/src/main/java/com/mattermost/networkclient/Extensions.kt

Lines changed: 264 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.mattermost.networkclient
22

3+
import android.util.JsonReader
4+
import android.util.JsonToken
35
import android.util.Log
46
import com.facebook.react.bridge.Arguments
57
import com.facebook.react.bridge.ReadableMap
@@ -11,17 +13,26 @@ import okhttp3.Request
1113
import okhttp3.Response
1214
import org.json.JSONArray
1315
import org.json.JSONObject
14-
import org.json.JSONTokener
1516
import java.io.FilterInputStream
16-
import java.io.IOException
1717
import java.io.InputStream
1818
import java.io.InputStreamReader
19-
import java.security.MessageDigest
19+
import java.io.PushbackInputStream
2020
import java.nio.charset.StandardCharsets
21+
import java.security.MessageDigest
2122

2223

2324
var Response.retriesExhausted: Boolean? by NetworkClient.RequestRetriesExhausted
2425

26+
// Number of bytes to peek for JSON sniffing. Large enough to skip any leading
27+
// whitespace or a UTF-8 BOM before the first meaningful character.
28+
private const val SNIFF_BYTES = 1024
29+
30+
// Maximum number of UTF-16 chars retained for non-JSON string bodies.
31+
// Each char occupies 2 bytes in heap, so this caps heap usage at ~1 MB.
32+
// Bodies beyond this limit are drained and discarded — they are not valid
33+
// API payloads the app can use through the JS bridge.
34+
private const val MAX_STRING_BODY_CHARS = 512 * 1024
35+
2536
/**
2637
* Wraps an InputStream and counts the bytes read through it. Used to compute the
2738
* uncompressed body size for the metrics payload without buffering the body up front.
@@ -43,6 +54,191 @@ private class CountingInputStream(stream: InputStream) : FilterInputStream(strea
4354
}
4455
}
4556

57+
/**
58+
* Returns true when the MIME type string declares a JSON content type,
59+
* meaning we can skip body sniffing entirely.
60+
*/
61+
internal fun isMimeTypeJson(mimeType: String): Boolean =
62+
mimeType == "application/json" ||
63+
(mimeType.startsWith("application/") && mimeType.endsWith("+json")) ||
64+
mimeType == "text/json"
65+
66+
/**
67+
* Returns true when the raw JSON number token should be routed to a
68+
* floating-point parse rather than a long parse. Covers decimal points
69+
* and both exponent indicator characters.
70+
*/
71+
internal fun isJsonNumberFloat(raw: String): Boolean =
72+
raw.contains('.') || raw.contains('e') || raw.contains('E')
73+
74+
/**
75+
* Strips a UTF-8 BOM (EF BB BF) from the head of [stream] if present,
76+
* then returns the stream ready for downstream parsing. When no BOM is
77+
* found every byte is pushed back so the stream is unmodified.
78+
*
79+
* Uses a 3-byte PushbackInputStream internally; the caller receives the
80+
* pushback stream directly so no extra wrapping is needed.
81+
*/
82+
internal fun stripBom(stream: InputStream): PushbackInputStream {
83+
val pushback = PushbackInputStream(stream, 3)
84+
val bom = ByteArray(3)
85+
var bomRead = 0
86+
while (bomRead < 3) {
87+
val n = pushback.read(bom, bomRead, 3 - bomRead)
88+
if (n == -1) break
89+
bomRead += n
90+
}
91+
if (bomRead > 0) {
92+
val hasBom = bomRead == 3 &&
93+
bom[0] == 0xEF.toByte() &&
94+
bom[1] == 0xBB.toByte() &&
95+
bom[2] == 0xBF.toByte()
96+
if (!hasBom) pushback.unread(bom, 0, bomRead)
97+
}
98+
return pushback
99+
}
100+
101+
/**
102+
* Sniffs up to [SNIFF_BYTES] from [stream] to find the first non-whitespace,
103+
* non-BOM byte without buffering the full body. Returns true when that byte
104+
* is '{' or '[', indicating a JSON object or array.
105+
*
106+
* The BOM (if present) is consumed and discarded. All other sniff bytes are
107+
* pushed back so the returned stream begins at the first meaningful byte
108+
* (preceded by any non-BOM whitespace that was part of the sniff window).
109+
*/
110+
internal fun sniffIsJson(stream: InputStream): Pair<Boolean, PushbackInputStream> {
111+
val pushback = PushbackInputStream(stripBom(stream), SNIFF_BYTES)
112+
val sniffBuf = ByteArray(SNIFF_BYTES)
113+
var sniffRead = 0
114+
while (sniffRead < SNIFF_BYTES) {
115+
val n = pushback.read(sniffBuf, sniffRead, SNIFF_BYTES - sniffRead)
116+
if (n == -1) break
117+
sniffRead += n
118+
}
119+
120+
var firstMeaningful: Byte = 0
121+
for (i in 0 until sniffRead) {
122+
val b = sniffBuf[i]
123+
if (b != ' '.code.toByte() &&
124+
b != '\t'.code.toByte() &&
125+
b != '\n'.code.toByte() &&
126+
b != '\r'.code.toByte()) {
127+
firstMeaningful = b
128+
break
129+
}
130+
}
131+
132+
if (sniffRead > 0) {
133+
pushback.unread(sniffBuf, 0, sniffRead)
134+
}
135+
136+
val isJson = firstMeaningful == '{'.code.toByte() || firstMeaningful == '['.code.toByte()
137+
return Pair(isJson, pushback)
138+
}
139+
140+
/**
141+
* Reads at most MAX_STRING_BODY_CHARS from the stream into a String and then
142+
* discards the remainder. Prevents unbounded heap allocation for non-JSON bodies.
143+
*/
144+
internal fun readCappedString(stream: InputStream): String {
145+
val sb = StringBuilder()
146+
val charBuffer = CharArray(64 * 1024)
147+
var totalChars = 0
148+
InputStreamReader(stream, StandardCharsets.UTF_8).use { reader ->
149+
var read = reader.read(charBuffer)
150+
while (read != -1) {
151+
val toAppend = minOf(read, MAX_STRING_BODY_CHARS - totalChars)
152+
if (toAppend > 0) sb.append(charBuffer, 0, toAppend)
153+
totalChars += read
154+
if (totalChars >= MAX_STRING_BODY_CHARS) {
155+
// Drain remaining bytes via the raw stream to release Okio segments
156+
// without accumulating any more data in heap.
157+
val drainBuffer = ByteArray(64 * 1024)
158+
while (stream.read(drainBuffer) != -1) { /* drain */ }
159+
break
160+
}
161+
read = reader.read(charBuffer)
162+
}
163+
}
164+
return sb.toString()
165+
}
166+
167+
/**
168+
* Recursively reads a JSON object from the JsonReader into a WritableMap.
169+
*/
170+
private fun JsonReader.readWritableMap(): WritableMap {
171+
val map = Arguments.createMap()
172+
beginObject()
173+
while (hasNext()) {
174+
val key = nextName()
175+
when (peek()) {
176+
JsonToken.BEGIN_OBJECT -> map.putMap(key, readWritableMap())
177+
JsonToken.BEGIN_ARRAY -> map.putArray(key, readWritableArray())
178+
JsonToken.STRING -> map.putString(key, nextString())
179+
JsonToken.BOOLEAN -> map.putBoolean(key, nextBoolean())
180+
JsonToken.NUMBER -> {
181+
val raw = nextString()
182+
if (isJsonNumberFloat(raw)) {
183+
val d = raw.toDoubleOrNull()
184+
if (d != null && d.isFinite()) map.putDouble(key, d) else map.putString(key, raw)
185+
} else {
186+
val l = raw.toLongOrNull()
187+
when {
188+
l == null -> {
189+
val d = raw.toDoubleOrNull()
190+
if (d != null && d.isFinite()) map.putDouble(key, d) else map.putString(key, raw)
191+
}
192+
l in Int.MIN_VALUE..Int.MAX_VALUE -> map.putInt(key, l.toInt())
193+
else -> map.putDouble(key, l.toDouble())
194+
}
195+
}
196+
}
197+
JsonToken.NULL -> { nextNull(); map.putNull(key) }
198+
else -> skipValue()
199+
}
200+
}
201+
endObject()
202+
return map
203+
}
204+
205+
/**
206+
* Recursively reads a JSON array from the JsonReader into a WritableArray.
207+
*/
208+
private fun JsonReader.readWritableArray(): WritableArray {
209+
val array = Arguments.createArray()
210+
beginArray()
211+
while (hasNext()) {
212+
when (peek()) {
213+
JsonToken.BEGIN_OBJECT -> array.pushMap(readWritableMap())
214+
JsonToken.BEGIN_ARRAY -> array.pushArray(readWritableArray())
215+
JsonToken.STRING -> array.pushString(nextString())
216+
JsonToken.BOOLEAN -> array.pushBoolean(nextBoolean())
217+
JsonToken.NUMBER -> {
218+
val raw = nextString()
219+
if (isJsonNumberFloat(raw)) {
220+
val d = raw.toDoubleOrNull()
221+
if (d != null && d.isFinite()) array.pushDouble(d) else array.pushString(raw)
222+
} else {
223+
val l = raw.toLongOrNull()
224+
when {
225+
l == null -> {
226+
val d = raw.toDoubleOrNull()
227+
if (d != null && d.isFinite()) array.pushDouble(d) else array.pushString(raw)
228+
}
229+
l in Int.MIN_VALUE..Int.MAX_VALUE -> array.pushInt(l.toInt())
230+
else -> array.pushDouble(l.toDouble())
231+
}
232+
}
233+
}
234+
JsonToken.NULL -> { nextNull(); array.pushNull() }
235+
else -> skipValue()
236+
}
237+
}
238+
endArray()
239+
return array
240+
}
241+
46242
/**
47243
* Composes an array of redirect URLs from all prior responses
48244
*
@@ -79,48 +275,77 @@ fun Response.toWritableMap(metadata: RequestMetadata?): WritableMap {
79275
map.putBoolean("ok", isSuccessful)
80276

81277
body?.let { responseBody ->
82-
// Stream-decode the body so peak memory is the resulting String (plus a small
83-
// rolling char buffer), not the full body buffered in Okio segments first.
84-
// CountingInputStream tracks the byte count for the size metric, which stays
85-
// accurate when Content-Length is missing (chunked) or stale (compression).
86278
val countingStream = CountingInputStream(responseBody.source().inputStream())
87-
val bodyString = InputStreamReader(countingStream, StandardCharsets.UTF_8).use { reader ->
88-
val sb = StringBuilder()
89-
val charBuffer = CharArray(64 * 1024)
90-
var read = reader.read(charBuffer)
91-
while (read != -1) {
92-
sb.append(charBuffer, 0, read)
93-
read = reader.read(charBuffer)
94-
}
95-
sb.toString()
96-
}
97279

98-
if (metadata != null) {
99-
val compressedSize = header("X-Compressed-Size")?.toDoubleOrNull() ?: header("Content-Length")?.toDoubleOrNull() ?: 0.0
100-
val startTime = header("X-Start-Time")?.toDoubleOrNull() ?: 0.0
101-
val endTime = header("X-End-Time")?.toDoubleOrNull() ?: 0.0
102-
val mbps = header("X-Speed-Mbps")?.toDoubleOrNull() ?: 0.0
103-
metrics.putDouble("compressedSize", compressedSize)
104-
metrics.putDouble("size", countingStream.count.toDouble())
105-
metrics.putDouble("startTime", startTime)
106-
metrics.putDouble("endTime", endTime)
107-
metrics.putDouble("speedInMbps", mbps)
280+
val mimeType = responseBody.contentType()?.let { "${it.type}/${it.subtype}" } ?: ""
281+
val isJson: Boolean
282+
val pushback: PushbackInputStream
283+
284+
if (isMimeTypeJson(mimeType)) {
285+
isJson = true
286+
pushback = stripBom(countingStream)
287+
} else {
288+
val (sniffed, sniffStream) = sniffIsJson(countingStream)
289+
isJson = sniffed
290+
pushback = sniffStream
108291
}
109292

110-
try {
111-
when (val json = JSONTokener(bodyString).nextValue()) {
112-
is JSONArray -> {
113-
map.putArray("data", json.toWritableArray())
114-
}
115-
is JSONObject -> {
116-
map.putMap("data", json.toWritableMap())
117-
}
118-
else -> {
119-
map.putString("data", bodyString)
293+
if (isJson) {
294+
// Stream-parse JSON token by token directly from the InputStream.
295+
// No intermediate String or StringBuilder — the Okio segment buffer
296+
// drains at token-read speed and peak heap is the WritableMap tree only.
297+
JsonReader(InputStreamReader(pushback, StandardCharsets.UTF_8)).use { reader ->
298+
try {
299+
when (reader.peek()) {
300+
JsonToken.BEGIN_OBJECT -> map.putMap("data", reader.readWritableMap())
301+
JsonToken.BEGIN_ARRAY -> map.putArray("data", reader.readWritableArray())
302+
JsonToken.STRING -> map.putString("data", reader.nextString())
303+
JsonToken.BOOLEAN -> map.putBoolean("data", reader.nextBoolean())
304+
JsonToken.NUMBER -> {
305+
val raw = reader.nextString()
306+
if (isJsonNumberFloat(raw)) {
307+
val d = raw.toDoubleOrNull()
308+
if (d != null && d.isFinite()) map.putDouble("data", d) else map.putString("data", raw)
309+
} else {
310+
val l = raw.toLongOrNull()
311+
when {
312+
l == null -> {
313+
val d = raw.toDoubleOrNull()
314+
if (d != null && d.isFinite()) map.putDouble("data", d) else map.putString("data", raw)
315+
}
316+
l in Int.MIN_VALUE..Int.MAX_VALUE -> map.putInt("data", l.toInt())
317+
else -> map.putDouble("data", l.toDouble())
318+
}
319+
}
320+
}
321+
JsonToken.NULL -> { reader.nextNull(); map.putNull("data") }
322+
else -> map.putString("data", "")
323+
}
324+
} catch (_: Exception) {
325+
map.putNull("data")
326+
} finally {
327+
// Drain any remaining bytes before the reader closes the stream so
328+
// OkHttp can reuse the connection and countingStream.count is accurate.
329+
try {
330+
val drainBuffer = ByteArray(64 * 1024)
331+
while (pushback.read(drainBuffer) != -1) { /* drain */ }
332+
} catch (_: Exception) { }
120333
}
121334
}
122-
} catch (_: Exception) {
123-
map.putString("data", bodyString)
335+
} else {
336+
// Non-JSON body — read into a string with a hard cap to prevent OOM.
337+
// Responses beyond the cap are not valid API payloads the app can use.
338+
map.putString("data", readCappedString(pushback))
339+
}
340+
341+
if (metadata != null) {
342+
val compressedSize = if (metadata.compressedSize >= 0) metadata.compressedSize
343+
else header("Content-Length")?.toLongOrNull() ?: 0L
344+
metrics.putDouble("compressedSize", compressedSize.toDouble())
345+
metrics.putDouble("size", countingStream.count.toDouble())
346+
metrics.putDouble("startTime", metadata.requestStartNanos.toDouble())
347+
metrics.putDouble("endTime", metadata.requestEndNanos.toDouble())
348+
metrics.putDouble("speedInMbps", metadata.getSpeedInMbps())
124349
}
125350
}
126351

android/src/main/java/com/mattermost/networkclient/NetworkClient.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ internal class NetworkClient(private val context: Context, private val baseUrl:
100100
builder.addNetworkInterceptor(BrotliInterceptor)
101101

102102
if (shouldCollectMetrics) {
103-
builder.addNetworkInterceptor(CompressedResponseSizeInterceptor())
103+
builder.addNetworkInterceptor(CompressedResponseSizeInterceptor(metricsEventFactory))
104104
}
105105

106106
if (baseUrl == null) {

0 commit comments

Comments
 (0)