Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

INTERNAL: Make the -S option available with ascii protocol #819

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

namsic
Copy link
Collaborator

@namsic namsic commented Jan 16, 2025

๐Ÿ”— Related Issue

  • jam2in/arcus-works#634

โŒจ๏ธ What I did

-S option์„ ascii protocol๊ณผ ํ•จ๊ป˜ ์‚ฌ์šฉํ•  ์ˆ˜ ์žˆ๋„๋ก ํ™•์žฅํ•ฉ๋‹ˆ๋‹ค.
ascii protocol์—์„œ SASL ์ธ์ฆ ์ˆ˜ํ–‰ํ•  ๋ฐฉ๋ฒ•์ด ์—†๋Š” ์ƒํƒœ์ด๋ฏ€๋กœ,
-S ์˜ต์…˜ ์‚ฌ์šฉ ์‹œ ascii conn์—์„œ๋Š” ํ—ˆ์šฉ๋œ ๋ช…๋ น๋งŒ ์‚ฌ์šฉํ•  ์ˆ˜ ์žˆ์Šต๋‹ˆ๋‹ค.

  1. ๊ธฐ์กด์—๋Š” -S option์„ ์„ค์ •ํ•˜๋ฉด binary protocol์œผ๋กœ ๊ณ ์ •๋˜๊ณ ,
    -B option์—์„œ auto๋‚˜ ascii๋ฅผ ๋ช…์‹œํ•œ ์ƒํƒœ์—์„œ -S ํ•จ๊ป˜ ์ง€์ •ํ•˜๋ฉด ๊ตฌ๋™ ์‹คํŒจํ•ฉ๋‹ˆ๋‹ค.

  2. ๋ณ€๊ฒฝ ํ›„์—๋Š” -S option์„ ์„ค์ •ํ•˜๋”๋ผ๋„ ascii protocol์„ ์‚ฌ์šฉํ•  ์ˆ˜ ์žˆ์Šต๋‹ˆ๋‹ค.

  3. -S option์„ ์„ค์ •ํ•˜์—ฌ ๊ตฌ๋™ํ•œ ๊ฒฝ์šฐ์— ascii protocol ์‚ฌ์šฉํ•˜๋Š” ์—ฐ๊ฒฐ์€
    ํ—ˆ์šฉ๋˜์ง€ ์•Š์€ ๋ช…๋ น์— ๋Œ€ํ•ด CLIENT_ERROR unauthenticated ์‘๋‹ตํ•ฉ๋‹ˆ๋‹ค.

@namsic namsic requested a review from jhpark816 January 16, 2025 09:08
memcached.c Outdated
(strcmp(tokens[COMMAND_TOKEN].value, "shutdown") == 0) ||
(strcmp(tokens[COMMAND_TOKEN].value, "stats") == 0) ||
(strcmp(tokens[COMMAND_TOKEN].value, "version") == 0) ||
(strcmp(tokens[COMMAND_TOKEN].value, "zkensemble") == 0)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

flush ๋ช…๋ น์€ ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

์ผ๋ฐ˜์ ์œผ๋กœ ์ ‘๊ทผ ์ œํ•œ์ด๋ผ๋Š” ํ‘œํ˜„์€ read/write๋ฅผ ์•„์šฐ๋ฅด๋Š” ์˜๋ฏธ๋กœ ์‚ฌ์šฉํ•˜๋Š” ๊ฒฝ์šฐ๊ฐ€ ๋งŽ๊ณ 
flush๋Š” ์บ์‹œ ์•„์ดํ…œ์— ๋Œ€ํ•œ write ์„ฑ๊ฒฉ์˜ ๋ช…๋ น์ด๋ผ๊ณ  ์ƒ๊ฐํ•ด์„œ ํ—ˆ์šฉ ๋ช…๋ น์—์„œ ์ œ์™ธํ–ˆ์Šต๋‹ˆ๋‹ค.

๊ทธ๋Ÿฐ๋ฐ ์ƒ๊ฐํ•ด ๋ณด๋‹ˆ ์‘์šฉ์—์„œ ํŠน์ • key๋‚˜ prefix๋ฅผ ์ œ๊ฑฐํ•ด ๋‹ฌ๋ผ๋Š” ์š”์ฒญ์ด ์žˆ์„ ์ˆ˜ ์žˆ๊ณ 
flush, delete ๋ช…๋ น์„ ํ—ˆ์šฉํ•œ๋‹ค๊ณ  ํ•ด์„œ ์ €์žฅ๋œ ์ •๋ณด๋ฅผ ์•Œ ์ˆ˜ ์žˆ๊ฒŒ ๋˜๋Š” ๊ฒƒ์ด ์•„๋‹ˆ๋ฏ€๋กœ
์ด๋Ÿฌํ•œ ๋ช…๋ น์„ ํ—ˆ์šฉํ•˜๋Š” ๊ฒƒ์ด ์œ ์šฉํ•œ๊ฐ€ ํ•˜๋Š” ์ƒ๊ฐ๋„ ๋“ค๊ธฐ๋Š” ํ•ฉ๋‹ˆ๋‹ค.

์ œ ์ƒ๊ฐ์— flush๋ฅผ ์šด์˜ ๊ณ„์—ด์˜ ๋ช…๋ น์œผ๋กœ ๋ถ„๋ฅ˜ํ–ˆ์„ ๋•Œ ๋ฌธ์ œ๊ฐ€ ๋˜๋Š” ๋ถ€๋ถ„์€,
์ธ์ฆํ•˜์ง€ ์•Š์€ ์‚ฌ์šฉ์ž or ๋ชจ๋‹ˆํ„ฐ๋ง ํ”„๋กœ์„ธ์Šค๊ฐ€ ์บ์‹œ ๋ฐ์ดํ„ฐ๋ฅผ ์ „๋ถ€ ์ œ๊ฑฐํ•  ์ˆ˜ ์žˆ๋‹ค๊ณ  ํ•ด์„ํ•  ์—ฌ์ง€๊ฐ€ ์žˆ๋‹ค๋Š” ์  ์ž…๋‹ˆ๋‹ค.

Copy link
Collaborator

@jhpark816 jhpark816 left a comment

Choose a reason for hiding this comment

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

๋ฆฌ๋ทฐ ์™„๋ฃŒ

}

return rv;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

์•„๋ž˜ ํ˜•ํƒœ๋กœ ๋ณ€๊ฒฝํ•˜์‹œ์ฃ . ์•„๋ž˜ ์ฝ”๋“œ๊ฐ€ ์ง๊ด€์ ์œผ๋กœ ์ฝ๊ธฐ๊ฐ€ ์‰ฝ์Šต๋‹ˆ๋‹ค.

static bool authenticated_ascii(conn *c, token_t *tokens, const size_t ntokens)
{
    if (c->authenticated) {
        return true;
    }
    if ((ntokens >= 2) &&
        (strcmp(tokens[COMMAND_TOKEN].value, "dump") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "cmdlog") == 0 ||
         . . . 
         strcmp(tokens[COMMAND_TOKEN].value, "version") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "zkensemble") == 0)) {
        return true;
    }
    if ((ntokens >= 3) &&
        (strcmp(tokens[COMMAND_TOKEN].value, "set") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "get") == 0)) {
        return (strncmp(tokens[KEY_TOKEN].value, "arcus:", 6) == 0);
    }
    if ((ntokens >= 5) &&
        (strcmp(tokens[COMMAND_TOKEN].value, "lop") == 0 ||
         ...
         strcmp(tokens[COMMAND_TOKEN].value, "bop") == 0)) {
        return (strncmp(tokens[COMMAND_TOKEN+2].value, "arcus:", 6) == 0);
    }
    return false;
}

@namsic namsic force-pushed the authenticated_ascii branch from adf98c2 to d2f6f61 Compare January 31, 2025 08:43
@namsic
Copy link
Collaborator Author

namsic commented Jan 31, 2025

  • ํ—ˆ์šฉ ๋ช…๋ น์— flush_XXX ๋ช…๋ น ์ถ”๊ฐ€
  • ์กฐ๊ฑด๋ฌธ ์ฝ”๋“œ ๋ณ€๊ฒฝ

@namsic namsic force-pushed the authenticated_ascii branch from d2f6f61 to bb9358e Compare January 31, 2025 08:44
strcmp(tokens[COMMAND_TOKEN].value, "bop") == 0)) {
return (strncmp(tokens[COMMAND_TOKEN+2].value, "arcus:", 6) == 0);
}
return false;
Copy link
Collaborator

@jhpark816 jhpark816 Jan 31, 2025

Choose a reason for hiding this comment

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

@namsic
์•„๋ž˜์™€ ๊ฐ™์ด ์กฐ๊ธˆ์ด๋ผ๋„ ์ตœ์ ํ™”ํ•˜๋Š” ๋ฐฉ์•ˆ์„ ๊ฒ€ํ† ํ•ด ์ฃผ์„ธ์š”.

    if ((ntokens >= 5) &&
        (strcmp(tokens[COMMAND_TOKEN].value+1, "op ") == 0)) { // lop, sop, mop, bop
        return (strncmp(tokens[COMMAND_TOKEN+2].value, "arcus:", 6) == 0);
    }
    if ((ntokens >= 3) &&
        (strcmp(tokens[COMMAND_TOKEN].value+1, "et ") == 0)) { // get, set
        return (strncmp(tokens[KEY_TOKEN].value, "arcus:", 6) == 0);
    }
    if ((ntokens >= 2) &&
        (strcmp(tokens[COMMAND_TOKEN].value, "dump") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "cmdlog") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "config") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "flush_all") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "flush_prefix") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "getattr") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "help") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "lqdetect") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "scan") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "setattr") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "stats") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "zkensemble") == 0)) {
        return true;
    }
    if ((ntokens == 2) &&
        (strcmp(tokens[COMMAND_TOKEN].value, "quit") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "ready") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "version") == 0 ||
         strcmp(tokens[COMMAND_TOKEN].value, "shutdown") == 0)) {
        return true;
    } 

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

์ฒซ ๊ธ€์ž ํ™•์ธํ•˜์—ฌ switchํ•œ ๋’ค ๋ฌธ์ž์—ด ๋น„๊ตํ•˜๋Š” ๊ตฌํ˜„๋„ ๊ฒ€ํ† ํ•  ๋งŒํ•œ ๊ฒƒ ๊ฐ™์Šต๋‹ˆ๋‹ค.

https://github.com/naver/arcus-c-client/blob/09c014d1fd0a97f3ab19ea46422251a4db3dbf25/libmemcached/response.cc#L281

  switch(buffer[0])
  {
  case 'V':
    if (memcmp(buffer, "VALUE", 5) == 0)
    {
      /* We add back in one because we will need to search for END */
      memcached_server_response_increment(ptr);
      return textual_value_fetch(ptr, buffer, buffer_length, result);
    }
    else if (memcmp(buffer, "VERSION", 7) == 0)
    {
      /* Find the space, and then move one past it to copy version */
      char *version_ptr= index(buffer, ' ');
      version_ptr++;
      return textual_version_fetch(ptr, version_ptr);
    }
    break;

Copy link
Collaborator

Choose a reason for hiding this comment

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

@namsic
c client ๋ฐฉ์‹์œผ๋กœ ๊ตฌํ˜„ํ•ฉ์‹œ๋‹ค.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants