Skip to content

Commit 8aa49aa

Browse files
committed
Fix logging IP version 6 addresses with scope in RediscoveryImpl (neo4j#1435)
The `Inet6Address.getHostAddress()` returns a string with `%scope-id` at the end if it is scoped. For instance, `fe80:0:0:0:ce66:1564:db8q:94b6%6`. The `RediscoveryImpl` error handling logic used to construct a log message with such address that it logged using the `Logger.warn(String, Object...)` method. Example: `log.warn(message)`. Some implementations of the `Logger` interface supply the values given directly to the `String.format(String, Object...)` method. Given that the first argument is considered to be a format string, an `IllegalFormatException` may be thrown. That used to happen when the log message contained the scoped IP version 6 `getHostAddress()` value since the log message was treated as the format string. This update fixes this issue by supplying the format string and the address values individually and letting the logger implementations to do the formatting as the log message should not be seen as the format string value.
1 parent cb09a4d commit 8aa49aa

File tree

2 files changed

+44
-4
lines changed

2 files changed

+44
-4
lines changed

driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -299,13 +299,12 @@ private ClusterComposition handleRoutingProcedureError(
299299
throw new CompletionException(error);
300300
}
301301

302-
// Retriable error happened during discovery.
302+
// Retryable error happened during discovery.
303303
DiscoveryException discoveryError =
304304
new DiscoveryException(format(RECOVERABLE_ROUTING_ERROR, routerAddress), error);
305305
Futures.combineErrors(baseError, discoveryError); // we record each failure here
306-
String warningMessage = format(RECOVERABLE_DISCOVERY_ERROR_WITH_SERVER, routerAddress);
307-
log.warn(warningMessage);
308-
log.debug(warningMessage, discoveryError);
306+
log.warn(RECOVERABLE_DISCOVERY_ERROR_WITH_SERVER, routerAddress);
307+
log.debug(format(RECOVERABLE_DISCOVERY_ERROR_WITH_SERVER, routerAddress), discoveryError);
309308
routingTable.forget(routerAddress);
310309
return null;
311310
}

driver/src/test/java/org/neo4j/driver/internal/cluster/RediscoveryTest.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import static org.junit.jupiter.api.Assertions.assertNotNull;
3030
import static org.junit.jupiter.api.Assertions.assertThrows;
3131
import static org.mockito.ArgumentMatchers.any;
32+
import static org.mockito.BDDMockito.given;
33+
import static org.mockito.Mockito.doAnswer;
3234
import static org.mockito.Mockito.mock;
3335
import static org.mockito.Mockito.never;
3436
import static org.mockito.Mockito.startsWith;
@@ -52,9 +54,11 @@
5254
import java.net.InetAddress;
5355
import java.net.UnknownHostException;
5456
import java.util.Arrays;
57+
import java.util.Collections;
5558
import java.util.HashMap;
5659
import java.util.List;
5760
import java.util.Map;
61+
import java.util.concurrent.CompletableFuture;
5862
import org.junit.jupiter.api.Test;
5963
import org.junit.jupiter.params.ParameterizedTest;
6064
import org.junit.jupiter.params.provider.ValueSource;
@@ -524,6 +528,43 @@ void shouldResolveToIP() throws UnknownHostException {
524528
assertEquals(new BoltServerAddress(A.host(), localhost.getHostAddress(), A.port()), addresses.get(0));
525529
}
526530

531+
@Test
532+
void shouldLogScopedIPV6AddressWithStringFormattingLogger() throws UnknownHostException {
533+
// GIVEN
534+
BoltServerAddress initialRouter = new BoltServerAddress("initialRouter", 7687);
535+
ClusterCompositionProvider compositionProvider = compositionProviderMock(Collections.emptyMap());
536+
ServerAddressResolver resolver = resolverMock(initialRouter, initialRouter);
537+
DomainNameResolver domainNameResolver = mock(DomainNameResolver.class);
538+
InetAddress address = mock(InetAddress.class);
539+
given(address.getHostAddress()).willReturn("fe80:0:0:0:ce66:1564:db8q:94b6%6");
540+
given(domainNameResolver.resolve(initialRouter.host())).willReturn(new InetAddress[] {address});
541+
RoutingTable table = routingTableMock(true);
542+
ConnectionPool pool = mock(ConnectionPool.class);
543+
CompletableFuture<Connection> failedAcquisition = new CompletableFuture<>();
544+
failedAcquisition.completeExceptionally(new ServiceUnavailableException("not available"));
545+
given(pool.acquire(any())).willReturn(failedAcquisition);
546+
Logging logging = mock(Logging.class);
547+
Logger logger = mock(Logger.class);
548+
given(logging.getLog(any(Class.class))).willReturn(logger);
549+
doAnswer(invocationOnMock -> String.format(invocationOnMock.getArgument(0), invocationOnMock.getArgument(1)))
550+
.when(logger)
551+
.warn(any());
552+
RoutingSettings settings = new RoutingSettings(1, 0, 0);
553+
RediscoveryImpl rediscovery = new RediscoveryImpl(
554+
initialRouter,
555+
settings,
556+
compositionProvider,
557+
GlobalEventExecutor.INSTANCE,
558+
resolver,
559+
logging,
560+
domainNameResolver);
561+
562+
// WHEN & THEN
563+
assertThrows(
564+
ServiceUnavailableException.class,
565+
() -> await(rediscovery.lookupClusterComposition(table, pool, null, null)));
566+
}
567+
527568
private Rediscovery newRediscovery(
528569
BoltServerAddress initialRouter,
529570
ClusterCompositionProvider compositionProvider,

0 commit comments

Comments
 (0)