Skip to content

Commit

Permalink
Abort parsing if server data comes first
Browse files Browse the repository at this point in the history
Redis seems to only want client data first to request server data. The
DPD signature seems to pick up on some cases where server data comes
first, but is otherwise "valid" RESP. See if this helps lower FP rates.
  • Loading branch information
evantypanski committed Jan 29, 2025
1 parent 57b5a1b commit 0fdcd68
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 3 deletions.
17 changes: 14 additions & 3 deletions analyzer/resp.spicy
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,27 @@ const MAX_SIZE = 1024 * 1024;
const MAX_RECURSION_DEPTH = 20;

public type ClientMessages = unit {
%context = bool;
on %init {
*self.context() = True;
}
: ClientData[];
};

public type ServerMessages = unit {
%context = bool;
on %init {
if (!*self.context()) {
throw "Server responses must come after a client request is parsed";
}
}
: (ServerData &synchronize)[];
};

public type ClientData = unit {
on %init() { self.start = self.input(); }
on %init() {
self.start = self.input();
}

# Clients can only be an array or inline
ty: uint8 &convert=DataType($$) {
Expand Down Expand Up @@ -49,7 +61,7 @@ type BulkStringArray = unit {

type BulkStringWithTy = unit {
# Need to consume the type here
: uint8 &requires=$$=='$';
: uint8 &requires=$$ == '$';

length: RedisBytes &convert=$$.to_int(10) &requires=self.length <= int64(MAX_SIZE);
# NullBulkString is a BulkString with content unset
Expand All @@ -59,7 +71,6 @@ type BulkStringWithTy = unit {
: skip RedisBytes;
};


public type ServerData = unit {
%synchronize-after = b"\x0d\x0a";
var depth: uint8& = new uint8;
Expand Down
1 change: 1 addition & 0 deletions testing/Baseline/tests.start-with-server/output
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
Binary file added testing/Traces/start-with-server.pcap
Binary file not shown.
14 changes: 14 additions & 0 deletions testing/tests/start-with-server.zeek
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# @TEST-DOC: Test that Redis does not parse if it starts with the server data
#
# @TEST-EXEC: zeek -Cr ${TRACES}/start-with-server.pcap ${PACKAGE} %INPUT >output
# @TEST-EXEC: btest-diff output

event Redis::command(c: connection, is_orig: bool, command: Redis::Command)
{
print "BAD", command;
}

event Redis::server_data(c: connection, is_orig: bool, dat: Redis::ServerData)
{
print "BAD", dat;
}

0 comments on commit 0fdcd68

Please sign in to comment.