Skip to content

Commit 638fbb0

Browse files
committed
Clean up indexing of ByteBufferView
Motivation: ByteBufferView is not zero indexed, but parseSimpleString assumes it is. Modifications: - Correctly compute on the distance between two indices. - New, somewhat contrived, test case. Result: No functional change: because RediStack assumes the remote peer will always correctly terminate with /r/n, there is no point at which this code could misbehave in the current implementation. However, with small changes it is possible to trigger it, as the new test demonstrates.
1 parent f957937 commit 638fbb0

File tree

3 files changed

+14
-1
lines changed

3 files changed

+14
-1
lines changed

Sources/RediStack/RESP/RESPTranslator.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ extension RESPTranslator {
167167
let bytes = buffer.readableBytesView
168168
guard
169169
let newlineIndex = bytes.firstIndex(of: .newline),
170-
newlineIndex >= 2 // strings should at least have a CRLF ending
170+
newlineIndex - bytes.startIndex >= 1 // strings should at least have a CRLF ending
171171
else { return nil }
172172

173173
// grab the bytes that we've determined is the full simple string,

Tests/RediStackTests/RESPTranslatorTests.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,18 @@ extension RESPTranslatorTests {
299299

300300
return RESPValue.simpleString(stringBuffer).string
301301
}
302+
303+
func testSimpleStringWithoutRemovingToken() {
304+
var buffer = allocator.buffer(capacity: 2)
305+
buffer.writeString("\r\n")
306+
307+
guard let stringBuffer = parser.parseSimpleString(from: &buffer) else {
308+
XCTFail("Did not return")
309+
return
310+
}
311+
312+
XCTAssertEqual(RESPValue.simpleString(stringBuffer).string, "")
313+
}
302314
}
303315

304316
// MARK: Integers

Tests/RediStackTests/XCTestManifests.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ extension RESPTranslatorTests {
3939
("testParsing_simpleString_withContent", testParsing_simpleString_withContent),
4040
("testParsing_simpleString_withNoContent", testParsing_simpleString_withNoContent),
4141
("testParsing_simpleString", testParsing_simpleString),
42+
("testSimpleStringWithoutRemovingToken", testSimpleStringWithoutRemovingToken),
4243
("testWriting_arrays", testWriting_arrays),
4344
("testWriting_bulkStrings", testWriting_bulkStrings),
4445
("testWriting_errors", testWriting_errors),

0 commit comments

Comments
 (0)