Skip to content

Commit

Permalink
Merge pull request #15 from gchq/gh-14-ipv6-brackets
Browse files Browse the repository at this point in the history
PR for #14 - Change DeviceUtil to strip `[]` from IP addresses
  • Loading branch information
at055612 authored Apr 29, 2022
2 parents 545bfa8 + e4da164 commit ee5d95d
Show file tree
Hide file tree
Showing 7 changed files with 181 additions and 6 deletions.
6 changes: 3 additions & 3 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ buildscript {
mavenCentral()
}
dependencies {
classpath 'com.jfrog.bintray.gradle:gradle-bintray-plugin:1.8.4'
classpath group: 'ca.cutterslade.gradle', name: 'gradle-dependency-analyze', version: '1.5.2'
}
}
Expand Down Expand Up @@ -75,7 +74,7 @@ def eventLoggingSchemaVer = "v4.0-beta.3"
// see if/how the java model has changed following schema changes or changes
// to the jaxb code generation.
// *****************************************************************************
ext.previousReleaseVersion = "v5.0-beta.29_schema-v4.0-beta.3"
ext.previousReleaseVersion = "v5.0-beta.30_schema-v4.0-beta.3"
// *****************************************************************************


Expand Down Expand Up @@ -158,7 +157,7 @@ ext.versions = [
eventLogging : projectVersionForMaven,

//------------3rd-party------------
junit_jupiter : '5.7.2',
junit_jupiter : '5.8.2',
logback : '1.2.3',
mockito : '3.12.4',
]
Expand All @@ -179,6 +178,7 @@ ext.libs = [
jaxb_bom : "org.glassfish.jaxb:jaxb-bom:2.4.0-b180830.0438",
junit_jupiter_api : "org.junit.jupiter:junit-jupiter-api:$versions.junit_jupiter",
junit_jupiter_engine : "org.junit.jupiter:junit-jupiter-engine:$versions.junit_jupiter",
junit_jupiter_params : "org.junit.jupiter:junit-jupiter-params:$versions.junit_jupiter",
logback_classic : "ch.qos.logback:logback-classic:${versions.logback}",
logback_core : "ch.qos.logback:logback-core:${versions.logback}",
mockito_core : "org.mockito:mockito-core:$versions.mockito",
Expand Down
1 change: 1 addition & 0 deletions event-logging-api/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ dependencies {
runtimeOnly libs.jaxb_runtime

testImplementation libs.junit_jupiter_api
testImplementation libs.junit_jupiter_params
testImplementation libs.assertj_core
testImplementation libs.mockito_core

Expand Down
1 change: 1 addition & 0 deletions event-logging-base/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ dependencies {

//testImplementation "org.junit.jupiter:junit-jupiter-api:${versions.junit}"
testImplementation libs.junit_jupiter_api
testImplementation libs.junit_jupiter_params
//testImplementation "org.assertj:assertj-core:${versions.assertj}"
testImplementation libs.assertj_core
//testImplementation "org.mockito:mockito-core:${versions.mockito}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public static Device createDevice(final String hostName,
final String ipAddress) {
final Device device = new Device();
device.setHostName(hostName);
device.setIPAddress(ipAddress);
device.setIPAddress(getValidIP(ipAddress));
return device;
}

Expand Down Expand Up @@ -71,14 +71,35 @@ public static String getValidIP(final String ip) {
return null;
}

final String tmp = ip.trim().toUpperCase(Locale.ENGLISH);
String tmp = ip.trim().toUpperCase(Locale.ENGLISH);
if (tmp.length() == 0 || tmp.contains(UNKNOWN_IP)) {
return null;
}
tmp = stripIpv6Brackets(tmp);

return tmp;
}

/**
* IPv6 standard allows an IPv6 address to be surrounded by square brackets when
* used in URI/URLs. This method removes those brackets to ensure IP addresses are stored
* in the event in a consistent form.
*/
public static String stripIpv6Brackets(final String ip) {
if (ip == null || ip.isEmpty()) {
return ip;
} else {
String cleanedIp = ip;
if (cleanedIp.startsWith("[")) {
cleanedIp = cleanedIp.substring(1);
}
if (cleanedIp.endsWith("]")) {
cleanedIp = cleanedIp.substring(0, cleanedIp.length() - 1);
}
return cleanedIp;
}
}

/**
* Gets a canonical host name from an InetAddress.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
package event.logging.base.util;

import event.logging.Device;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.net.InetAddress;
import java.net.UnknownHostException;

class TestDeviceUtil {
private static final Logger LOGGER = LoggerFactory.getLogger(TestDeviceUtil.class);

@ParameterizedTest
@CsvSource(value = {
"'[0:0:0:0:0:0:0:1]','0:0:0:0:0:0:0:1'",
"'[]',''",
"'NULL','NULL'",
"'',''",
})
void stripIpv6Brackets(final String input, final String expectedOutput) {
final String cleanedIp = DeviceUtil.stripIpv6Brackets(input);

Assertions.assertThat(cleanedIp)
.isEqualTo(expectedOutput);

// Now clean the cleaned thing to make sure it is unchanged.
final String cleanedIp2 = DeviceUtil.stripIpv6Brackets(cleanedIp);

Assertions.assertThat(cleanedIp2)
.isEqualTo(cleanedIp);
}

@ParameterizedTest
@CsvSource(value = {
"'192.168.0.1','192.168.0.1'",
"' 192.168.0.1','192.168.0.1'",
"'192.168.0.1 ','192.168.0.1'",
"' 192.168.0.1 ','192.168.0.1'",
"'0:0:0:0:0:0:0:1','0:0:0:0:0:0:0:1'",
"'[0:0:0:0:0:0:0:1]','0:0:0:0:0:0:0:1'",
"' [0:0:0:0:0:0:0:1] ','0:0:0:0:0:0:0:1'",
"'NULL','NULL'",
"'UNKNOWN','NULL'",
"'','NULL'",
}, nullValues = {"NULL"})
void getValidIP(final String input, final String expectedOutput) {

final String actual = DeviceUtil.getValidIP(input);
LOGGER.debug("input [{}], expectedOutput [{}], actual [{}]", input, expectedOutput, actual);
Assertions.assertThat(actual)
.isEqualTo(expectedOutput);
}

@Test
void createDeviceFromInetAddress_localhost() throws UnknownHostException {
final InetAddress inetAddress = InetAddress.getLocalHost();
final Device device = DeviceUtil.createDeviceFromInetAddress(inetAddress);
logDeviceAddressDetails(device);
Assertions.assertThat(device.getIPAddress())
.matches("^(10\\.|127\\.|0{1,3}:).*");

Assertions.assertThat(device.getHostName())
.isNotNull();
}

@Test
void createDeviceFromInetAddress_fromHost() throws UnknownHostException {
final InetAddress inetAddress = InetAddress.getByName("10.0.0.9");
final Device device = DeviceUtil.createDeviceFromInetAddress(inetAddress);
logDeviceAddressDetails(device);

Assertions.assertThat(device.getIPAddress())
.isEqualTo("10.0.0.9");
// Can't resolve a hostname for this ip so host name will be null
Assertions.assertThat(device.getHostName())
.isNull();
}

@Test
void createDeviceFromInetAddress_ipv6() throws UnknownHostException {
final InetAddress inetAddress = InetAddress.getByName("[0:0:0:0:0:0:0:1]");
final Device device = DeviceUtil.createDeviceFromInetAddress(inetAddress);
logDeviceAddressDetails(device);

Assertions.assertThat(device.getIPAddress())
.isEqualTo("0:0:0:0:0:0:0:1");
// Will be localhost's hostname
Assertions.assertThat(device.getHostName())
.isNotNull();
}

@Test
void createDeviceFromInetAddress_ipv6_2() throws UnknownHostException {
final InetAddress inetAddress = InetAddress.getByName("0:0:0:0:0:0:0:1");
final Device device = DeviceUtil.createDeviceFromInetAddress(inetAddress);
logDeviceAddressDetails(device);

Assertions.assertThat(device.getIPAddress())
.isEqualTo("0:0:0:0:0:0:0:1");
// Will be localhost's hostname
Assertions.assertThat(device.getHostName())
.isNotNull();
}

private void logDeviceAddressDetails(final Device device) {
LOGGER.debug("Device: (ip: [{}], host: [{}], mac: [{}])",
device.getIPAddress(),
device.getHostName(),
device.getMACAddress());
}

@Test
void createDevice() {
final String hostname = "host.domain";
final String ip = "192.168.0.1";
final Device device = DeviceUtil.createDevice(hostname, ip);
logDeviceAddressDetails(device);

Assertions.assertThat(device.getHostName())
.isEqualTo(hostname);
Assertions.assertThat(device.getIPAddress())
.isEqualTo(ip);
}
}
2 changes: 1 addition & 1 deletion event-logging-base/src/test/resources/logback-test.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
</encoder>
</appender>

<logger name="event.logging" level="info" />
<logger name="event.logging" level="debug" />

<root level="error">
<appender-ref ref="STDOUT" />
Expand Down
24 changes: 24 additions & 0 deletions unreleased_changes/20220429_100837_560__14.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
* Issue **#14** : Change DeviceUtil to strip square brackets from IPv6 addresses to ensure they are valid in the Schema.


```sh
# ********************************************************************************
# Issue title: Change DeviceUtil to strip `[]` from IP addresses
# Issue link: https://github.com/gchq/event-logging/issues/14
# ********************************************************************************

# ONLY the top line will be included as a change entry in the CHANGELOG.
# The entry should be in GitHub flavour markdown and should be written on a SINGLE
# line with no hard breaks. You can have multiple change files for a single GitHub issue.
# The entry should be written in the imperative mood, i.e. 'Fix nasty bug' rather than
# 'Fixed nasty bug'.
#
# Examples of acceptable entries are:
#
#
# * Issue **123** : Fix bug with an associated GitHub issue in this repository
#
# * Issue **namespace/other-repo#456** : Fix bug with an associated GitHub issue in another repository
#
# * Fix bug with no associated GitHub issue.
```

0 comments on commit ee5d95d

Please sign in to comment.