[
https://issues.apache.org/jira/browse/GEODE-4059?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16282547#comment-16282547
]
ASF GitHub Bot commented on GEODE-4059:
---------------------------------------
WireBaron commented on a change in pull request #1137: GEODE-4059: Changing
protobuf handshake to not need communication mod…
URL: https://github.com/apache/geode/pull/1137#discussion_r155644472
##########
File path:
geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
##########
@@ -379,89 +379,13 @@ private void processRequest(final Socket socket) {
+ (socket.getInetAddress().getHostAddress() + ":" +
socket.getPort()), e);
return;
}
- int gossipVersion = readGossipVersion(socket, input);
-
- short versionOrdinal;
- if (gossipVersion == NON_GOSSIP_REQUEST_VERSION) {
- if (input.readUnsignedByte() == PROTOBUF_CLIENT_SERVER_PROTOCOL
- && Boolean.getBoolean("geode.feature-protobuf-protocol")) {
- try {
- int protocolVersion = input.readUnsignedByte();
- ClientProtocolService clientProtocolService =
- clientProtocolServiceLoader.lookupService(protocolVersion);
- clientProtocolService.initializeStatistics("LocatorStats",
- internalLocator.getDistributedSystem());
- try (ClientProtocolProcessor pipeline =
-
clientProtocolService.createProcessorForLocator(internalLocator)) {
- pipeline.processMessage(input, socket.getOutputStream());
- } catch (IncompatibleVersionException e) {
- // should not happen on the locator as there is no handshake.
- log.error("Unexpected exception in client message processing",
e);
- }
- } catch (ServiceLoadingFailureException e) {
- log.error("There was an error looking up the client protocol
service", e);
- socket.close();
- throw new IOException("There was an error looking up the client
protocol service", e);
- } catch (ServiceVersionNotFoundException e) {
- log.error("Unable to find service matching the client protocol
version byte", e);
- socket.close();
- throw new IOException(
- "Unable to find service matching the client protocol version
byte", e);
- }
- } else {
- rejectUnknownProtocolConnection(socket, gossipVersion);
- }
- } else if (gossipVersion <= getCurrentGossipVersion()
- && GOSSIP_TO_GEMFIRE_VERSION_MAP.containsKey(gossipVersion)) {
- // Create a versioned stream to remember sender's GemFire version
- versionOrdinal = (short)
GOSSIP_TO_GEMFIRE_VERSION_MAP.get(gossipVersion);
-
- if (Version.GFE_71.compareTo(versionOrdinal) <= 0) {
- // Recent versions of TcpClient will send the version ordinal
- versionOrdinal = input.readShort();
- }
-
- if (log.isDebugEnabled() && versionOrdinal !=
Version.CURRENT_ORDINAL) {
- log.debug("Locator reading request from " +
socket.getInetAddress() + " with version "
- + Version.fromOrdinal(versionOrdinal, false));
- }
- input = new VersionedDataInputStream(input,
Version.fromOrdinal(versionOrdinal, false));
- request = DataSerializer.readObject(input);
- if (log.isDebugEnabled()) {
- log.debug("Locator received request " + request + " from " +
socket.getInetAddress());
- }
- if (request instanceof ShutdownRequest) {
- shuttingDown = true;
- // Don't call shutdown from within the worker thread, see java bug
#6576792.
- // Closing the socket will cause our acceptor thread to shutdown
the executor
- this.serverSocketPortAtClose = srv_sock.getLocalPort();
- srv_sock.close();
- response = new ShutdownResponse();
- } else if (request instanceof InfoRequest) {
- response = handleInfoRequest(request);
- } else if (request instanceof VersionRequest) {
- response = handleVersionRequest(request);
- } else {
- response = handler.processRequest(request);
- }
-
- handler.endRequest(request, startTime);
-
- startTime = DistributionStats.getStatTime();
- if (response != null) {
- DataOutputStream output = new
DataOutputStream(socket.getOutputStream());
- if (versionOrdinal != Version.CURRENT_ORDINAL) {
- output =
- new VersionedDataOutputStream(output,
Version.fromOrdinal(versionOrdinal, false));
- }
- DataSerializer.writeObject(response, output);
- output.flush();
- }
-
- handler.endResponse(request, startTime);
+ // read the first byte & check for an improperly configured client
pool trying
+ // to contact a cache server
+ int firstByte = input.readUnsignedByte();
+ if (firstByte != CommunicationMode.ReservedForGossip.getModeNumber()) {
+ handleNonGossipConnection(socket, input, firstByte);
} else {
- // Close the socket. We can not accept requests from a newer version
- rejectUnknownProtocolConnection(socket, gossipVersion);
+ processOneConnection(socket, startTime, input);
Review comment:
No, in this case the first byte was the high order byte of the gossip
version, and thus guaranteed to be 0.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Stop using magic bytes in protobuf connection establishment
> -----------------------------------------------------------
>
> Key: GEODE-4059
> URL: https://issues.apache.org/jira/browse/GEODE-4059
> Project: Geode
> Issue Type: New Feature
> Components: client/server
> Reporter: Brian Rowe
>
> When connection to the server, the client currently uses some magic bytes to
> identify the protocol being used. This is non-intuitive and problematic for
> client developers, and should not be necessary if we make clever use of a
> magic protobuf connection establishing message. This should also serve as
> our handshake.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
