ijuma commented on code in PR #18295:
URL: https://github.com/apache/kafka/pull/18295#discussion_r1896286562
##########
clients/src/main/java/org/apache/kafka/common/security/authenticator/SaslServerAuthenticator.java:
##########
@@ -503,63 +499,51 @@ private void handleSaslToken(byte[] clientToken) throws
IOException {
}
}
- private boolean handleKafkaRequest(byte[] requestBytes) throws
IOException, AuthenticationException {
- boolean isKafkaRequest = false;
- String clientMechanism = null;
+ /**
+ * @throws InvalidRequestException if the request is not in Kafka format
or if the API key is invalid. Clients
+ * that support SASL without support for KIP-43 (e.g. Kafka Clients 0.9.x)
are in the former bucket - the first
+ * packet such clients send is a GSSAPI token starting with 0x60.
+ */
+ private void handleKafkaRequest(byte[] requestBytes) throws IOException,
AuthenticationException {
try {
ByteBuffer requestBuffer = ByteBuffer.wrap(requestBytes);
RequestHeader header = RequestHeader.parse(requestBuffer);
ApiKeys apiKey = header.apiKey();
- // A valid Kafka request header was received. SASL authentication
tokens are now expected only
- // following a SaslHandshakeRequest since this is not a GSSAPI
client token from a Kafka 0.9.0.x client.
- if (saslState == SaslState.INITIAL_REQUEST)
- setSaslState(SaslState.HANDSHAKE_OR_VERSIONS_REQUEST);
- isKafkaRequest = true;
-
// Raise an error prior to parsing if the api cannot be handled at
this layer. This avoids
// unnecessary exposure to some of the more complex schema types.
if (apiKey != ApiKeys.API_VERSIONS && apiKey !=
ApiKeys.SASL_HANDSHAKE)
- throw new IllegalSaslStateException("Unexpected Kafka request
of type " + apiKey + " during SASL handshake.");
+ throw new InvalidRequestException("Unexpected Kafka request of
type " + apiKey + " during SASL handshake.");
Review Comment:
@rajinisivaram Do you know why we sent `IllegalSaslStateException` here and
`InvalidRequestException` for every other error? We seem to have special
handling for Sasl related exceptions, but it doesn't seem to make sense in this
case since we can't properly propagate an error if we get the wrong request
type.
##########
clients/src/main/java/org/apache/kafka/common/security/authenticator/SaslServerAuthenticator.java:
##########
@@ -503,63 +499,51 @@ private void handleSaslToken(byte[] clientToken) throws
IOException {
}
}
- private boolean handleKafkaRequest(byte[] requestBytes) throws
IOException, AuthenticationException {
- boolean isKafkaRequest = false;
- String clientMechanism = null;
+ /**
+ * @throws InvalidRequestException if the request is not in Kafka format
or if the API key is invalid. Clients
+ * that support SASL without support for KIP-43 (e.g. Kafka Clients 0.9.x)
are in the former bucket - the first
+ * packet such clients send is a GSSAPI token starting with 0x60.
+ */
+ private void handleKafkaRequest(byte[] requestBytes) throws IOException,
AuthenticationException {
try {
ByteBuffer requestBuffer = ByteBuffer.wrap(requestBytes);
RequestHeader header = RequestHeader.parse(requestBuffer);
ApiKeys apiKey = header.apiKey();
- // A valid Kafka request header was received. SASL authentication
tokens are now expected only
- // following a SaslHandshakeRequest since this is not a GSSAPI
client token from a Kafka 0.9.0.x client.
- if (saslState == SaslState.INITIAL_REQUEST)
- setSaslState(SaslState.HANDSHAKE_OR_VERSIONS_REQUEST);
- isKafkaRequest = true;
-
// Raise an error prior to parsing if the api cannot be handled at
this layer. This avoids
// unnecessary exposure to some of the more complex schema types.
if (apiKey != ApiKeys.API_VERSIONS && apiKey !=
ApiKeys.SASL_HANDSHAKE)
- throw new IllegalSaslStateException("Unexpected Kafka request
of type " + apiKey + " during SASL handshake.");
+ throw new InvalidRequestException("Unexpected Kafka request of
type " + apiKey + " during SASL handshake.");
Review Comment:
@rajinisivaram Do you know why we send `IllegalSaslStateException` here and
`InvalidRequestException` for every other error? We seem to have special
handling for Sasl related exceptions, but it doesn't seem to make sense in this
case since we can't properly propagate an error if we get the wrong request
type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
