Skip to content

Commit 6d0f8ed

Browse files
rebello95rebello95
authored
Resolve all outstanding conformance opt-outs (#271)
This PR includes updates to fix all outstanding conformance test failures and removes the corresponding opt-outs. All changes are validated by the conformance test suite. Many of these fixes are similar to connectrpc/connect-kotlin#248 and connectrpc/connect-kotlin#274. Resolves #268. Resolves #269. Resolves #270. This should also be one of the final changes before v1.0 #222. --------- Signed-off-by: Michael Rebello <[email protected]>
1 parent 12a86a1 commit 6d0f8ed

30 files changed

+555
-170
lines changed

‎Examples/ElizaCocoaPodsApp/Podfile.lock‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,4 @@ SPEC CHECKSUMS:
2020

2121
PODFILE CHECKSUM: b598f373a6ab5add976b09c2ac79029bf2200d48
2222

23-
COCOAPODS: 1.13.0
23+
COCOAPODS: 1.15.2

‎Libraries/Connect/Internal/Interceptors/ConnectInterceptor.swift‎

Lines changed: 59 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -83,28 +83,51 @@ extension ConnectInterceptor: UnaryInterceptor {
8383
] = current.value
8484
})
8585

86-
if let encoding = response.headers[HeaderConstants.contentEncoding]?.first,
87-
let compressionPool = self.config.responseCompressionPool(forName: encoding),
88-
let message = response.message.flatMap({ try? compressionPool.decompress(data: $0) })
86+
let finalResponse: HTTPResponse
87+
let contentType = response.headers[HeaderConstants.contentType]?.first ?? ""
88+
if response.code == .ok && !contentType.hasPrefix("application/\(self.config.codec.name())")
8989
{
90-
proceed(HTTPResponse(
91-
code: response.code,
92-
headers: headers,
93-
message: message,
94-
trailers: trailers,
95-
error: response.error,
90+
// If content-type looks like it could be an RPC server's response, consider
91+
// this an internal error.
92+
let code: Code = contentType.hasPrefix("application/") ? .internalError : .unknown
93+
finalResponse = HTTPResponse(
94+
code: code, headers: headers, message: nil, trailers: trailers,
95+
error: ConnectError(code: code, message: "unexpected content-type: \(contentType)"),
9696
tracingInfo: response.tracingInfo
97-
))
97+
)
98+
} else if let encoding = response.headers[HeaderConstants.contentEncoding]?.first {
99+
if let compressionPool = self.config.responseCompressionPool(forName: encoding),
100+
let message = response.message.flatMap({ try? compressionPool.decompress(data: $0) })
101+
{
102+
finalResponse = HTTPResponse(
103+
code: response.code,
104+
headers: headers,
105+
message: message,
106+
trailers: trailers,
107+
error: response.error,
108+
tracingInfo: response.tracingInfo
109+
)
110+
} else {
111+
finalResponse = HTTPResponse(
112+
code: .internalError,
113+
headers: headers,
114+
message: nil,
115+
trailers: trailers,
116+
error: ConnectError(code: .internalError, message: "unexpected encoding"),
117+
tracingInfo: response.tracingInfo
118+
)
119+
}
98120
} else {
99-
proceed(HTTPResponse(
121+
finalResponse = HTTPResponse(
100122
code: response.code,
101123
headers: headers,
102124
message: response.message,
103125
trailers: trailers,
104126
error: response.error,
105127
tracingInfo: response.tracingInfo
106-
))
128+
)
107129
}
130+
proceed(finalResponse)
108131
}
109132
}
110133

@@ -146,13 +169,36 @@ extension ConnectInterceptor: StreamInterceptor {
146169
switch result {
147170
case .headers(let headers):
148171
self.streamResponseHeaders.value = headers
149-
proceed(result)
150172

173+
let contentType = headers[HeaderConstants.contentType]?.first ?? ""
174+
if contentType != "application/connect+\(self.config.codec.name())" {
175+
// If content-type looks like it could be an RPC server's response, consider
176+
// this an internal error.
177+
let code: Code = contentType.hasPrefix("application/connect+")
178+
? .internalError
179+
: .unknown
180+
proceed(.complete(
181+
code: code, error: ConnectError(
182+
code: code, message: "unexpected content-type: \(contentType)"
183+
), trailers: nil
184+
))
185+
} else {
186+
proceed(result)
187+
}
151188
case .message(let data):
152189
do {
153190
let responseCompressionPool = self.streamResponseHeaders.value?[
154191
HeaderConstants.connectStreamingContentEncoding
155192
]?.first.flatMap { self.config.responseCompressionPool(forName: $0) }
193+
if responseCompressionPool == nil && Envelope.isCompressed(data) {
194+
proceed(.complete(
195+
code: .internalError, error: ConnectError(
196+
code: .internalError, message: "received unexpected compressed message"
197+
), trailers: nil
198+
))
199+
return
200+
}
201+
156202
let (headerByte, message) = try Envelope.unpackMessage(
157203
data, compressionPool: responseCompressionPool
158204
)

‎Libraries/Connect/Internal/Interceptors/GRPCWebInterceptor.swift‎

Lines changed: 86 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,39 @@ extension GRPCWebInterceptor: UnaryInterceptor {
6161
response.headers,
6262
trailers: response.trailers
6363
)
64+
if grpcCode != .ok || connectError != nil {
65+
proceed(HTTPResponse(
66+
// Rewrite the gRPC code if it is "ok" but `connectError` is non-nil.
67+
code: grpcCode == .ok ? .unknown : grpcCode,
68+
headers: response.headers,
69+
message: response.message,
70+
trailers: response.trailers,
71+
error: connectError,
72+
tracingInfo: response.tracingInfo
73+
))
74+
} else {
75+
proceed(HTTPResponse(
76+
code: .unimplemented,
77+
headers: response.headers,
78+
message: response.message,
79+
trailers: response.trailers,
80+
error: ConnectError(
81+
code: .unimplemented, message: "unary response has no message"
82+
),
83+
tracingInfo: response.tracingInfo
84+
))
85+
}
86+
return
87+
}
88+
89+
let contentType = response.headers[HeaderConstants.contentType]?.first ?? ""
90+
if response.code == .ok && !self.contentTypeIsExpectedGRPCWeb(contentType) {
91+
// If content-type looks like it could be a gRPC server's response, consider
92+
// this an internal error.
93+
let code: Code = self.contentTypeIsGRPCWeb(contentType) ? .internalError : .unknown
6494
proceed(HTTPResponse(
65-
code: grpcCode,
66-
headers: response.headers,
67-
message: response.message,
68-
trailers: response.trailers,
69-
error: connectError,
95+
code: code, headers: response.headers, message: nil, trailers: response.trailers,
96+
error: ConnectError(code: code, message: "unexpected content-type: \(contentType)"),
7097
tracingInfo: response.tracingInfo
7198
))
7299
return
@@ -75,6 +102,18 @@ extension GRPCWebInterceptor: UnaryInterceptor {
75102
let compressionPool = response.headers[HeaderConstants.grpcContentEncoding]?
76103
.first
77104
.flatMap { self.config.responseCompressionPool(forName: $0) }
105+
if compressionPool == nil && Envelope.isCompressed(responseData) {
106+
proceed(HTTPResponse(
107+
code: .internalError, headers: response.headers, message: nil,
108+
trailers: response.trailers,
109+
error: ConnectError(
110+
code: .internalError, message: "received unexpected compressed message"
111+
),
112+
tracingInfo: response.tracingInfo
113+
))
114+
return
115+
}
116+
78117
do {
79118
// gRPC Web returns data in 2 chunks (either/both of which may be compressed):
80119
// 1. OPTIONAL (when not trailers-only): The (headers and length prefixed)
@@ -107,7 +146,7 @@ extension GRPCWebInterceptor: UnaryInterceptor {
107146
}
108147
} catch let error {
109148
proceed(HTTPResponse(
110-
code: .unknown,
149+
code: .unimplemented,
111150
headers: response.headers,
112151
message: response.message,
113152
trailers: response.trailers,
@@ -146,6 +185,19 @@ extension GRPCWebInterceptor: StreamInterceptor {
146185
) {
147186
switch result {
148187
case .headers(let headers):
188+
let contentType = headers[HeaderConstants.contentType]?.first ?? ""
189+
if !self.contentTypeIsExpectedGRPCWeb(contentType) {
190+
// If content-type looks like it could be a gRPC server's response, consider
191+
// this an internal error.
192+
let code: Code = self.contentTypeIsGRPCWeb(contentType) ? .internalError : .unknown
193+
proceed(.complete(
194+
code: code, error: ConnectError(
195+
code: code, message: "unexpected content-type: \(contentType)"
196+
), trailers: headers
197+
))
198+
return
199+
}
200+
149201
if let grpcCode = headers.grpcStatus() {
150202
// Headers-only response.
151203
proceed(.complete(
@@ -193,9 +245,20 @@ extension GRPCWebInterceptor: StreamInterceptor {
193245
proceed(result)
194246
}
195247
}
196-
}
197248

198-
// MARK: - Private
249+
// MARK: - Private
250+
251+
private func contentTypeIsGRPCWeb(_ contentType: String) -> Bool {
252+
return contentType == "application/grpc-web"
253+
|| contentType.hasPrefix("application/grpc-web+")
254+
}
255+
256+
private func contentTypeIsExpectedGRPCWeb(_ contentType: String) -> Bool {
257+
let codecName = self.config.codec.name()
258+
return (codecName == "proto" && contentType == "application/grpc-web")
259+
|| contentType == "application/grpc-web+\(codecName)"
260+
}
261+
}
199262

200263
private struct TrailersDecodingError: Error {}
201264

@@ -228,13 +291,23 @@ private extension Trailers {
228291
private extension HTTPResponse {
229292
func withHandledGRPCWebTrailers(_ trailers: Trailers, message: Data?) -> Self {
230293
let (grpcCode, error) = ConnectError.parseGRPCHeaders(self.headers, trailers: trailers)
231-
if grpcCode == .ok {
294+
if grpcCode != .ok || error != nil {
232295
return HTTPResponse(
233-
code: grpcCode,
296+
// Rewrite the gRPC code if it is "ok" but `connectError` is non-nil.
297+
code: grpcCode == .ok ? .unknown : grpcCode,
234298
headers: self.headers,
235-
message: message,
299+
message: nil,
236300
trailers: trailers,
237-
error: nil,
301+
error: error,
302+
tracingInfo: self.tracingInfo
303+
)
304+
} else if message?.isEmpty != false {
305+
return HTTPResponse(
306+
code: .unimplemented,
307+
headers: self.headers,
308+
message: nil,
309+
trailers: trailers,
310+
error: ConnectError(code: .unimplemented, message: "unary response has no message"),
238311
tracingInfo: self.tracingInfo
239312
)
240313
} else {
@@ -243,7 +316,7 @@ private extension HTTPResponse {
243316
headers: self.headers,
244317
message: message,
245318
trailers: trailers,
246-
error: error,
319+
error: nil,
247320
tracingInfo: self.tracingInfo
248321
)
249322
}

‎Libraries/Connect/Internal/Streaming/BidirectionalAsyncStream.swift‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,14 @@
1414

1515
import SwiftProtobuf
1616

17-
/// Concrete implementation of `BidirectionalAsyncStreamInterface`.
17+
/// Concrete **internal** implementation of `BidirectionalAsyncStreamInterface`.
1818
/// Provides the necessary wiring to bridge from closures/callbacks to Swift's `AsyncStream`
1919
/// to work with async/await.
20+
///
21+
/// If the library removes callback support in favor of only supporting async/await in the future,
22+
/// this class can be simplified.
2023
@available(iOS 13, *)
21-
final class BidirectionalAsyncStream<
24+
class BidirectionalAsyncStream<
2225
Input: ProtobufMessage, Output: ProtobufMessage
2326
>: @unchecked Sendable {
2427
/// The underlying async stream that will be exposed to the consumer.
@@ -71,10 +74,10 @@ final class BidirectionalAsyncStream<
7174
}
7275

7376
/// Send a result to the consumer over the `results()` `AsyncStream`.
74-
/// Should be called by the protocol client when a result is received.
77+
/// Should be called by the protocol client when a result is received from the network.
7578
///
7679
/// - parameter result: The new result that was received.
77-
func receive(_ result: StreamResult<Output>) {
80+
func handleResultFromServer(_ result: StreamResult<Output>) {
7881
self.receiveResult(result)
7982
}
8083
}
@@ -103,7 +106,3 @@ extension BidirectionalAsyncStream: BidirectionalAsyncStreamInterface {
103106
self.requestCallbacks?.cancel()
104107
}
105108
}
106-
107-
// Conforms to the client-only interface since it matches exactly and the implementation is internal
108-
@available(iOS 13, *)
109-
extension BidirectionalAsyncStream: ClientOnlyAsyncStreamInterface {}

‎Libraries/Connect/Internal/Streaming/BidirectionalStream.swift‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
import SwiftProtobuf
1616

17-
/// Concrete implementation of `BidirectionalStreamInterface`.
17+
/// Concrete **internal** implementation of `BidirectionalStreamInterface`.
1818
final class BidirectionalStream<Message: ProtobufMessage>: Sendable {
1919
private let requestCallbacks: RequestCallbacks<Message>
2020

@@ -40,6 +40,3 @@ extension BidirectionalStream: BidirectionalStreamInterface {
4040
self.requestCallbacks.cancel()
4141
}
4242
}
43-
44-
// Conforms to the client-only interface since it matches exactly and the implementation is internal
45-
extension BidirectionalStream: ClientOnlyStreamInterface {}

‎Libraries/Connect/Internal/Streaming/ClientOnlyAsyncStream.swift‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Copyright 2022-2024 The Connect Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
import Foundation
16+
17+
/// Concrete **internal** implementation of `ClientOnlyAsyncStreamInterface`.
18+
/// Provides the necessary wiring to bridge from closures/callbacks to Swift's `AsyncStream`
19+
/// to work with async/await.
20+
///
21+
/// This subclasses `BidirectionalAsyncStream` since its behavior is purely additive (it overlays
22+
/// some additional validation) and both types are internal to the package, not public.
23+
@available(iOS 13, *)
24+
final class ClientOnlyAsyncStream<
25+
Input: ProtobufMessage, Output: ProtobufMessage
26+
>: BidirectionalAsyncStream<Input, Output> {
27+
private let receivedResults = Locked([StreamResult<Output>]())
28+
29+
override func handleResultFromServer(_ result: StreamResult<Output>) {
30+
let (isComplete, results) = self.receivedResults.perform { results in
31+
results.append(result)
32+
if case .complete = result {
33+
return (true, ClientOnlyStreamValidation.validatedFinalClientStreamResults(results))
34+
} else {
35+
return (false, [])
36+
}
37+
}
38+
guard isComplete else {
39+
return
40+
}
41+
results.forEach(super.handleResultFromServer)
42+
}
43+
}
44+
45+
@available(iOS 13, *)
46+
extension ClientOnlyAsyncStream: ClientOnlyAsyncStreamInterface {
47+
func closeAndReceive() {
48+
self.close()
49+
}
50+
}

0 commit comments

Comments
 (0)