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

GEODE-4300 Server logs warning message when protobuf client closes socket #1335

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ public GenericProtocolServerConnection(Socket socket, InternalCache c, CachedReg

@Override
protected void doOneMessage() {
Socket socket = this.getSocket();
try {
Socket socket = this.getSocket();
InputStream inputStream = socket.getInputStream();
OutputStream outputStream = socket.getOutputStream();

Expand All @@ -77,7 +77,9 @@ protected void doOneMessage() {
setClientDisconnectedException(e);
logger.debug("Encountered EOF while processing message: {}", e);
} catch (IOException | IncompatibleVersionException e) {
logger.warn(e);
if (!socket.isClosed()) { // GEODE-4300, IOException may be thrown thrown on EOF
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to put GEODE-4300 in the code since the commit comment also includes it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were actually complemented on doing this when VMWare audited the GemFire source base, but I understand where you're coming from.

logger.warn(e);
}
this.setFlagProcessMessagesAsFalse();
setClientDisconnectedException(e);
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1708,10 +1708,6 @@ public void setFlagProcessMessagesAsFalse() {
this.processMessages = false;
}

boolean getFlagProcessMessages() {
return this.processMessages;
}

public InternalLogWriter getLogWriter() {
return this.logWriter; // TODO:LOG:CONVERT: remove getLogWriter after callers are converted
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* 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.cache.tier.sockets;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.io.IOException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.Socket;
import java.net.UnknownHostException;

import org.junit.Rule;
import org.junit.Test;
import org.junit.contrib.java.lang.system.SystemOutRule;
import org.junit.experimental.categories.Category;

import org.apache.geode.cache.IncompatibleVersionException;
import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.client.protocol.ClientProtocolProcessor;
import org.apache.geode.internal.cache.tier.CachedRegionHelper;
import org.apache.geode.internal.cache.tier.CommunicationMode;
import org.apache.geode.internal.security.SecurityService;
import org.apache.geode.test.junit.categories.UnitTest;


@Category(UnitTest.class)
public class OutputCapturingServerConnectionTest {

@Rule
public SystemOutRule systemOutRule = new SystemOutRule().enableLog();

@Test
public void testEOFDoesNotCauseWarningMessage() throws IOException, IncompatibleVersionException {
Socket socketMock = mock(Socket.class);
when(socketMock.getInetAddress()).thenReturn(InetAddress.getByName("localhost"));
when(socketMock.isClosed()).thenReturn(true);

AcceptorImpl acceptorStub = mock(AcceptorImpl.class);
ClientProtocolProcessor clientProtocolProcessor = mock(ClientProtocolProcessor.class);
doThrow(new IOException()).when(clientProtocolProcessor).processMessage(any(), any());

ServerConnection serverConnection =
getServerConnection(socketMock, clientProtocolProcessor, acceptorStub);

String expectedMessage = "invoking doOneMessage";
String unexpectedMessage = "IOException";

// Create some stdout content so we can tell that the capture worked.
System.out.println(expectedMessage);

serverConnection.doOneMessage();

// verify that an IOException wasn't logged
String stdoutCapture = systemOutRule.getLog();
assertTrue(stdoutCapture.contains(expectedMessage));
assertFalse(stdoutCapture.contains(unexpectedMessage));
}

private GenericProtocolServerConnection getServerConnection(Socket socketMock,
ClientProtocolProcessor clientProtocolProcessorMock, AcceptorImpl acceptorStub)
throws UnknownHostException {
ClientHealthMonitor clientHealthMonitorMock = mock(ClientHealthMonitor.class);
when(acceptorStub.getClientHealthMonitor()).thenReturn(clientHealthMonitorMock);
InetSocketAddress inetSocketAddressStub = InetSocketAddress.createUnresolved("localhost", 9071);
InetAddress inetAddressStub = mock(InetAddress.class);
when(socketMock.getInetAddress()).thenReturn(InetAddress.getByName("localhost"));
when(socketMock.getRemoteSocketAddress()).thenReturn(inetSocketAddressStub);
when(socketMock.getInetAddress()).thenReturn(inetAddressStub);

return new GenericProtocolServerConnection(socketMock, mock(InternalCache.class),
mock(CachedRegionHelper.class), mock(CacheServerStats.class), 0, 0, "",
CommunicationMode.ProtobufClientServerProtocol.getModeNumber(), acceptorStub,
clientProtocolProcessorMock, mock(SecurityService.class));
}

}