Skip to content
Prev Previous commit
Next Next commit
reimpl decoder as state machine
  • Loading branch information
dkz2 committed May 31, 2023
commit 72b883e40231a4d1069762e111e823b49e91c1f7
21 changes: 21 additions & 0 deletions Sources/SwiftMemcache/Extensions/ByteBuffer+IntegerAsASCII.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,24 @@ extension ByteBuffer {
self.writeString(string)
}
}

extension ByteBuffer {
/// Checks if the next two bytes in the buffer are a carriage return and newline.
/// Does not consume any bytes.
func isNextEndOfLine() -> Bool {
return self.readableBytes >= 2 &&
self.getInteger(at: self.readerIndex, as: UInt8.self) == UInt8.carriageReturn &&
self.getInteger(at: self.readerIndex + 1, as: UInt8.self) == UInt8.newline
}

/// Consumes the next two bytes in the buffer if they are a carriage return and newline.
/// Returns `true` if the end of line was successfully consumed, `false` otherwise.
mutating func consumeEndOfLine() -> Bool {
guard self.isNextEndOfLine() else {
return false
}

self.moveReaderIndex(forwardBy: 2)
return true
}
}
10 changes: 6 additions & 4 deletions Sources/SwiftMemcache/MemcachedResponse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@
//===----------------------------------------------------------------------===//

struct MemcachedResponse {
enum ReturnCode: UInt16 {
enum ReturnCode {
case stored
case notStored
case exists
case notFound

init(_ value: UInt16) {
switch value {
init(_ bytes: UInt16) {
switch bytes {
case 0x4844: // "HD"
self = .stored
case 0x4E53: // "NS"
Expand All @@ -35,5 +35,7 @@ struct MemcachedResponse {
}
}

let returnCode: ReturnCode
var returnCode: ReturnCode
var dataLength: UInt64?
var flags: [UInt8]
}
213 changes: 148 additions & 65 deletions Sources/SwiftMemcache/MemcachedResponseDecoder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,84 +15,167 @@
import NIOCore
import NIOPosix

/// Responses look like:
///
/// <RC> <datalen*> <flag1> <flag2> <...>\r\n
///
/// Where <RC> is a 2 character return code. The number of flags returned are
/// based off of the flags supplied.
///
/// <datalen> is only for responses with payloads, with the return code 'VA'.
///
/// Flags are single character codes, ie 'q' or 'k' or 'I', which adjust the
/// behavior of the command. If a flag requests a response flag (ie 't' for TTL
/// remaining), it is returned in the same order as they were in the original
/// command, though this is not strict.
///
/// Flags are single character codes, ie 'q' or 'k' or 'O', which adjust the
/// behavior of a command. Flags may contain token arguments, which come after the
/// flag and before the next space or newline, ie 'Oopaque' or 'Kuserkey'. Flags
/// can return new data or reflect information, in the same order they were
/// supplied in the request. Sending an 't' flag with a get for an item with 20
/// seconds of TTL remaining, would return 't20' in the response.
///
/// All commands accept a tokens 'P' and 'L' which are completely ignored. The
/// arguments to 'P' and 'L' can be used as hints or path specifications to a
/// proxy or router inbetween a client and a memcached daemon. For example, a
/// client may prepend a "path" in the key itself: "mg /path/foo v" or in a proxy
/// token: "mg foo Lpath/ v" - the proxy may then optionally remove or forward the
/// token to a memcached daemon, which will ignore them.
///
/// Syntax errors are handled the same as noted under 'Error strings' section
/// below.
///
/// For usage examples beyond basic syntax, please see the wiki:
/// https://github.com/memcached/memcached/wiki/MetaCommands
struct MemcachedResponseDecoder: NIOSingleStepByteToMessageDecoder {
/// Responses look like:
///
/// <RC> <datalen*> <flag1> <flag2> <...>\r\n
///
/// Where <RC> is a 2 character return code. The number of flags returned are
/// based off of the flags supplied.
///
/// <datalen> is only for responses with payloads, with the return code 'VA'.
///
/// Flags are single character codes, ie 'q' or 'k' or 'I', which adjust the
/// behavior of the command. If a flag requests a response flag (ie 't' for TTL
/// remaining), it is returned in the same order as they were in the original
/// command, though this is not strict.
///
/// Flags are single character codes, ie 'q' or 'k' or 'O', which adjust the
/// behavior of a command. Flags may contain token arguments, which come after the
/// flag and before the next space or newline, ie 'Oopaque' or 'Kuserkey'. Flags
/// can return new data or reflect information, in the same order they were
/// supplied in the request. Sending an 't' flag with a get for an item with 20
/// seconds of TTL remaining, would return 't20' in the response.
///
/// All commands accept a tokens 'P' and 'L' which are completely ignored. The
/// arguments to 'P' and 'L' can be used as hints or path specifications to a
/// proxy or router inbetween a client and a memcached daemon. For example, a
/// client may prepend a "path" in the key itself: "mg /path/foo v" or in a proxy
/// token: "mg foo Lpath/ v" - the proxy may then optionally remove or forward the
/// token to a memcached daemon, which will ignore them.
///
/// Syntax errors are handled the same as noted under 'Error strings' section
/// below.
///
/// For usage examples beyond basic syntax, please see the wiki:
/// https://github.com/memcached/memcached/wiki/MetaCommands
typealias InboundOut = MemcachedResponse

func decode(buffer: inout ByteBuffer) throws -> InboundOut? {
// Ensure the buffer has at least 3 bytes (minimum for a response code and newline)
guard buffer.readableBytes >= 3 else {
return nil // Need more data
}
/// Describes the errors that can occur during the decoding process.
enum MemcachedDecoderError: Error {
/// This error is thrown when EOF is encountered but there are still
/// readable bytes in the buffer, which can indicate a bad message.
case unexpectedEOF

// Read the first two characters
guard let firstReturnCode = buffer.readInteger(as: UInt8.self),
let secondReturnCode = buffer.readInteger(as: UInt8.self) else {
preconditionFailure("Response code could not be read.")
}
/// This error is thrown when EOF is encountered but the decoder's next step
/// is not `.none`. This error suggests that the message ended prematurely,
/// possibly indicating a bad message.
case unexpectedNextStep(NextStep)
}

/// The next step that the decoder will take. The value of this enum determines how the decoder
/// processes the current state of the ByteBuffer.
enum NextStep: Hashable {
/// No further steps are needed, the decoding process is complete for the current message.
case none
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we never transition to that state and since our state machine is looping we can get rid of this

/// The initial step.
case returnCode
/// Decode the data length, flags or check if we are the end
case dataLengthOrFlag(MemcachedResponse.ReturnCode)
/// Decode the next flag
case decodeNextFlag(MemcachedResponse.ReturnCode, UInt64?, [UInt8])
/// Decode end of line
case decodeEndOfLine(MemcachedResponse.ReturnCode, UInt64?, [UInt8])
}

/// The action that the decoder will take in response to the current state of the ByteBuffer and the `NextStep`.
enum NextDecodeAction {
/// We need more bytes to decode the next step.
case waitForMoreBytes
/// We can continue decoding.
case continueDecodeLoop
/// We have decoded the next response and need to return it.
case returnDecodedResponse(MemcachedResponse)
}

let returnCode = MemcachedResponse.ReturnCode(
UInt16(firstReturnCode) << 8 | UInt16(secondReturnCode)
)
/// The next step in decoding.
var nextStep: NextStep = .returnCode

// If there is not a whitespace, then we are at the end of the line.
guard buffer.readableBytes > 0, let nextByte = buffer.getInteger(at: buffer.readerIndex, as: UInt8.self) else {
return nil // Need more dat
mutating func decode(buffer: inout ByteBuffer) throws -> InboundOut? {
while self.nextStep != .none {
switch try self.next(buffer: &buffer) {
case .returnDecodedResponse(let response):
return response

case .waitForMoreBytes:
return nil

case .continueDecodeLoop:
()
}
}
return nil
}

if nextByte != UInt8.whitespace {
// We're at the end of the line
buffer.moveReaderIndex(forwardBy: 1)
} else {
// We have additional data or flags to read
buffer.moveReaderIndex(forwardBy: 1)

// Assert that we really read \r\n
guard buffer.readableBytes >= 2,
buffer.getInteger(at: buffer.readerIndex, as: UInt8.self) == UInt8.carriageReturn,
buffer.getInteger(at: buffer.readerIndex + 1, as: UInt8.self) == UInt8.newline else {
preconditionFailure("Response ending '\r\n' not found.")
mutating func next(buffer: inout ByteBuffer) throws -> NextDecodeAction {
switch self.nextStep {
case .none:
return .waitForMoreBytes

case .returnCode:
guard let bytes = buffer.readInteger(as: UInt16.self) else {
return .waitForMoreBytes
}

let returnCode = MemcachedResponse.ReturnCode(bytes)
self.nextStep = .dataLengthOrFlag(returnCode)
return .continueDecodeLoop

case .dataLengthOrFlag(let returnCode):
if returnCode == .stored {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not correct we should only decode the data length if the returnCode is VA

// TODO: Implement decoding of data length
}

// Check if the next bytes are \r\n
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of checking if the next two bytes are something we should rather read the next byte and just check if it is a whitespace. If is a whitespace we have to decode flags otherwise we check if it is a \ and continue to read the next 3 bytes. It is fine if we return waitForMoreBytes when we got a \ and don't have 3 more bytes in the buffer. This should be rarely hit and we don't have to introduce a new state.

if buffer.consumeEndOfLine() {
let response = MemcachedResponse(returnCode: returnCode, dataLength: nil, flags: [])
self.nextStep = .returnCode
return .returnDecodedResponse(response)
} else {
self.nextStep = .decodeNextFlag(returnCode, nil, [])
return .continueDecodeLoop
}

case .decodeNextFlag(let returnCode, let dataLength, var flags):
if let nextByte = buffer.readInteger(as: UInt8.self), nextByte != UInt8.whitespace {
flags.append(nextByte)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is going to allocate a new array each time, I could ignore flags for the time being

self.nextStep = .decodeNextFlag(returnCode, dataLength, flags)
return .continueDecodeLoop
} else {
self.nextStep = .decodeEndOfLine(returnCode, dataLength, flags)
return .continueDecodeLoop
}

case .decodeEndOfLine(let returnCode, let dataLength, let flags):
guard buffer.consumeEndOfLine() else {
return .waitForMoreBytes
}

// Skip the CRLF
buffer.moveReaderIndex(forwardBy: 2)
let response = MemcachedResponse(returnCode: returnCode, dataLength: dataLength, flags: flags)
self.nextStep = .returnCode
return .returnDecodedResponse(response)
}

return MemcachedResponse(returnCode: returnCode)
}

func decodeLast(buffer: inout ByteBuffer, seenEOF: Bool) throws -> InboundOut? {
return try self.decode(buffer: &buffer)
mutating func decodeLast(buffer: inout ByteBuffer, seenEOF: Bool) throws -> MemcachedResponse? {
// Try to decode what is left in the buffer.
if let output = try self.decode(buffer: &buffer) {
return output
}

guard buffer.readableBytes == 0 || seenEOF else {
// If there are still readable bytes left and we haven't seen an EOF
// then something is wrong with the message or how we called the decoder.
throw MemcachedDecoderError.unexpectedEOF
}

switch self.nextStep {
case .none, .returnCode:
return nil
default:
throw MemcachedDecoderError.unexpectedNextStep(self.nextStep)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,20 @@ final class MemcachedResponseDecoderTests: XCTestCase {
let notFoundResponseCode = [UInt8(ascii: "N"), UInt8(ascii: "F")]
try testDecodeSetResponse(returnCode: notFoundResponseCode, expectedReturnCode: .notFound)
}
/*
func testDecodeSetNotStoredResponse() throws {
let notStoredReturnCode: UInt16 = (UInt16(UInt8(ascii: "N")) << 8) | UInt16(UInt8(ascii: "S"))
try testDecodeSetResponse(returnCode: notStoredReturnCode, expectedReturnCode: .notStored)
}

func testDecodeSetExistResponse() throws {
let existReturnCode: UInt16 = (UInt16(UInt8(ascii: "E")) << 8) | UInt16(UInt8(ascii: "X"))
try testDecodeSetResponse(returnCode: existReturnCode, expectedReturnCode: .exists)
}

func testDecodeSetNotFoundResponse() throws {
let notFoundReturnCode: UInt16 = (UInt16(UInt8(ascii: "N")) << 8) | UInt16(UInt8(ascii: "F"))
try testDecodeSetResponse(returnCode: notFoundReturnCode, expectedReturnCode: .notFound)
}
*/
}