ENHANCE: Support binary protocol authentication #995

uhm0311 · 2025-10-28T07:33:43Z

🔗 Related Issue

https://github.com/jam2in/arcus-works/issues/754#issuecomment-3449309985

⌨️ What I did

Binary 프로토콜에서도 auth 기능을 사용할 수 있도록 합니다.
Binary의 finishedPayload() 메소드를 Ascii의 handleLine()과 최대한 동일하게 구현합니다.
- 단, sasl mech 명령어의 handleError() 로직을 finishedPayload() 메소드에 포함합니다.
다른 Binary 연산 클래스의 오버라이드된 finishedPayload() 메소드 구현을 참고하여, resetInput() 메소드 호출을 추가합니다.

jhpark816

리뷰 완료

jhpark816 · 2025-10-28T08:41:56Z

src/main/java/net/spy/memcached/protocol/binary/SASLBaseOperationImpl.java

+      complete(new OperationStatus(false, line, StatusCode.fromAsciiLine(line)));
+      throw new AuthException(line);
    }
+    resetInput();


resetInput() 호출이 필요한 이유를 간단히 설명해 주세요.

resetInput()의 원래 용도는 한 번에 모든 패킷을 읽지 않는 연산, 예시로는 mget 연산의 응답을 읽기 위해 호출하는 용도입니다.
mget과 같은 연산의 응답은 <header><body> 형태의 패킷이 N개로 구성되어 있습니다.
이러한 응답을 읽을 때에는 24 바이트 길이의 header를 먼저 읽고, header에 명시된 body 길이만큼 읽은 후에는 그 다음 24 바이트 길이의 header를 읽는 과정으로 돌아갑니다.
이 때 resetInput()을 호출해주면 기존에 패킷을 읽었던 정보를 버리는 역할을 합니다.
Ascii 커맨드로 비유하면, handleLine()과 handleRead()를 번갈아가며 호출하여 mget 응답을 읽기 위해 setReadType()을 호출하는 것과 같습니다.

위와 같은 resetInput()의 구현과 용도를 고려하면 auth 연산에서는 호출할 필요는 없지만, 다른 연산의 오버로드된 finishedPayload() 메소드에서 호출되는 것을 그대로 사용했습니다.

resetInput() 호출은 제거합시다.
오버로드된 decodePayload() 메소드를 가진다고 resetInput() 호출하는 것이 아니기 때문입니다.

참고로, 다른 연산에서는 아래 메소드를 override하여 응답 처리를 구현하였습니다.

getStatusForErrorCode()

decodePayload()

auth 처리에서도 기존 틀을 따라 위의 메소드를 override하여 구현하면 좋겠지만,
그게 잘 맞지 않아서, finishedPayload 메소드를 overrider한 것으로 생각했습니다.

jhpark816 · 2025-10-28T09:19:09Z

src/main/java/net/spy/memcached/protocol/binary/SASLMechsOperationImpl.java

+      if (isInternal) {
+        throw new AuthException(line);
+      }
+    }


추가 코멘트입니다.
SASLBaseOperation과 일관되게 구현 한다면, 아래와 같아야 하지 않는 지?

} else if (isInternal && errorCode == UNKNOWN_COMMAND) { complete(SASL_NOT_SUPPORTED); } else { String line = new String(pl); complete(new OperationStatus(false, line, StatusCode.fromAsciiLine(line))); throw new AuthException(line); }

SASLBaseOperation의 처리는 엄밀히 따지면 Ascii 프로토콜의 동작을 그대로 가져올 수 없었던 부분입니다.

Ascii 프로토콜의 경우 handleLine() 호출과 handleError() 호출은 CLIENT_ERROR | SERVER_ERROR | ERROR 접두사를 갖고 있는지 여부에 따라 분기가 됩니다.
즉, handleLine() 내에서는 절대로 CLIENT_ERROR | SERVER_ERROR | ERROR 접두사가 붙지 않는다는 점이 보장됩니다.
반대로 handleError() 내에서는 항상 CLIENT_ERROR | SERVER_ERROR | ERROR 접두사가 붙는다는 점이 보장됩니다.

Binary 프로토콜에서는 위와 같이 에러인지 아닌지 구분은 없고, finishedPayload() 메소드가 호출됩니다.
따라서 finishedPayload() 메소드를 오버라이드하게 된다면, finishedPayload() 메소드에서 Ascii의 handleLine()에 해당하는 로직과 handleError()에 해당하는 로직을 모두 넣어야 합니다.

SASLBaseOperation의 경우 handleError() 처리를 하기 위해서는 CLIENT_ERROR | SERVER_ERROR | ERROR로 분류되는 모든 에러 코드에 해당하는 경우에 대한 분기 처리를 추가하면 됩니다.
SASLMechsOperation의 경우 unknown command 에러에 한하여 Ascii 프로토콜과 동일한 동작을 하도록 하였으며, 그 외에 CLIENT_ERROR | SERVER_ERROR | ERROR로 분류되는 모든 에러 코드는 AuthException을 전파시키게 됩니다.

지난 번에 얘기한 대로,
Ascii Protocol에서 CLIENT_ERROR | SERVER_ERROR | ERROR 오류일 경우
현재 연결에 남은 garbage data를 제거하기 위해 immediate reconnect 합니다.

Binary Protocol은 다릅니다.
Binary에서는 예상하지 않은 응답이 올 경우 SERVER_ERROR 오류로 처리하는 데요.
그 이유를 코드를 통해 확인해 보면 좋겠지만,
확인해 보지 않은 상태라면 기존 코드를 그대로 유지하자고 했습니다.

sasl mech 연산이 외부에서 요청될 수 있으므로,
internal 경우가 아니면, 아래와 같이 기존 코드를 유지하는 것이 나을 것 같습니다.

} else if (isInternal) { if (errorCode == UNKNOWN_COMMAND) { complete(SASL_NOT_SUPPORTED); } else { String line = new String(pl); complete(new OperationStatus(false, line, StatusCode.fromAsciiLine(line))); throw new AuthException(line); } } else { super.finishedPayload(pl); }

uhm0311 requested a review from jhpark816 October 28, 2025 07:33

jhpark816 reviewed Oct 28, 2025

View reviewed changes

ENHANCE: Support binary protocol authentication

edcd175

uhm0311 force-pushed the uhm0311/f/1 branch from 19fc2bc to edcd175 Compare October 28, 2025 11:06

uhm0311 requested a review from jhpark816 October 28, 2025 11:06

jhpark816 approved these changes Oct 28, 2025

View reviewed changes

jhpark816 merged commit 55e5339 into naver:develop Oct 28, 2025
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ENHANCE: Support binary protocol authentication #995

ENHANCE: Support binary protocol authentication #995

Uh oh!

uhm0311 commented Oct 28, 2025 •

edited

Loading

Uh oh!

jhpark816 left a comment

Uh oh!

jhpark816 Oct 28, 2025

Uh oh!

uhm0311 Oct 28, 2025

Uh oh!

jhpark816 Oct 28, 2025

Uh oh!

jhpark816 Oct 28, 2025

Uh oh!

uhm0311 Oct 28, 2025

Uh oh!

jhpark816 Oct 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ENHANCE: Support binary protocol authentication #995

ENHANCE: Support binary protocol authentication #995

Uh oh!

Conversation

uhm0311 commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Related Issue

⌨️ What I did

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

jhpark816 Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

uhm0311 Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

jhpark816 Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

jhpark816 Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

uhm0311 Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

jhpark816 Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

uhm0311 commented Oct 28, 2025 •

edited

Loading