Skip to content

Commit 2bb78bc

Browse files
jasonsandlinjasonsandlin
authored
Merge branch 'main' into websocket-compression
2 parents c626405 + 1ef4f92 commit 2bb78bc

26 files changed

Lines changed: 1460 additions & 223 deletions

File tree

‎Build/libHttpClient.Android.Workspace/gradle.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
android.useAndroidX=true

‎Build/libHttpClient.Android/build.gradle‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ android {
88
targetSdkVersion 34
99
minSdkVersion 21
1010

11+
testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
12+
1113
externalNativeBuild {
1214
cmake {
1315
if (project.hasProperty("HC_NOWEBSOCKETS")) {
@@ -51,4 +53,10 @@ android {
5153

5254
dependencies {
5355
implementation "com.squareup.okhttp3:okhttp:4.9.2"
56+
57+
androidTestImplementation "androidx.test:runner:1.5.2"
58+
androidTestImplementation "androidx.test:rules:1.5.0"
59+
androidTestImplementation "androidx.test.ext:junit:1.1.5"
60+
androidTestImplementation "junit:junit:4.13.2"
61+
androidTestImplementation "com.squareup.okhttp3:mockwebserver:4.9.2"
5462
}

‎Build/libHttpClient.Android/src/androidTest/AndroidManifest.xml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
3+
package="com.xbox.httpclient.test">
4+
5+
<uses-permission android:name="android.permission.INTERNET"/>
6+
7+
<application
8+
android:networkSecurityConfig="@xml/network_security_config" />
9+
</manifest>

‎Build/libHttpClient.Android/src/androidTest/java/com/xbox/httpclient/HttpMethodTest.java‎

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
// Copyright (c) Microsoft Corporation
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
package com.xbox.httpclient;
4+
5+
import android.content.Context;
6+
7+
import androidx.test.ext.junit.runners.AndroidJUnit4;
8+
import androidx.test.platform.app.InstrumentationRegistry;
9+
10+
import org.junit.After;
11+
import org.junit.Before;
12+
import org.junit.Test;
13+
import org.junit.runner.RunWith;
14+
15+
import java.io.IOException;
16+
import java.lang.reflect.Field;
17+
import java.util.concurrent.TimeUnit;
18+
19+
import okhttp3.OkHttpClient;
20+
import okhttp3.Request;
21+
import okhttp3.Response;
22+
import okhttp3.mockwebserver.MockResponse;
23+
import okhttp3.mockwebserver.MockWebServer;
24+
import okhttp3.mockwebserver.RecordedRequest;
25+
26+
import static org.junit.Assert.assertEquals;
27+
import static org.junit.Assert.assertNotNull;
28+
import static org.junit.Assert.fail;
29+
30+
/**
31+
* Instrumentation tests for HttpClientRequest HTTP method handling.
32+
*
33+
* These tests verify that setHttpMethodAndBody() correctly sets the HTTP method
34+
* on the OkHttp Request.Builder for ALL HTTP verbs, not just POST/PUT.
35+
*
36+
* A regression in commit 80d9d8c moved the requestBuilder.method() call inside
37+
* the POST/PUT conditional, causing DELETE, HEAD, OPTIONS, and PATCH with no body
38+
* to never have their method set (defaulting to GET).
39+
*
40+
* Tests use two strategies:
41+
* 1. Reflection: Read the Request.Builder after setHttpMethodAndBody() and verify
42+
* the built Request has the correct method.
43+
* 2. MockWebServer: Actually send the request and verify what the server received.
44+
*/
45+
@RunWith(AndroidJUnit4.class)
46+
public class HttpMethodTest {
47+
48+
private Context context;
49+
private MockWebServer server;
50+
51+
@Before
52+
public void setUp() throws IOException {
53+
context = InstrumentationRegistry.getInstrumentation().getTargetContext();
54+
server = new MockWebServer();
55+
server.start();
56+
}
57+
58+
@After
59+
public void tearDown() throws IOException {
60+
server.shutdown();
61+
}
62+
63+
// =========================================================================
64+
// Reflection-based tests: verify the Request.Builder method is set correctly
65+
// =========================================================================
66+
67+
/**
68+
* Build an OkHttp Request from HttpClientRequest's internal requestBuilder
69+
* after calling setHttpUrl() and setHttpMethodAndBody().
70+
*/
71+
private Request buildRequestViaReflection(String method, long contentLength) throws Exception {
72+
HttpClientRequest request = new HttpClientRequest(context);
73+
request.setHttpUrl(server.url("/test").toString());
74+
request.setHttpMethodAndBody(method, 0L, null, contentLength);
75+
76+
Field builderField = HttpClientRequest.class.getDeclaredField("requestBuilder");
77+
builderField.setAccessible(true);
78+
Request.Builder builder = (Request.Builder) builderField.get(request);
79+
return builder.build();
80+
}
81+
82+
@Test
83+
public void testDeleteMethodSetOnRequest() throws Exception {
84+
Request req = buildRequestViaReflection("DELETE", 0);
85+
assertEquals("DELETE method should be set on Request", "DELETE", req.method());
86+
}
87+
88+
@Test
89+
public void testGetMethodSetOnRequest() throws Exception {
90+
Request req = buildRequestViaReflection("GET", 0);
91+
assertEquals("GET method should be set on Request", "GET", req.method());
92+
}
93+
94+
@Test
95+
public void testPostMethodSetOnRequest() throws Exception {
96+
Request req = buildRequestViaReflection("POST", 0);
97+
assertEquals("POST method should be set on Request", "POST", req.method());
98+
}
99+
100+
@Test
101+
public void testPutMethodSetOnRequest() throws Exception {
102+
Request req = buildRequestViaReflection("PUT", 0);
103+
assertEquals("PUT method should be set on Request", "PUT", req.method());
104+
}
105+
106+
@Test
107+
public void testHeadMethodSetOnRequest() throws Exception {
108+
Request req = buildRequestViaReflection("HEAD", 0);
109+
assertEquals("HEAD method should be set on Request", "HEAD", req.method());
110+
}
111+
112+
@Test
113+
public void testOptionsMethodSetOnRequest() throws Exception {
114+
Request req = buildRequestViaReflection("OPTIONS", 0);
115+
assertEquals("OPTIONS method should be set on Request", "OPTIONS", req.method());
116+
}
117+
118+
@Test
119+
public void testPatchMethodSetOnRequest() throws Exception {
120+
Request req = buildRequestViaReflection("PATCH", 0);
121+
assertEquals("PATCH method should be set on Request", "PATCH", req.method());
122+
}
123+
124+
// =========================================================================
125+
// MockWebServer-based tests: verify the server actually receives the right method
126+
// =========================================================================
127+
128+
/**
129+
* Build the request via HttpClientRequest, then send it directly through
130+
* OkHttpClient to the MockWebServer and verify the recorded method.
131+
*/
132+
private RecordedRequest executeAndRecordRequest(String method, long contentLength) throws Exception {
133+
server.enqueue(new MockResponse().setResponseCode(200).setBody("ok"));
134+
135+
Request request = buildRequestViaReflection(method, contentLength);
136+
137+
OkHttpClient client = new OkHttpClient.Builder()
138+
.connectTimeout(5, TimeUnit.SECONDS)
139+
.readTimeout(5, TimeUnit.SECONDS)
140+
.build();
141+
142+
try (Response response = client.newCall(request).execute()) {
143+
assertNotNull("Response should not be null", response.body());
144+
}
145+
146+
RecordedRequest recorded = server.takeRequest(5, TimeUnit.SECONDS);
147+
assertNotNull("Server should have received a request", recorded);
148+
return recorded;
149+
}
150+
151+
@Test
152+
public void testDeleteMethodReceivedByServer() throws Exception {
153+
RecordedRequest recorded = executeAndRecordRequest("DELETE", 0);
154+
assertEquals("Server should receive DELETE", "DELETE", recorded.getMethod());
155+
}
156+
157+
@Test
158+
public void testGetMethodReceivedByServer() throws Exception {
159+
RecordedRequest recorded = executeAndRecordRequest("GET", 0);
160+
assertEquals("Server should receive GET", "GET", recorded.getMethod());
161+
}
162+
163+
@Test
164+
public void testPostMethodReceivedByServer() throws Exception {
165+
RecordedRequest recorded = executeAndRecordRequest("POST", 0);
166+
assertEquals("Server should receive POST", "POST", recorded.getMethod());
167+
}
168+
169+
@Test
170+
public void testPutMethodReceivedByServer() throws Exception {
171+
RecordedRequest recorded = executeAndRecordRequest("PUT", 0);
172+
assertEquals("Server should receive PUT", "PUT", recorded.getMethod());
173+
}
174+
175+
@Test
176+
public void testHeadMethodReceivedByServer() throws Exception {
177+
RecordedRequest recorded = executeAndRecordRequest("HEAD", 0);
178+
assertEquals("Server should receive HEAD", "HEAD", recorded.getMethod());
179+
}
180+
181+
@Test
182+
public void testOptionsMethodReceivedByServer() throws Exception {
183+
RecordedRequest recorded = executeAndRecordRequest("OPTIONS", 0);
184+
assertEquals("Server should receive OPTIONS", "OPTIONS", recorded.getMethod());
185+
}
186+
187+
@Test
188+
public void testPatchMethodReceivedByServer() throws Exception {
189+
RecordedRequest recorded = executeAndRecordRequest("PATCH", 0);
190+
assertEquals("Server should receive PATCH", "PATCH", recorded.getMethod());
191+
}
192+
193+
// =========================================================================
194+
// httpbin.org E2E fallback test
195+
// =========================================================================
196+
197+
@Test
198+
public void testHttpbinDeleteEcho() throws Exception {
199+
// Build a DELETE request using the actual HttpClientRequest class
200+
// and verify it works end-to-end against httpbin.org
201+
HttpClientRequest httpClientRequest = new HttpClientRequest(context);
202+
httpClientRequest.setHttpUrl("https://httpbin.org/delete");
203+
httpClientRequest.setHttpMethodAndBody("DELETE", 0L, null, 0L);
204+
205+
Field builderField = HttpClientRequest.class.getDeclaredField("requestBuilder");
206+
builderField.setAccessible(true);
207+
Request.Builder builder = (Request.Builder) builderField.get(httpClientRequest);
208+
Request request = builder.build();
209+
210+
assertEquals("Request method should be DELETE", "DELETE", request.method());
211+
212+
OkHttpClient client = new OkHttpClient.Builder()
213+
.connectTimeout(10, TimeUnit.SECONDS)
214+
.readTimeout(10, TimeUnit.SECONDS)
215+
.build();
216+
217+
try (Response response = client.newCall(request).execute()) {
218+
// httpbin.org/delete returns 200 for DELETE, 405 for GET
219+
assertEquals("httpbin.org/delete should return 200 for DELETE",
220+
200, response.code());
221+
} catch (IOException e) {
222+
// Network may not be available in emulator — skip gracefully
223+
System.out.println("httpbin.org test skipped due to network: " + e.getMessage());
224+
}
225+
}
226+
}

‎Build/libHttpClient.Android/src/androidTest/res/xml/network_security_config.xml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<network-security-config>
3+
<!-- Allow cleartext HTTP to localhost for MockWebServer instrumentation tests -->
4+
<domain-config cleartextTrafficPermitted="true">
5+
<domain includeSubdomains="true">localhost</domain>
6+
<domain includeSubdomains="true">127.0.0.1</domain>
7+
</domain-config>
8+
</network-security-config>

‎Build/libHttpClient.Android/src/main/java/com/xbox/httpclient/HttpClientRequest.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,20 +66,18 @@ public void setHttpUrl(String url) {
6666
public void setHttpMethodAndBody(String method, long call, String contentType, long contentLength) {
6767
RequestBody requestBody = null;
6868
if (contentLength == 0) {
69-
if ("POST".equals(method) || "PUT".equals(method)) {
69+
if ("POST".equals(method) || "PUT".equals(method) || "PATCH".equals(method)) {
7070
MediaType mediaType = (contentType != null ? MediaType.parse(contentType) : null);
7171
requestBody = RequestBody.create(NO_BODY, mediaType);
72-
73-
this.requestBuilder.method(method, requestBody);
7472
}
7573
} else {
7674
requestBody = new HttpClientRequestBody(call, contentType, contentLength);
7775

7876
// Decorate the request body to keep track of the upload progress
7977
CountingRequestBody countingBody = new CountingRequestBody(requestBody, uploadProgressListener, call);
80-
81-
this.requestBuilder.method(method, countingBody);
78+
requestBody = countingBody;
8279
}
80+
this.requestBuilder.method(method, requestBody);
8381
}
8482

8583
@SuppressWarnings("unused")

0 commit comments

Comments
 (0)