Skip to content

Commit 5f880b0

Browse files
authored
Compression Query Paramater Fix (#368)
This PR resolves an issue we found with `unaryGET` requests between iOS and Android libraries when looking at our CDN logs. ## Background We noticed that we were appending `compression=identity` to all our iOS requests and not our equivalent Android requests. That led me on quite a journey to understand how this all works. The [connect-kotlin](https://github.com/connectrpc/connect-kotlin/blob/01db4796e47bf5f2a4b677ec1ea6f483a2344bd7/library/src/main/kotlin/com/connectrpc/protocols/ConnectInterceptor.kt#L362-L364) and [connect-swift](https://github.com/connectrpc/connect-swift/blob/main/Libraries/Connect/Internal/Interceptors/ConnectInterceptor.swift#L250-L260) implementations are not currently the same, and the changes proposed in this PR bring them as close together as possible without a much larger refactor to the iOS approach. <img width="1729" height="164" alt="Screenshot 2025-09-19 at 5 46 40 PM" src="https://github.com/user-attachments/assets/04df5cb5-0944-4611-baf5-6f8da560662b" /> ## Proposal Prior to this change, the `unaryGET` logic was incorrectly always appending the `compression=identity` query parameter even when compression wasn't being applied. This logic has now been modified ( with ✅ tests ✅ added 😎 ) to only apply the query parameter when the `Content-Type` header has been set. You cannot use the `requestCompression` object attached to the `ProtocolClientConfig` since the data has already been compressed when it reaches this part of the code. Because of this, I opted to use the `Content-Type` header as the signal to apply the query parameter. Please have a deeper look at this to see if you'd rather use the same approach as the Kotlin library does. I think it would be better to use the `requestCompression` object to determine whether it should be set or not, but that will require an order of operations refactor to this system. Cheers. 🍻 --------- Signed-off-by: Christian Noon <christian.noon@gmail.com>
1 parent 3f7b4a9 commit 5f880b0

File tree

2 files changed

+61
-6
lines changed

2 files changed

+61
-6
lines changed

Libraries/Connect/Internal/Interceptors/ConnectInterceptor.swift

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -241,23 +241,28 @@ extension ConnectInterceptor: StreamInterceptor {
241241
}
242242
}
243243

244-
private extension ProtocolClientConfig {
244+
extension ProtocolClientConfig {
245245
func transformToGETIfNeeded(_ request: HTTPRequest<Data?>) -> HTTPRequest<Data?> {
246246
guard self.shouldUseUnaryGET(for: request) else {
247247
return request
248248
}
249249

250250
var components = URLComponents(url: request.url, resolvingAgainstBaseURL: true)
251-
components?.queryItems = [
251+
252+
var queryItems: [URLQueryItem] = [
252253
URLQueryItem(name: "base64", value: "1"),
253-
URLQueryItem(
254-
name: "compression",
255-
value: request.headers[HeaderConstants.contentEncoding]?.first ?? "identity"
256-
),
257254
URLQueryItem(name: "connect", value: "v\(ConnectInterceptor.protocolVersion)"),
258255
URLQueryItem(name: "encoding", value: self.codec.name()),
259256
URLQueryItem(name: "message", value: request.message?.base64EncodedString()),
260257
]
258+
259+
if let contentEncoding = request.headers[HeaderConstants.contentEncoding]?.first {
260+
let queryItem = URLQueryItem(name: "compression", value: contentEncoding)
261+
queryItems.append(queryItem)
262+
}
263+
264+
components?.queryItems = queryItems
265+
261266
guard let url = components?.url else {
262267
return request
263268
}

Tests/UnitTests/ConnectLibraryTests/ConnectTests/ProtocolClientConfigTests.swift

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,4 +213,54 @@ final class ProtocolClientConfigTests: XCTestCase {
213213
unaryGET: .disabled
214214
).shouldUseUnaryGET(for: request))
215215
}
216+
217+
func testTransformToGETWithoutRequestCompression() {
218+
let request = HTTPRequest<Data?>(
219+
url: URL(string: "https://connectrpc.com")!,
220+
headers: Headers(),
221+
message: Data([0x0, 0x1, 0x2]),
222+
method: .post,
223+
trailers: nil,
224+
idempotencyLevel: .noSideEffects
225+
)
226+
227+
let config = ProtocolClientConfig(
228+
host: "https://connectrpc.com",
229+
networkProtocol: .connect,
230+
unaryGET: .alwaysEnabled
231+
)
232+
233+
let requestWithoutCompression = config.transformToGETIfNeeded(request)
234+
235+
XCTAssertEqual(requestWithoutCompression.method, .get)
236+
XCTAssertEqual(
237+
requestWithoutCompression.url.absoluteString,
238+
"https://connectrpc.com?base64=1&connect=v1&encoding=json&message=AAEC"
239+
)
240+
}
241+
242+
func testTransformToGETWithRequestCompression() {
243+
let compressedRequest = HTTPRequest<Data?>(
244+
url: URL(string: "https://connectrpc.com")!,
245+
headers: [HeaderConstants.contentEncoding: ["gzip"]], // mimics inceptor behavior
246+
message: Data([0x0, 0x1, 0x2]),
247+
method: .post,
248+
trailers: nil,
249+
idempotencyLevel: .noSideEffects
250+
)
251+
252+
let config = ProtocolClientConfig(
253+
host: "https://connectrpc.com",
254+
networkProtocol: .connect,
255+
unaryGET: .alwaysEnabled
256+
)
257+
258+
let requestWithCompression = config.transformToGETIfNeeded(compressedRequest)
259+
260+
XCTAssertEqual(requestWithCompression.method, .get)
261+
XCTAssertEqual(
262+
requestWithCompression.url.absoluteString,
263+
"https://connectrpc.com?base64=1&connect=v1&encoding=json&message=AAEC&compression=gzip"
264+
)
265+
}
216266
}

0 commit comments

Comments
 (0)