Merge pull request #367 from ably/360-escape-roomName-and-serial

[ECO-5578] Encode room name and serial

Author: maratal

Sources/AblyChat/ChatAPI.swift

@@ -15,9 +15,21 @@ internal final class ChatAPI {
         self.realtime = realtime
     }
 
+    private func escapePathSegment(_ segment: String) -> String {
+        segment.encodePathSegment()
+    }
+
+    private func roomUrl(roomName: String, suffix: String = "") -> String {
+        "\(apiVersionV4)/rooms/\(escapePathSegment(roomName))\(suffix)" // CHA-RST6
+    }
+
+    private func messageUrl(roomName: String, serial: String, suffix: String = "") -> String {
+        "\(roomUrl(roomName: roomName, suffix: "/messages/"))\(escapePathSegment(serial))\(suffix)"
+    }
+
     // (CHA-M6) Messages should be queryable from a paginated REST API.
     internal func getMessages(roomName: String, params: HistoryParams) async throws(InternalError) -> some PaginatedResult<Message> {
-        let endpoint = "\(apiVersionV4)/rooms/\(roomName)/messages"
+        let endpoint = roomUrl(roomName: roomName, suffix: "/messages")
         return try await makePaginatedRequest(endpoint, params: params.asQueryItems())
     }
 
@@ -43,7 +55,7 @@ internal final class ChatAPI {
     // (CHA-M3) Messages are sent to Ably via the Chat REST API, using the send method.
     // (CHA-M3a) When a message is sent successfully, the caller shall receive a struct representing the Message in response (as if it were received via Realtime event).
     internal func sendMessage(roomName: String, params: SendMessageParams) async throws(InternalError) -> Message {
-        let endpoint = "\(apiVersionV4)/rooms/\(roomName)/messages"
+        let endpoint = roomUrl(roomName: roomName, suffix: "/messages")
         var body: [String: JSONValue] = ["text": .string(params.text)]
 
         // (CHA-M3b) A message may be sent without metadata or headers. When these are not specified by the user, they must be omitted from the REST payload.
@@ -62,7 +74,7 @@ internal final class ChatAPI {
     // (CHA-M8) A client must be able to update a message in a room.
     // (CHA-M8a) A client may update a message via the Chat REST API by calling the update method.
     internal func updateMessage(roomName: String, with modifiedMessage: Message, details: OperationDetails?) async throws(InternalError) -> Message {
-        let endpoint = "\(apiVersionV4)/rooms/\(roomName)/messages/\(modifiedMessage.serial)"
+        let endpoint = messageUrl(roomName: roomName, serial: modifiedMessage.serial)
         var body: [String: JSONValue] = [:]
         let messageObject: [String: JSONValue] = [
             "text": .string(modifiedMessage.text),
@@ -90,7 +102,7 @@ internal final class ChatAPI {
     // (CHA-M9) A client must be able to delete a message in a room.
     // (CHA-M9a) A client may delete a message via the Chat REST API by calling the delete method.
     internal func deleteMessage(roomName: String, message: Message, details: OperationDetails?) async throws(InternalError) -> Message {
-        let endpoint = "\(apiVersionV4)/rooms/\(roomName)/messages/\(message.serial)/delete"
+        let endpoint = messageUrl(roomName: roomName, serial: message.serial, suffix: "/delete")
         var body: [String: JSONValue] = [:]
 
         if let description = details?.description {
@@ -107,7 +119,7 @@ internal final class ChatAPI {
     }
 
     internal func getOccupancy(roomName: String) async throws(InternalError) -> OccupancyData {
-        let endpoint = "\(apiVersionV4)/rooms/\(roomName)/occupancy"
+        let endpoint = roomUrl(roomName: roomName, suffix: "/occupancy")
         return try await makeRequest(endpoint, method: "GET")
     }
 
@@ -118,7 +130,7 @@ internal final class ChatAPI {
             throw ChatError.messageReactionInvalidMessageSerial.toInternalError()
         }
 
-        let endpoint = "\(apiVersionV4)/rooms/\(roomName)/messages/\(messageSerial)/reactions"
+        let endpoint = messageUrl(roomName: roomName, serial: messageSerial, suffix: "/reactions")
 
         let ablyCocoaBody: [String: Any] = [
             "type": params.type.rawValue,
@@ -136,7 +148,7 @@ internal final class ChatAPI {
             throw ChatError.messageReactionInvalidMessageSerial.toInternalError()
         }
 
-        let endpoint = "\(apiVersionV4)/rooms/\(roomName)/messages/\(messageSerial)/reactions"
+        let endpoint = messageUrl(roomName: roomName, serial: messageSerial, suffix: "/reactions")
 
         var httpParams: [String: String] = [
             "type": params.type.rawValue,

Sources/AblyChat/Extensions/String+Extensions.swift

@@ -0,0 +1,16 @@
+import Foundation
+
+internal extension String {
+    // Source: https://datatracker.ietf.org/doc/html/rfc3986#section-3.3
+    // i.e. segment = unreserved / pct-encoded / sub-delims / ":" / "@", where
+    //  unreserved = ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~
+    //  pct-encoded = %XX
+    //  sub-delims = !$&'()*+,;=
+    func encodePathSegment() -> String {
+        let allowedSet = CharacterSet(charactersIn: "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~!$&'()*+,;=:@")
+        guard let escaped = addingPercentEncoding(withAllowedCharacters: allowedSet) else {
+            fatalError("String '\(self)' can't be percent encoded.")
+        }
+        return escaped
+    }
+}

Tests/AblyChatTests/ChatAPITests.swift

@@ -46,13 +46,13 @@ struct ChatAPITests {
 
         // Then
         let expectedMessage = Message(
-            serial: "3446456",
+            serial: "123456789-000@123456789:000",
             action: .messageCreate,
             clientID: "mockClientId",
             text: "hello",
             metadata: [:],
             headers: [:],
-            version: .init(serial: "3446456", timestamp: Date(timeIntervalSince1970: 1_631_840_000)),
+            version: .init(serial: "123456789-000@123456789:000", timestamp: Date(timeIntervalSince1970: 1_631_840_000)),
             timestamp: Date(timeIntervalSince1970: 1_631_840_000),
             reactions: .empty,
         )

Tests/AblyChatTests/DefaultMessageReactionsTests.swift

@@ -12,6 +12,7 @@ struct DefaultMessageReactionsTests {
     // @spec CHA-MR11b
     // @spec CHA-MR11b1
     // @spec CHA-MR11b2
+    // @specOneOf(5/6) CHA-RST6 - Escaping room name for API send/delete reaction
     @Test
     func sendAndDeleteReactionForMessage() async throws {
         // Given
@@ -20,7 +21,7 @@ struct DefaultMessageReactionsTests {
         }
         let chatAPI = ChatAPI(realtime: realtime)
         let channel = MockRealtimeChannel(initialState: .attached)
-        let defaultMessages = DefaultMessages(channel: channel, chatAPI: chatAPI, roomName: "basketball", logger: TestLogger())
+        let defaultMessages = DefaultMessages(channel: channel, chatAPI: chatAPI, roomName: "basket/ball", logger: TestLogger())
 
         // When
         let message = try await defaultMessages.send(withParams: .init(text: "a joke"))
@@ -32,7 +33,7 @@ struct DefaultMessageReactionsTests {
             matching: "request(_:path:params:body:headers:)",
             arguments: [
                 "method": "POST",
-                "path": "/chat/v4/rooms/basketball/messages/\(message.serial)/reactions",
+                "path": "/chat/v4/rooms/basket%2Fball/messages/123456789-000@123456789:000/reactions",
                 "body": [
                     "name": "😆",
                     "type": "reaction:multiple.v1",
@@ -46,7 +47,7 @@ struct DefaultMessageReactionsTests {
             matching: "request(_:path:params:body:headers:)",
             arguments: [
                 "method": "DELETE",
-                "path": "/chat/v4/rooms/basketball/messages/\(message.serial)/reactions",
+                "path": "/chat/v4/rooms/basket%2Fball/messages/123456789-000@123456789:000/reactions",
                 "body": [:],
                 "params": [
                     "name": "😆",
@@ -353,7 +354,7 @@ struct DefaultMessageReactionsTests {
             matching: "request(_:path:params:body:headers:)",
             arguments: [
                 "method": "POST",
-                "path": "/chat/v4/rooms/basketball/messages/\(message.serial)/reactions",
+                "path": "/chat/v4/rooms/basketball/messages/123456789-000@123456789:000/reactions",
                 "body": [
                     "name": "😆",
                     "type": "reaction:multiple.v1",
@@ -392,7 +393,7 @@ struct DefaultMessageReactionsTests {
             matching: "request(_:path:params:body:headers:)",
             arguments: [
                 "method": "POST",
-                "path": "/chat/v4/rooms/basketball/messages/\(message.serial)/reactions",
+                "path": "/chat/v4/rooms/basketball/messages/123456789-000@123456789:000/reactions",
                 "body": [
                     "name": "😆",
                     "type": "reaction:distinct.v1",

Tests/AblyChatTests/DefaultMessagesTests.swift

@@ -9,6 +9,7 @@ struct DefaultMessagesTests {
     // @spec CHA-M3a
     // @spec CHA-M3b
     // @spec CHA-M3f
+    // @specOneOf(1/6) CHA-RST6 - Escaping room name for API send message
     @Test
     func sendMessage() async throws {
         // Given
@@ -35,7 +36,7 @@ struct DefaultMessagesTests {
         }
         let chatAPI = ChatAPI(realtime: realtime)
         let channel = MockRealtimeChannel(initialState: .attached)
-        let defaultMessages = DefaultMessages(channel: channel, chatAPI: chatAPI, roomName: "basketball", logger: TestLogger())
+        let defaultMessages = DefaultMessages(channel: channel, chatAPI: chatAPI, roomName: "basket/ball", logger: TestLogger())
 
         // When
         let sentMessage = try await defaultMessages.send(withParams: .init(text: "hey", metadata: ["key1": "val1"], headers: ["key2": "val2"]))
@@ -52,12 +53,13 @@ struct DefaultMessagesTests {
         #expect(sentMessage.timestamp == Date(timeIntervalSince1970: 1_631_840_000_000 / 1000))
         #expect(realtime.callRecorder.hasRecord(
             matching: "request(_:path:params:body:headers:)",
-            arguments: ["method": "POST", "path": "/chat/v4/rooms/basketball/messages", "body": ["text": "hey", "metadata": ["key1": "val1"], "headers": ["key2": "val2"]], "params": [:], "headers": [:]],
+            arguments: ["method": "POST", "path": "/chat/v4/rooms/basket%2Fball/messages", "body": ["text": "hey", "metadata": ["key1": "val1"], "headers": ["key2": "val2"]], "params": [:], "headers": [:]],
         ))
     }
 
     // @spec CHA-M8a
     // @spec CHA-M8b
+    // @specOneOf(2/6) CHA-RST6 - Escaping room name for API update message
     @Test
     func updateMessage() async throws {
         // Given
@@ -66,7 +68,7 @@ struct DefaultMessagesTests {
             MockHTTPPaginatedResponse(
                 items: [
                     [
-                        "serial": "0",
+                        "serial": "123456789-000@123456789:000",
                         "version": [
                             "serial": "1",
                             "metadata": ["key": "val"],
@@ -88,17 +90,17 @@ struct DefaultMessagesTests {
         }
         let chatAPI = ChatAPI(realtime: realtime)
         let channel = MockRealtimeChannel(initialState: .attached)
-        let defaultMessages = DefaultMessages(channel: channel, chatAPI: chatAPI, roomName: "basketball", logger: TestLogger())
+        let defaultMessages = DefaultMessages(channel: channel, chatAPI: chatAPI, roomName: "basket/ball", logger: TestLogger())
 
-        let sentMessage = try Message(jsonObject: ["serial": "0", "version": ["serial": "0"], "text": .string(text), "clientId": "0", "action": "message.create", "metadata": [:], "headers": [:]]) // arbitrary
+        let sentMessage = try Message(jsonObject: ["serial": "123456789-000@123456789:000", "version": ["serial": "123456789-000@123456789:000"], "text": .string(text), "clientId": "0", "action": "message.create", "metadata": [:], "headers": [:]]) // arbitrary
 
         // When
         var newMessage = sentMessage
         newMessage.text = text + "!" // see https://github.com/ably/ably-chat-swift/issues/333
         let updatedMessage = try await defaultMessages.update(newMessage: newMessage, details: .init(description: "add exclamation", metadata: ["key": "val"]))
 
         // Then
-        #expect(updatedMessage.serial == "0")
+        #expect(updatedMessage.serial == "123456789-000@123456789:000")
         #expect(updatedMessage.action == .messageUpdate)
         #expect(updatedMessage.text == "hey!")
         #expect(updatedMessage.clientID == "clientId")
@@ -110,12 +112,13 @@ struct DefaultMessagesTests {
         #expect(updatedMessage.timestamp == Date(timeIntervalSince1970: 1_631_840_000_000 / 1000))
         #expect(realtime.callRecorder.hasRecord(
             matching: "request(_:path:params:body:headers:)",
-            arguments: ["method": "PUT", "path": "/chat/v4/rooms/basketball/messages/\(sentMessage.serial)", "body": ["message": ["text": "hey!", "metadata": [:], "headers": [:]], "description": "add exclamation", "metadata": ["key": "val"]], "params": [:], "headers": [:]],
+            arguments: ["method": "PUT", "path": "/chat/v4/rooms/basket%2Fball/messages/123456789-000@123456789:000", "body": ["message": ["text": "hey!", "metadata": [:], "headers": [:]], "description": "add exclamation", "metadata": ["key": "val"]], "params": [:], "headers": [:]],
         ))
     }
 
     // @spec CHA-M9a
     // @spec CHA-M9b
+    // @specOneOf(3/6) CHA-RST6 - Escaping room name for API delete message
     @Test
     func deleteMessage() async throws {
         // Given
@@ -124,10 +127,10 @@ struct DefaultMessagesTests {
             MockHTTPPaginatedResponse(
                 items: [
                     [
-                        "serial": "0",
+                        "serial": "123456789-000@123456789:000",
                         "action": "message.delete",
                         "version": [
-                            "serial": "1",
+                            "serial": "123456789-000@123456789:000",
                             "timestamp": 1_631_840_030_000,
                             "clientId": "clientId2",
                         ],
@@ -144,16 +147,16 @@ struct DefaultMessagesTests {
         }
         let chatAPI = ChatAPI(realtime: realtime)
         let channel = MockRealtimeChannel(initialState: .attached)
-        let defaultMessages = DefaultMessages(channel: channel, chatAPI: chatAPI, roomName: "basketball", logger: TestLogger())
+        let defaultMessages = DefaultMessages(channel: channel, chatAPI: chatAPI, roomName: "basket/ball", logger: TestLogger())
 
-        let sentMessage = try Message(jsonObject: ["serial": "0", "version": ["serial": "0"], "text": .string(text), "clientId": "0", "action": "message.create", "metadata": ["key": "val"], "headers": [:]]) // arbitrary
+        let sentMessage = try Message(jsonObject: ["serial": "123456789-000@123456789:000", "version": ["serial": "123456789-000@123456789:000"], "text": .string(text), "clientId": "0", "action": "message.create", "metadata": ["key": "val"], "headers": [:]]) // arbitrary
 
         // When
         let deletedMessage = try await defaultMessages.delete(message: sentMessage, details: nil)
 
         // Then
-        #expect(deletedMessage.serial == "0")
-        #expect(deletedMessage.version.serial == "1")
+        #expect(deletedMessage.serial == "123456789-000@123456789:000")
+        #expect(deletedMessage.version.serial == "123456789-000@123456789:000")
         #expect(deletedMessage.version.timestamp == Date(timeIntervalSince1970: 1_631_840_030_000 / 1000))
         #expect(deletedMessage.version.clientID == "clientId2")
         #expect(deletedMessage.action == .messageDelete)
@@ -163,7 +166,7 @@ struct DefaultMessagesTests {
         #expect(deletedMessage.timestamp == Date(timeIntervalSince1970: 1_631_840_000_000 / 1000))
         #expect(realtime.callRecorder.hasRecord(
             matching: "request(_:path:params:body:headers:)",
-            arguments: ["method": "POST", "path": "/chat/v4/rooms/basketball/messages/\(sentMessage.serial)/delete", "body": [:], "params": [:], "headers": [:]],
+            arguments: ["method": "POST", "path": "/chat/v4/rooms/basket%2Fball/messages/123456789-000@123456789:000/delete", "body": [:], "params": [:], "headers": [:]],
         ))
     }
 
@@ -239,6 +242,7 @@ struct DefaultMessagesTests {
     }
 
     // @spec CHA-M5a
+    // @specOneOf(4/6) CHA-RST6 - Escaping room name for API get messages
     @Test
     func subscriptionPointIsChannelSerialWhenUnderlyingRealtimeChannelIsAttached() async throws {
         // Given
@@ -251,14 +255,14 @@ struct DefaultMessagesTests {
             properties: ARTChannelProperties(attachSerial: nil, channelSerial: channelSerial),
             initialState: .attached,
         )
-        let defaultMessages = DefaultMessages(channel: channel, chatAPI: chatAPI, roomName: "basketball", logger: TestLogger())
+        let defaultMessages = DefaultMessages(channel: channel, chatAPI: chatAPI, roomName: "basket/ball", logger: TestLogger())
         let subscription = defaultMessages.subscribe()
         _ = try await subscription.historyBeforeSubscribe(withParams: .init())
 
         // Then: subscription point is the current channelSerial of the realtime channel
         #expect(realtime.callRecorder.hasRecord(
             matching: "request(_:path:params:body:headers:)",
-            arguments: ["method": "GET", "path": "/chat/v4/rooms/basketball/messages", "body": [:], "params": ["direction": "backwards", "fromSerial": "\(channelSerial)"], "headers": [:]],
+            arguments: ["method": "GET", "path": "/chat/v4/rooms/basket%2Fball/messages", "body": [:], "params": ["direction": "backwards", "fromSerial": "\(channelSerial)"], "headers": [:]],
         ))
     }
 

Tests/AblyChatTests/DefaultRoomOccupancyTests.swift

@@ -6,6 +6,7 @@ import Testing
 struct DefaultRoomOccupancyTests {
     // @spec CHA-O3
     // @spec CHA-O7b
+    // @specOneOf(6/6) CHA-RST6 - Escaping room name for API get occupancy
     @Test
     func occupancyGet() async throws {
         // Given
@@ -24,7 +25,7 @@ struct DefaultRoomOccupancyTests {
         let defaultOccupancy = DefaultOccupancy(
             channel: channel,
             chatAPI: chatAPI,
-            roomName: "basketball",
+            roomName: "basket/ball",
             logger: TestLogger(),
             options: .init(enableEvents: true),
         )
@@ -38,6 +39,17 @@ struct DefaultRoomOccupancyTests {
 
         let currentOccupancy = defaultOccupancy.current
         #expect(currentOccupancy == nil)
+
+        #expect(realtime.callRecorder.hasRecord(
+            matching: "request(_:path:params:body:headers:)",
+            arguments: [
+                "method": "GET",
+                "path": "/chat/v4/rooms/basket%2Fball/occupancy",
+                "body": [:],
+                "params": [:],
+                "headers": [:],
+            ],
+        ))
     }
 
     // @specUntested CHA-O4e - We chose to implement this failure with an idiomatic fatalError instead of throwing, but we can’t test this.