[
https://issues.apache.org/jira/browse/GEODE-4010?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16282210#comment-16282210
]
ASF GitHub Bot commented on GEODE-4010:
---------------------------------------
WireBaron closed pull request #1091: GEODE-4010: Don't use gossip with new
protocol on locator
URL: https://github.com/apache/geode/pull/1091
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/security/exception/IncompatibleAuthenticationMechanismsException.java
b/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/security/exception/IncompatibleAuthenticationMechanismsException.java
deleted file mode 100644
index e7995223e0..0000000000
---
a/geode-client-protocol/src/main/java/org/apache/geode/internal/protocol/security/exception/IncompatibleAuthenticationMechanismsException.java
+++ /dev/null
@@ -1,23 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
- * agreements. See the NOTICE file distributed with this work for additional
information regarding
- * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
- * "License"); you may not use this file except in compliance with the
License. You may obtain a
- * copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
distributed under the License
- * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
- * or implied. See the License for the specific language governing permissions
and limitations under
- * the License.
- */
-package org.apache.geode.internal.protocol.security.exception;
-
-import org.apache.geode.security.AuthenticationFailedException;
-
-public class IncompatibleAuthenticationMechanismsException extends
AuthenticationFailedException {
- public IncompatibleAuthenticationMechanismsException(String message) {
- super(message);
- }
-}
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
index aadbfb40bf..30dbaee8d6 100755
---
a/geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
+++
b/geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
@@ -103,6 +103,9 @@
private static/* GemStoneAddition */ final Map GOSSIP_TO_GEMFIRE_VERSION_MAP
= new HashMap();
+ // Any value below this is not a valid gossip version, but may be a
communication mode
+ private static final int MINIMUM_GOSSIP_VERSION = OLDGOSSIPVERSION;
+
/**
* For the new client-server protocol, which ignores the usual handshake
mechanism.
*/
@@ -379,11 +382,11 @@ private void processRequest(final Socket socket) {
+ (socket.getInetAddress().getHostAddress() + ":" +
socket.getPort()), e);
return;
}
- int gossipVersion = readGossipVersion(socket, input);
+ int gossipVersion = readGossipVersionOrCommunicationMode(socket,
input);
short versionOrdinal;
- if (gossipVersion == NON_GOSSIP_REQUEST_VERSION) {
- if (input.readUnsignedByte() == PROTOBUF_CLIENT_SERVER_PROTOCOL
+ if (gossipVersion < MINIMUM_GOSSIP_VERSION) {
+ if (gossipVersion == PROTOBUF_CLIENT_SERVER_PROTOCOL
&& Boolean.getBoolean("geode.feature-protobuf-protocol")) {
try {
int protocolVersion = input.readUnsignedByte();
@@ -400,11 +403,9 @@ private void processRequest(final Socket socket) {
}
} 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);
}
@@ -527,11 +528,15 @@ private void rejectUnknownProtocolConnection(Socket
socket, int gossipVersion)
socket.close();
}
- private int readGossipVersion(Socket sock, DataInputStream input) throws
Exception {
+ private int readGossipVersionOrCommunicationMode(Socket sock,
DataInputStream input)
+ throws Exception {
// read the first byte & check for an improperly configured client pool
trying
// to contact a cache server
int firstByte = input.readUnsignedByte();
if (CommunicationMode.isValidMode(firstByte)) {
+ if (CommunicationMode.isValidLocatorMode(firstByte)) {
+ return firstByte;
+ }
sock.getOutputStream().write(HandShake.REPLY_SERVER_IS_LOCATOR);
throw new Exception("Improperly configured client detected - use
addPoolLocator to "
+ "configure its locators instead of addPoolServer.");
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/CommunicationMode.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/CommunicationMode.java
index 053a556b58..6bd6d42bc6 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/CommunicationMode.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/CommunicationMode.java
@@ -131,6 +131,13 @@ public static boolean isValidMode(int mode) {
return 100 <= mode && mode <= 110;
}
+ /**
+ * check if the given communication mode is valid for locators
+ */
+ public static boolean isValidLocatorMode(int mode) {
+ return mode == 110;
+ }
+
public static CommunicationMode fromModeNumber(byte modeNumber) {
switch (modeNumber) {
case 100:
diff --git
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/security/AuthenticationRequestOperationHandler.java
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/security/AuthenticationRequestOperationHandler.java
index 3decb49f63..6f1381a260 100644
---
a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/security/AuthenticationRequestOperationHandler.java
+++
b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/security/AuthenticationRequestOperationHandler.java
@@ -30,7 +30,6 @@
import org.apache.geode.internal.protocol.protobuf.v1.ClientProtocol;
import org.apache.geode.internal.protocol.protobuf.v1.ConnectionAPI;
import
org.apache.geode.internal.protocol.protobuf.v1.utilities.ProtobufResponseUtilities;
-import
org.apache.geode.internal.protocol.security.exception.IncompatibleAuthenticationMechanismsException;
import org.apache.geode.internal.protocol.serialization.SerializationService;
import
org.apache.geode.internal.protocol.state.ConnectionAuthenticatingStateProcessor;
import
org.apache.geode.internal.protocol.state.exception.ConnectionStateException;
@@ -59,10 +58,6 @@
messageExecutionContext.setConnectionStateProcessor(stateProcessor.authenticate(properties));
return Success
.of(ConnectionAPI.AuthenticationResponse.newBuilder().setAuthenticated(true).build());
- } catch (IncompatibleAuthenticationMechanismsException e) {
- return Failure.of(ClientProtocol.ErrorResponse.newBuilder().setError(
- buildAndLogError(ProtocolErrorCode.UNSUPPORTED_AUTHENTICATION_MODE,
e.getMessage(), e))
- .build());
} catch (AuthenticationFailedException e) {
return Success
.of(ConnectionAPI.AuthenticationResponse.newBuilder().setAuthenticated(false).build());
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/LocatorIntegrationTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/LocatorIntegrationTest.java
new file mode 100644
index 0000000000..7ae3a88067
--- /dev/null
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/LocatorIntegrationTest.java
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.protocol.protobuf.v1;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.nio.channels.SocketChannel;
+import java.util.concurrent.TimeUnit;
+
+import org.awaitility.Awaitility;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.contrib.java.lang.system.RestoreSystemProperties;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.distributed.LocatorLauncher;
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.internal.cache.tier.CommunicationMode;
+import
org.apache.geode.internal.protocol.protobuf.v1.serializer.ProtobufProtocolSerializer;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+
+@Category(IntegrationTest.class)
+public class LocatorIntegrationTest {
+ @Rule
+ public final RestoreSystemProperties restoreSystemProperties = new
RestoreSystemProperties();
+
+ private OutputStream outputStream;
+ private InputStream inputStream;
+ private ProtobufProtocolSerializer protobufProtocolSerializer;
+ private Socket socket;
+ private SocketChannel socketChannel;
+ private LocatorLauncher locatorLauncher;
+
+ @Before
+ public void setUp() throws Exception {
+ System.setProperty("geode.feature-protobuf-protocol", "true");
+
+ int locatorPort = AvailablePortHelper.getRandomAvailableTCPPort();
+
+ LocatorLauncher.Builder builder = new LocatorLauncher.Builder();
+ builder.setPort(locatorPort);
+ locatorLauncher = builder.build();
+ locatorLauncher.start();
+
+ InetSocketAddress localhost = new InetSocketAddress("localhost",
locatorPort);
+ socketChannel = SocketChannel.open(localhost);
+
+ socket = socketChannel.socket();
+
+ Awaitility.await().atMost(5, TimeUnit.SECONDS).until(socket::isConnected);
+ outputStream = socket.getOutputStream();
+ inputStream = socket.getInputStream();
+
+ protobufProtocolSerializer = new ProtobufProtocolSerializer();
+ }
+
+ @After
+ public void tearDown() throws IOException {
+ if (socket.isConnected()) {
+ socket.close();
+ }
+ locatorLauncher.stop();
+ }
+
+ @Test
+ public void testLocatorMessageSucceedsAndDisconnects() throws Exception {
+
outputStream.write(CommunicationMode.ProtobufClientServerProtocol.getModeNumber());
+
outputStream.write(ConnectionAPI.MajorVersions.CURRENT_MAJOR_VERSION_VALUE);
+
+ ClientProtocol.Message.newBuilder()
+ .setRequest(ClientProtocol.Request.newBuilder()
+
.setGetAvailableServersRequest(LocatorAPI.GetAvailableServersRequest.newBuilder()))
+ .build().writeDelimitedTo(outputStream);
+ ClientProtocol.Message response =
protobufProtocolSerializer.deserialize(inputStream);
+
assertEquals(ClientProtocol.Response.GETAVAILABLESERVERSRESPONSE_FIELD_NUMBER,
+ response.getResponse().getResponseAPICase().getNumber());
+
+ // Set timeout to ensure test doesn't block forever if socket isn't closed
+ socket.setSoTimeout(5000);
+ assertEquals(-1, inputStream.read());
+ }
+}
diff --git
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionDUnitTest.java
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionDUnitTest.java
index ac432af9ca..e6a0683188 100644
---
a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionDUnitTest.java
+++
b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/acceptance/LocatorConnectionDUnitTest.java
@@ -73,7 +73,6 @@ private Socket createSocket() throws IOException {
int locatorPort = DistributedTestUtils.getDUnitLocatorPort();
Socket socket = new Socket(host.getHostName(), locatorPort);
DataOutputStream dataOutputStream = new
DataOutputStream(socket.getOutputStream());
- dataOutputStream.writeInt(0);
// Using the constant from AcceptorImpl to ensure that magic byte is the
same
dataOutputStream.writeByte(ProtobufClientServerProtocol.getModeNumber());
dataOutputStream.writeByte(ConnectionAPI.MajorVersions.CURRENT_MAJOR_VERSION_VALUE);
----------------------------------------------------------------
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]
> Locator should not require non-gossip request version to be written for new
> protocol
> ------------------------------------------------------------------------------------
>
> Key: GEODE-4010
> URL: https://issues.apache.org/jira/browse/GEODE-4010
> Project: Geode
> Issue Type: Bug
> Components: client/server
> Reporter: Michael Dodge
> Fix For: 1.4.0
>
>
> When connecting to a server using the new protocol, two bytes must be
> written, the communication mode and the major version. The locator, however,
> currently insists on four bytes of non-gossip request version (all zeros)
> being written _before_ the other two bytes. This insistence should be removed
> to ensure consistent behavior between locator and server for the new protocol.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
