8000 proxy: deduplicate protocol processing code · memcached/memcached@384a817 · GitHub
[go: up one dir, main page]
Skip to content

Commit 384a817

Browse files
dormandodormando
committed
proxy: deduplicate protocol processing code
This commit is step one of... a few... for standardizing the code paths for protocol handling. We started with three: - mcmc response parsing - proxy_request.c request parsing - proto_text.c (original tokenizer) Both requests and responses can now be tokenized by the same code from mcmc. This commit series does this in stages, which will result in removal of duplicated code and fixing many long standing issues. In this particular commit the duplicated tokenizer is removed from proxy_request.c, and mcmc functions are now used in various places.
1 parent 0e6ba4d commit 384a817

File tree

9 files changed

+271
-526
lines changed

9 files changed

+271
-526
lines changed

‎proto_proxy.c

Lines changed: 60 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,34 +1027,43 @@ static void proxy_process_command(conn *c, char *command, size_t cmdlen, bool mu
10271027
// might be better to split this function; the below bits turn into a
10281028
// function call, then we don't re-process the above bits in the same way?
10291029
// The way this is detected/passed on is very fragile.
1030-
if (!multiget && pr.cmd_type == CMD_TYPE_GET && pr.has_space) {
1031-
uint32_t keyoff = pr.tokens[pr.keytoken];
1032-
while (pr.klen != 0) {
1030+
if (!multiget && pr.cmd_type == CMD_TYPE_GET && pr.tok.ntokens > pr.keytoken+1) {
1031+
uint32_t keyoff = pr.tok.tokens[pr.keytoken];
1032+
const char *curkey = pr.request + keyoff;
1033+
int klen = pr.klen;
1034+
int cmd_prefix_len = pr.tok.tokens[pr.keytoken];
1035+
1036+
// TODO: TEST THIS CASE
1037+
// check once if our prefix is too long.
1038+
if (cmd_prefix_len > MAX_CMD_PREFIX) {
1039+
if (!resp_start(c)) {
1040+
conn_set_state(c, conn_closing);
1041+
return;
1042+
}
1043+
proxy_out_errstring(c->resp, PROXY_CLIENT_ERROR, "malformed request");
1044+
return;
1045+
}
1046+
1047+
while (klen != 0) {
10331048
char temp[KEY_MAX_LENGTH + MAX_CMD_PREFIX + 30];
10341049
char *cur = temp;
10351050
// Core daemon can abort the entire command if one key is bad, but
10361051
// we cannot from the proxy. Instead we have to inject errors into
10371052
// the stream. This should, thankfully, be rare at least.
1038-
if (pr.tokens[pr.keytoken] > MAX_CMD_PREFIX) {
1039-
if (!resp_start(c)) {
1040-
conn_set_state(c, conn_closing);
1041-
return;
1042-
}
1043-
proxy_out_errstring(c->resp, PROXY_CLIENT_ERROR, "malformed request");
1044-
} else if (pr.klen > KEY_MAX_LENGTH) {
1053+
if (klen > KEY_MAX_LENGTH) {
10451054
if (!resp_start(c)) {
10461055
conn_set_state(c, conn_closing);
10471056
return;
10481057
}
10491058
proxy_out_errstring(c->resp, PROXY_CLIENT_ERROR, "key too long");
10501059
} else {
10511060
// copy original request up until the original key token.
1052-
memcpy(cur, pr.request, pr.tokens[pr.keytoken]);
1053-
cur += pr.tokens[pr.keytoken];
1061+
memcpy(cur, pr.request, cmd_prefix_len);
1062+
cur += cmd_prefix_len;
10541063

10551064
// now copy in our "current" key.
1056-
memcpy(cur, &pr.request[keyoff], pr.klen);
1057-
cur += pr.klen;
1065+
memcpy(cur, curkey, klen);
1066+
cur += klen;
10581067

10591068
memcpy(cur, "\r\n", 2);
10601069
cur += 2;
@@ -1064,8 +1073,31 @@ static void proxy_process_command(conn *c, char *command, size_t cmdlen, bool mu
10641073
proxy_process_command(c, temp, cur - temp, PROCESS_MULTIGET);
10651074
}
10661075

1067-
// now advance to the next key.
1068-
keyoff = _process_request_next_key(&pr);
1076+
// FIXME: test multigets with excess space at the end.
1077+
// Find the next key manually. This keeps the special cased code
1078+
// here instead of adding extra junk to the parser structure.
1079+
curkey += klen;
1080+
int remain = pr.endlen - (curkey - pr.request);
1081+
while (remain) {
1082+
if (*curkey == ' ') {
1083+
remain--;
1084+
curkey++;
1085+
} else {
1086+
break;
1087+
}
1088+
}
1089+
1090+
if (remain) {
1091+
const char *s = memchr(curkey, ' ', remain);
1092+
if (s != NULL) {
1093+
klen = s - curkey;
1094+
} else {
1095+
klen = remain;
1096+
}
1097+
} else {
1098+
// no more keys
1099+
break;
1100+
}
10691101
}
10701102

10711103
if (!resp_start(c)) {
@@ -1211,20 +1243,25 @@ void mcp_set_resobj(mcp_resp_t *r, mcp_request_t *rq, mcp_backend_t *be, LIBEVEN
12111243
// the coroutine but the structure doesn't allow that yet.
12121244
// Should also be able to settle this exact mode from the parser so we
12131245
// don't have to re-branch here.
1214-
if (rq->pr.noreply) {
1215-
if (rq->pr.cmd_type == CMD_TYPE_META) {
1246+
if (rq->pr.cmd_type == CMD_TYPE_META) {
1247+
if (mcmc_token_has_flag_bit(&rq->pr.tok, (uint64_t)1<<48) == MCMC_OK) {
12161248
r->mode = RESP_MODE_METAQUIET;
1217-
for (int x = 2; x < rq->pr.ntokens; x++) {
1218-
if (rq->request[rq->pr.tokens[x]] == 'q') {
1219-
rq->request[rq->pr.tokens[x]] = ' ';
1249+
// FIXME: should this use the new interface? re-evaluate?
1250+
for (int x = 2; x < rq->pr.tok.ntokens; x++) {
1251+
if (rq->request[rq->pr.tok.tokens[x]] == 'q') {
1252+
rq->request[rq->pr.tok.tokens[x]] = ' ';
12201253
}
12211254
}
12221255
} else {
1256+
r->mode = RESP_MODE_NORMAL;
1257+
}
1258+
} else {
1259+
if (rq->pr.noreply) {
12231260
r->mode = RESP_MODE_NOREPLY;
12241261
rq->request[rq->pr.reqlen - 3] = 'Y';
1262+
} else {
1263+
r->mode = RESP_MODE_NORMAL;
12251264
}
1226-
} else {
1227-
r->mode = RESP_MODE_NORMAL;
12281265
}
12291266

12301267
r->cmd = rq->pr.command;

‎proto_text.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
#include "authfile.h"
1313
#include "storage.h"
1414
#include "base64.h"
15+
#ifdef TLS
1516
#include "tls.h"
17+
#endif
1618
#include <string.h>
1719
#include <stdlib.h>
1820

‎proxy.h

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -336,24 +336,18 @@ struct mcp_parser_meta_s {
336336
struct mcp_parser_s {
337337
const char *request;
338338
void *vbuf; // temporary buffer for holding value lengths.
339+
mcmc_tokenizer_t tok; // tokenizer structure
339340
uint8_t command;
340341
uint8_t cmd_type; // command class.
341-
uint8_t ntokens;
342342
uint8_t keytoken; // because GAT. sigh. also cmds without a key.
343-
uint32_t parsed; // how far into the request we parsed already
344343
uint32_t reqlen; // full length of request buffer.
345344
uint32_t endlen; // index to the start of \r\n or \n
346345
int vlen;
347346
uint32_t klen; // length of key.
348-
uint16_t tokens[PARSER_MAX_TOKENS]; // offsets for start of each token
349-
bool has_space; // a space was found after the last byte parsed.
350347
bool noreply; // if quiet/noreply mode is set.
351-
union {
352-
struct mcp_parser_meta_s meta;
353-
} t;
354348
};
355349

356-
#define MCP_PARSER_KEY(pr) (&pr.request[pr.tokens[pr.keytoken]])
350+
#define MCP_PARSER_KEY(pr) (&(pr)->request[(pr)->tok.tokens[(pr)->keytoken]])
357351

358352
#define MAX_REQ_TOKENS 2
359353
struct mcp_request_s {
@@ -856,9 +850,6 @@ mcp_backend_t *mcplib_pool_proxy_call_helper(mcp_pool_proxy_t *pp, const char *k
856850
void mcp_request_attach(mcp_request_t *rq, io_pending_proxy_t *p);
857851
int mcp_request_render(mcp_request_t *rq, int idx, char flag, const char *tok, size_t len);
858852
int mcp_request_append(mcp_request_t *rq, const char flag, const char *tok, size_t len);
859-
int mcp_request_find_flag_index(mcp_request_t *rq, const char flag);
860-
int mcp_request_find_flag_token(mcp_request_t *rq, const char flag, const char **token, size_t *len);
861-
int mcp_request_find_flag_tokenint64(mcp_request_t *rq, const char flag, int64_t *token);
862853
void proxy_lua_error(lua_State *L, const char *s);
863854
#define proxy_lua_ferror(L, fmt, ...) \
864855
do { \

‎proxy_inspector.c

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ static int mcp_inspector_sepkey_r(lua_State *L, struct mcp_inspector *ins, struc
203203
mcp_request_t *rq = arg;
204204
struct mcp_ins_sepkey *c = &s->c.sepkey;
205205

206-
const char *key = MCP_PARSER_KEY(rq->pr);
206+
const char *key = MCP_PARSER_KEY(&rq->pr);
207207
const char *end = key + rq->pr.klen;
208208
char sep = c->sep;
209209
int pos = c->pos;
@@ -267,7 +267,7 @@ static int mcp_inspector_keybegin_r(lua_State *L, struct mcp_inspector *ins, str
267267
mcp_request_t *rq = arg;
268268
struct mcp_ins_string *c = &s->c.string;
269269

270-
const char *key = MCP_PARSER_KEY(rq->pr);
270+
const char *key = MCP_PARSER_KEY(&rq->pr);
271271
int klen = rq->pr.klen;
272272
const char *str = ins->arena + c->str;
273273

@@ -284,7 +284,7 @@ static int mcp_inspector_keyis_r(lua_State *L, struct mcp_inspector *ins, struct
284284
mcp_request_t *rq = arg;
285285
struct mcp_ins_string *c = &s->c.string;
286286

287-
const char *key = MCP_PARSER_KEY(rq->pr);
287+
const char *key = MCP_PARSER_KEY(&rq->pr);
288288
int klen = rq->pr.klen;
289289
const char *str = ins->arena + c->str;
290290

@@ -302,7 +302,7 @@ static int mcp_inspector_hasflag_r(lua_State *L, struct mcp_inspector *ins, stru
302302
if (ins->type == INS_REQ) {
303303
mcp_request_t *rq = arg;
304304
// requests should always be tokenized, so we can just check the bit.
305-
if (rq->pr.t.meta.flags & c->bit) {
305+
if (mcmc_token_has_flag_bit(&rq->pr.tok, c->bit) == MCMC_OK) {
306306
lua_pushboolean(L, 1);
307307
} else {
308308
lua_pushboolean(L, 0);
@@ -332,11 +332,10 @@ static int mcp_inspector_flagtoken_r(lua_State *L, struct mcp_inspector *ins, st
332332
if (ins->type == INS_REQ) {
333333
mcp_request_t *rq = arg;
334334

335-
if (rq->pr.t.meta.flags & c->bit) {
335+
if (mcmc_token_has_flag_bit(&rq->pr.tok, c->bit) == MCMC_OK) {
336336
lua_pushboolean(L, 1); // flag exists
337-
const char *tok = NULL;
338-
size_t tlen = 0;
339-
mcp_request_find_flag_token(rq, c->f, &tok, &tlen);
337+
int tlen = 0;
338+
const char *tok = mcmc_token_get_flag(rq->pr.request, &rq->pr.tok, c->f, &tlen);
340339
lua_pushlstring(L, tok, tlen); // flag's token
341340
return 2;
342341
}
@@ -366,12 +365,14 @@ static int mcp_inspector_flagint_r(lua_State *L, struct mcp_inspector *ins, stru
366365
if (ins->type == INS_REQ) {
367366
mcp_request_t *rq = arg;
368367

369-
if (rq->pr.t.meta.flags & c->bit) {
368+
if (mcmc_token_has_flag_bit(&rq->pr.tok, c->bit) == MCMC_OK) {
370369
lua_pushboolean(L, 1); // flag exists
371370
int64_t tok = 0;
372-
if (mcp_request_find_flag_tokenint64(rq, c->f, &tok) == 0) {
371+
int status = mcmc_token_get_flag_64(rq->pr.request, &rq->pr.tok, c->f, &tok);
372+
if (status == MCMC_OK) {
373373
lua_pushinteger(L, tok);
374374
} else {
375+
// Potential error messages?
375376
lua_pushnil(L);
376377
}
377378
return 2;
@@ -431,18 +432,17 @@ static int mcp_inspector_flagstr_i(lua_State *L, int tidx, int sc, struct mcp_in
431432
return len;
432433
}
433434

434-
// FIXME: size_t vs int consistency for tlen would shorten the code.
435435
static int mcp_inspector_flagis_r(lua_State *L, struct mcp_inspector *ins, struct mcp_ins_step *s, void *arg) {
436436
struct mcp_ins_flagstr *c = &s->c.flagstr;
437437
const char *str = ins->arena + c->str;
438438
if (ins->type == INS_REQ) {
439439
mcp_request_t *rq = arg;
440440

441-
if (rq->pr.t.meta.flags & c->bit) {
441+
if (mcmc_token_has_flag_bit(&rq->pr.tok, c->bit) == MCMC_OK) {
442442
lua_pushboolean(L, 1); // flag exists
443-
const char *tok = NULL;
444-
size_t tlen = 0;
445-
mcp_request_find_flag_token(rq, c->f, &tok, &tlen);
443+
int tlen = -1;
444+
const char *tok = mcmc_token_get_flag(rq->pr.request, &rq->pr.tok, c->f, &tlen);
445+
446446
if (tlen == c->len && strncmp(tok, str, c->len) == 0) {
447447
lua_pushboolean(L, 1);
448448
} else {

0 commit comments

Comments
 (0)
0