Skip to content

Commit a9750bf

Browse files
pavelzemanpavelzeman
authored
Fix OkHttp header validation crashes from non-ASCII characters on Android (#160)
1 parent 7305814 commit a9750bf

File tree

4 files changed

+369
-3
lines changed

4 files changed

+369
-3
lines changed

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

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

3+
import android.util.Log
34
import com.facebook.react.bridge.Arguments
45
import com.facebook.react.bridge.ReadableMap
56
import com.facebook.react.bridge.WritableArray
@@ -134,8 +135,17 @@ fun Response.toDownloadMap(path: String): WritableMap {
134135
fun Request.Builder.applyHeaders(headers: Map<String, Any?>?): Request.Builder {
135136
if (headers != null){
136137
for ((k, v) in headers) {
137-
this.removeHeader(k)
138-
this.addHeader(k, v.toString())
138+
try {
139+
this.removeHeader(k)
140+
this.addHeader(k, v.toString())
141+
} catch (e: IllegalArgumentException) {
142+
// OkHttp validates header values and rejects non-ASCII characters
143+
// (e.g. control chars like 0x02 in corrupted auth tokens, or
144+
// Arabic-Indic digits from locale-dependent formatting).
145+
// Skip the invalid header and let the request proceed — the server
146+
// will reject it with a 4xx that the JS layer can handle gracefully.
147+
Log.w("NetworkClient", "Skipping header '$k': ${e.message}")
148+
}
139149
}
140150
}
141151

‎android/src/main/java/com/mattermost/networkclient/interceptors/CompressedResponseSizeInterceptor.kt‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package com.mattermost.networkclient.interceptors
33
import okhttp3.Interceptor
44
import okhttp3.Response
55
import okhttp3.ResponseBody.Companion.toResponseBody
6+
import java.util.Locale
67

78
class CompressedResponseSizeInterceptor: Interceptor {
89
override fun intercept(chain: Interceptor.Chain): Response {
@@ -37,7 +38,9 @@ class CompressedResponseSizeInterceptor: Interceptor {
3738
.header("X-Compressed-Size", compressedSize.toString())
3839
.header("X-Start-Time", startTime.toString())
3940
.header("X-End-Time", endTime.toString())
40-
.header("X-Speed-Mbps", "%.4f".format(speedMbps)) // Format to 4 decimal places
41+
// Use Locale.US to ensure ASCII digits in header values.
42+
// System locale formatting (e.g. Arabic) produces non-ASCII digits that OkHttp rejects.
43+
.header("X-Speed-Mbps", String.format(Locale.US, "%.4f", speedMbps))
4144
.build()
4245
}
4346
}

‎test-runner/src/test/kotlin/com/mattermost/networkclient/ApplyHeadersTest.kt‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
package com.mattermost.networkclient
2+
3+
import okhttp3.OkHttpClient
4+
import okhttp3.Request
5+
import okhttp3.mockwebserver.MockResponse
6+
import okhttp3.mockwebserver.MockWebServer
7+
import okhttp3.Interceptor
8+
import okhttp3.Response
9+
import org.junit.Assert
10+
import org.junit.Test
11+
12+
/**
13+
* Tests for applyHeaders resilience against invalid header values.
14+
*
15+
* Reproduces MATTERMOST-MOBILE-ANDROID-AYPW: corrupted tokens containing
16+
* non-ASCII control characters (e.g. 0x02) cause OkHttp to throw
17+
* IllegalArgumentException in addHeader() during request construction.
18+
* The fix catches this in applyHeaders and skips the invalid header.
19+
*/
20+
class ApplyHeadersTest {
21+
22+
/**
23+
* Interceptor that applies headers using OkHttp's addHeader WITHOUT
24+
* any error handling (broken behavior — reproduces the crash).
25+
*/
26+
class BrokenHeaderInterceptor(private val headers: Map<String, String>) : Interceptor {
27+
override fun intercept(chain: Interceptor.Chain): Response {
28+
val builder = chain.request().newBuilder()
29+
for ((k, v) in headers) {
30+
builder.removeHeader(k)
31+
builder.addHeader(k, v) // Throws on non-ASCII
32+
}
33+
return chain.proceed(builder.build())
34+
}
35+
}
36+
37+
/**
38+
* Interceptor that applies headers with the same catch-and-skip pattern
39+
* used in the fixed applyHeaders extension function.
40+
*/
41+
class FixedHeaderInterceptor(private val headers: Map<String, String>) : Interceptor {
42+
override fun intercept(chain: Interceptor.Chain): Response {
43+
val builder = chain.request().newBuilder()
44+
for ((k, v) in headers) {
45+
try {
46+
builder.removeHeader(k)
47+
builder.addHeader(k, v)
48+
} catch (e: IllegalArgumentException) {
49+
// Skip invalid header, same as the production fix
50+
}
51+
}
52+
return chain.proceed(builder.build())
53+
}
54+
}
55+
56+
@Test
57+
fun brokenHeaders_crashWithControlCharInToken() {
58+
val headers = mapOf("Authorization" to "Bearer abc\u0002def")
59+
60+
MockWebServer().use { server ->
61+
server.enqueue(MockResponse().setBody("ok"))
62+
server.start()
63+
64+
val client = OkHttpClient.Builder()
65+
.addInterceptor(BrokenHeaderInterceptor(headers))
66+
.build()
67+
68+
val request = Request.Builder()
69+
.url(server.url("/test"))
70+
.build()
71+
72+
try {
73+
client.newCall(request).execute().use { /* auto-close */ }
74+
Assert.fail("Expected IllegalArgumentException from OkHttp header validation")
75+
} catch (e: IllegalArgumentException) {
76+
Assert.assertTrue(
77+
"Expected error about unexpected char",
78+
e.message?.contains("Unexpected char") == true
79+
)
80+
}
81+
}
82+
}
83+
84+
@Test
85+
fun fixedHeaders_skipInvalidAndProceed() {
86+
val headers = mapOf(
87+
"Authorization" to "Bearer abc\u0002def",
88+
"X-Custom" to "valid-value"
89+
)
90+
91+
MockWebServer().use { server ->
92+
server.enqueue(MockResponse().setBody("ok"))
93+
server.start()
94+
95+
val client = OkHttpClient.Builder()
96+
.addInterceptor(FixedHeaderInterceptor(headers))
97+
.build()
98+
99+
val request = Request.Builder()
100+
.url(server.url("/test"))
101+
.build()
102+
103+
client.newCall(request).execute().use { response ->
104+
Assert.assertEquals(200, response.code)
105+
}
106+
107+
// Invalid Authorization header should be skipped, valid header kept
108+
val recorded = server.takeRequest()
109+
Assert.assertNull(
110+
"Invalid Authorization header should be skipped",
111+
recorded.getHeader("Authorization")
112+
)
113+
Assert.assertEquals("valid-value", recorded.getHeader("X-Custom"))
114+
}
115+
}
116+
117+
@Test
118+
fun fixedHeaders_cleanHeadersPassThrough() {
119+
val headers = mapOf(
120+
"Authorization" to "Bearer eyJhbGciOiJIUzI1NiJ9.valid.token",
121+
"Content-Type" to "application/json"
122+
)
123+
124+
MockWebServer().use { server ->
125+
server.enqueue(MockResponse().setBody("ok"))
126+
server.start()
127+
128+
val client = OkHttpClient.Builder()
129+
.addInterceptor(FixedHeaderInterceptor(headers))
130+
.build()
131+
132+
val request = Request.Builder()
133+
.url(server.url("/test"))
134+
.build()
135+
136+
client.newCall(request).execute().use { response ->
137+
Assert.assertEquals(200, response.code)
138+
}
139+
140+
val recorded = server.takeRequest()
141+
Assert.assertEquals(
142+
"Bearer eyJhbGciOiJIUzI1NiJ9.valid.token",
143+
recorded.getHeader("Authorization")
144+
)
145+
Assert.assertEquals("application/json", recorded.getHeader("Content-Type"))
146+
}
147+
}
148+
149+
@Test
150+
fun fixedHeaders_allInvalidHeadersSkipped() {
151+
val headers = mapOf(
152+
"X-Bad1" to "value\u0001here",
153+
"X-Bad2" to "\u0003\u0004\u0005"
154+
)
155+
156+
MockWebServer().use { server ->
157+
server.enqueue(MockResponse().setBody("ok"))
158+
server.start()
159+
160+
val client = OkHttpClient.Builder()
161+
.addInterceptor(FixedHeaderInterceptor(headers))
162+
.build()
163+
164+
val request = Request.Builder()
165+
.url(server.url("/test"))
166+
.build()
167+
168+
client.newCall(request).execute().use { response ->
169+
Assert.assertEquals(200, response.code)
170+
}
171+
172+
val recorded = server.takeRequest()
173+
Assert.assertNull(recorded.getHeader("X-Bad1"))
174+
Assert.assertNull(recorded.getHeader("X-Bad2"))
175+
}
176+
}
177+
}

0 commit comments

Comments
 (0)