Skip to content

Commit 137dbba

Browse files
HilbrandHilbrand
committed
[dsmr] Improved error handling to better handle corrupt messages.
Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl>
1 parent a8f6719 commit 137dbba

24 files changed

+326
-215
lines changed

‎bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/DSMRDeviceRunnable.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313
package org.openhab.binding.dsmr.internal.device;
1414

15+
import java.util.Optional;
1516
import java.util.concurrent.Semaphore;
1617

1718
import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -45,7 +46,7 @@ public class DSMRDeviceRunnable implements Runnable {
4546
* @param device the device to control
4647
* @param eventListener listener to used ot report errors.
4748
*/
48-
public DSMRDeviceRunnable(DSMRDevice device, DSMREventListener eventListener) {
49+
public DSMRDeviceRunnable(final DSMRDevice device, final DSMREventListener eventListener) {
4950
this.device = device;
5051
this.portEventListener = eventListener;
5152
}
@@ -83,10 +84,11 @@ public void run() {
8384
}
8485
}
8586
logger.trace("Device shutdown");
86-
} catch (RuntimeException e) {
87+
} catch (final RuntimeException e) {
8788
logger.warn("DSMRDeviceRunnable stopped with a RuntimeException", e);
88-
portEventListener.handleErrorEvent(DSMRConnectorErrorEvent.READ_ERROR);
89-
} catch (InterruptedException e) {
89+
portEventListener.handleErrorEvent(DSMRConnectorErrorEvent.READ_ERROR,
90+
Optional.ofNullable(e.getMessage()).orElse(""));
91+
} catch (final InterruptedException e) {
9092
Thread.currentThread().interrupt();
9193
} finally {
9294
device.stop();

‎bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/DSMREventListener.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,9 @@ public interface DSMREventListener {
2828
* Callback for DSMRPortEvent events
2929
*
3030
* @param connectorErrorEvent {@link DSMRConnectorErrorEvent} that has occurred
31+
* @param message Additional error message
3132
*/
32-
public void handleErrorEvent(DSMRConnectorErrorEvent connectorErrorEvent);
33+
public void handleErrorEvent(DSMRConnectorErrorEvent connectorErrorEvent, String message);
3334

3435
/**
3536
* Callback for received P1 telegrams

‎bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/DSMRSerialAutoDevice.java‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ enum DeviceState {
113113
/**
114114
* The listener of the class handling the connector events
115115
*/
116-
private DSMREventListener parentListener;
116+
private final DSMREventListener parentListener;
117117

118118
/**
119119
* Time in nanos the last time the baudrate was switched. This is used during discovery ignore errors retrieved
@@ -132,9 +132,9 @@ enum DeviceState {
132132
* @param baudrateSwitchTimeoutSeconds timeout period for when to try other baudrate settings and end the discovery
133133
* of the baudrate
134134
*/
135-
public DSMRSerialAutoDevice(SerialPortManager serialPortManager, String serialPortName, DSMREventListener listener,
136-
DSMRTelegramListener telegramListener, ScheduledExecutorService scheduler,
137-
int baudrateSwitchTimeoutSeconds) {
135+
public DSMRSerialAutoDevice(final SerialPortManager serialPortManager, final String serialPortName,
136+
final DSMREventListener listener, final DSMRTelegramListener telegramListener,
137+
final ScheduledExecutorService scheduler, final int baudrateSwitchTimeoutSeconds) {
138138
this.parentListener = listener;
139139
this.scheduler = scheduler;
140140
this.baudrateSwitchTimeoutSeconds = baudrateSwitchTimeoutSeconds;
@@ -178,13 +178,11 @@ public synchronized void stop() {
178178
* @param telegram the details of the received telegram
179179
*/
180180
@Override
181-
public void handleTelegramReceived(P1Telegram telegram) {
182-
if (!telegram.getCosemObjects().isEmpty()) {
183-
stopDiscover(DeviceState.NORMAL);
184-
parentListener.handleTelegramReceived(telegram);
185-
logger.info("Start receiving telegrams on port {} with settings: {}", dsmrConnector.getPortName(),
186-
portSettings);
187-
}
181+
public void handleTelegramReceived(final P1Telegram telegram) {
182+
stopDiscover(DeviceState.NORMAL);
183+
parentListener.handleTelegramReceived(telegram);
184+
logger.info("Start receiving telegrams on port {} with settings: {}", dsmrConnector.getPortName(),
185+
portSettings);
188186
}
189187

190188
/**
@@ -193,23 +191,23 @@ public void handleTelegramReceived(P1Telegram telegram) {
193191
* @param portEvent {@link DSMRConnectorErrorEvent} to handle
194192
*/
195193
@Override
196-
public void handleErrorEvent(DSMRConnectorErrorEvent portEvent) {
194+
public void handleErrorEvent(final DSMRConnectorErrorEvent portEvent, final String message) {
197195
logger.trace("Received portEvent {}", portEvent.getEventDetails());
198196
if (portEvent == DSMRConnectorErrorEvent.READ_ERROR) {
199197
switchBaudrate();
200198
} else {
201199
logger.debug("Error during discovery of port settings: {}, current state:{}.", portEvent.getEventDetails(),
202200
state);
203201
stopDiscover(DeviceState.ERROR);
204-
parentListener.handleErrorEvent(portEvent);
202+
parentListener.handleErrorEvent(portEvent, message);
205203
}
206204
}
207205

208206
/**
209207
* @param lenientMode the lenientMode to set
210208
*/
211209
@Override
212-
public void setLenientMode(boolean lenientMode) {
210+
public void setLenientMode(final boolean lenientMode) {
213211
telegramListener.setLenientMode(lenientMode);
214212
}
215213

@@ -248,7 +246,7 @@ private void switchBaudrate() {
248246
private void endTimeScheduledCall() {
249247
if (state == DeviceState.DISCOVER_SETTINGS) {
250248
stopDiscover(DeviceState.ERROR);
251-
parentListener.handleErrorEvent(DSMRConnectorErrorEvent.DONT_EXISTS);
249+
parentListener.handleErrorEvent(DSMRConnectorErrorEvent.DONT_EXISTS, "");
252250
}
253251
}
254252

@@ -257,7 +255,8 @@ private void endTimeScheduledCall() {
257255
*
258256
* @param state the state with which the process was stopped.
259257
*/
260-
private void stopDiscover(DeviceState state) {
258+
private void stopDiscover(final DeviceState state) {
259+
this.state = state;
261260
telegramListener.setDsmrEventListener(parentListener);
262261
logger.debug("Stop discovery of port settings.");
263262
if (halfTimeTimer != null) {
@@ -268,7 +267,6 @@ private void stopDiscover(DeviceState state) {
268267
endTimeTimer.cancel(true);
269268
endTimeTimer = null;
270269
}
271-
this.state = state;
272270
}
273271

274272
/**

‎bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/DSMRTelegramListener.java‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
package org.openhab.binding.dsmr.internal.device;
1414

1515
import java.util.List;
16-
import java.util.stream.Collectors;
1716

1817
import org.eclipse.jdt.annotation.NonNullByDefault;
1918
import org.openhab.binding.dsmr.internal.device.connector.DSMRConnectorErrorEvent;
@@ -76,8 +75,8 @@ public void handleData(final byte[] data, final int length) {
7675
}
7776

7877
@Override
79-
public void handleErrorEvent(final DSMRConnectorErrorEvent portEvent) {
80-
dsmrEventListener.handleErrorEvent(portEvent);
78+
public void handleErrorEvent(final DSMRConnectorErrorEvent portEvent, final String message) {
79+
dsmrEventListener.handleErrorEvent(portEvent, message);
8180
parser.reset();
8281
}
8382

@@ -94,14 +93,12 @@ public void telegramReceived(final P1Telegram telegram) {
9493
if (logger.isTraceEnabled()) {
9594
logger.trace("Received {} Cosem Objects with state: '{}'", cosemObjects.size(), telegramState);
9695
}
97-
if (telegramState == TelegramState.OK || telegramState == TelegramState.INVALID_ENCRYPTION_KEY) {
98-
dsmrEventListener.handleTelegramReceived(telegram);
99-
} else {
100-
if (logger.isDebugEnabled()) {
101-
logger.debug("Telegram received with error state '{}': {}", telegramState,
102-
cosemObjects.stream().map(CosemObject::toString).collect(Collectors.joining(",")));
103-
}
104-
}
96+
dsmrEventListener.handleTelegramReceived(telegram);
97+
}
98+
99+
@Override
100+
public void onTelegramError(final TelegramState state, final String message) {
101+
dsmrEventListener.handleErrorEvent(DSMRConnectorErrorEvent.PARSE_ERROR, message);
105102
}
106103

107104
/**

‎bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/SmartyDecrypter.java‎

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
import java.security.InvalidAlgorithmParameterException;
1717
import java.security.InvalidKeyException;
1818
import java.security.NoSuchAlgorithmException;
19-
import java.util.Collections;
19+
import java.util.Arrays;
20+
import java.util.Optional;
2021

2122
import javax.crypto.BadPaddingException;
2223
import javax.crypto.Cipher;
@@ -28,7 +29,6 @@
2829
import org.eclipse.jdt.annotation.NonNullByDefault;
2930
import org.eclipse.jdt.annotation.Nullable;
3031
import org.openhab.binding.dsmr.internal.DSMRBindingConstants;
31-
import org.openhab.binding.dsmr.internal.device.p1telegram.P1Telegram;
3232
import org.openhab.binding.dsmr.internal.device.p1telegram.P1Telegram.TelegramState;
3333
import org.openhab.binding.dsmr.internal.device.p1telegram.P1TelegramListener;
3434
import org.openhab.binding.dsmr.internal.device.p1telegram.TelegramParser;
@@ -53,8 +53,7 @@ private enum State {
5353
READ_SEPARATOR_30,
5454
READ_FRAME_COUNTER,
5555
READ_PAYLOAD,
56-
READ_GCM_TAG,
57-
DONE_READING_TELEGRAM
56+
READ_GCM_TAG
5857
}
5958

6059
private static final byte START_BYTE = (byte) 0xDB;
@@ -113,6 +112,11 @@ public void parse(final byte[] data, final int length) {
113112
}
114113

115114
private boolean processStateActions(final byte rawInput) {
115+
// Safeguard against buffer overrun in case corrupt data is received.
116+
if (ivLength == IV_BUFFER_LENGTH) {
117+
reset();
118+
return false;
119+
}
116120
switch (state) {
117121
case WAITING_FOR_START_BYTE:
118122
if (rawInput == START_BYTE) {
@@ -179,26 +183,23 @@ private boolean processStateActions(final byte rawInput) {
179183
// All input has been read.
180184
cipherText.put(rawInput);
181185
if (currentBytePosition >= changeToNextStateAt) {
182-
state = State.DONE_READING_TELEGRAM;
186+
state = State.WAITING_FOR_START_BYTE;
187+
return true;
183188
}
184189
break;
185190
}
186-
if (state == State.DONE_READING_TELEGRAM) {
187-
state = State.WAITING_FOR_START_BYTE;
188-
return true;
189-
}
190191
return false;
191192
}
192193

193194
private void processCompleted() {
194-
final byte[] plainText = decrypt();
195-
196-
reset();
197-
if (plainText == null) {
198-
telegramListener
199-
.telegramReceived(new P1Telegram(Collections.emptyList(), TelegramState.INVALID_ENCRYPTION_KEY));
200-
} else {
201-
parser.parse(plainText, plainText.length);
195+
try {
196+
final byte[] plainText = decrypt();
197+
198+
if (plainText != null) {
199+
parser.parse(plainText, plainText.length);
200+
}
201+
} finally {
202+
reset();
202203
}
203204
}
204205

@@ -218,7 +219,15 @@ private void processCompleted() {
218219
}
219220
} catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException
220221
| InvalidAlgorithmParameterException | IllegalBlockSizeException | BadPaddingException e) {
221-
logger.warn("Decrypting smarty telegram failed: ", e);
222+
if (lenientMode || logger.isDebugEnabled()) {
223+
// log in lenient mode or when debug is enabled. But log to warn to also work when lenientMode is
224+
// enabled.
225+
logger.warn("Failed encrypted telegram: {}",
226+
HexUtils.bytesToHex(Arrays.copyOf(cipherText.array(), cipherText.position())));
227+
logger.warn("Exception of failed decryption of telegram: ", e);
228+
}
229+
telegramListener.onTelegramError(TelegramState.INVALID_ENCRYPTION_KEY,
230+
Optional.ofNullable(e.getMessage()).orElse(""));
222231
}
223232
return null;
224233
}

‎bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/connector/DSMRBaseConnector.java‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.io.BufferedInputStream;
1616
import java.io.IOException;
1717
import java.io.InputStream;
18+
import java.util.Optional;
1819

1920
import org.eclipse.jdt.annotation.NonNullByDefault;
2021
import org.eclipse.jdt.annotation.Nullable;
@@ -53,7 +54,7 @@ class DSMRBaseConnector {
5354
*/
5455
private boolean open;
5556

56-
public DSMRBaseConnector(DSMRConnectorListener connectorListener) {
57+
public DSMRBaseConnector(final DSMRConnectorListener connectorListener) {
5758
this.dsmrConnectorListener = connectorListener;
5859
}
5960

@@ -68,7 +69,7 @@ public DSMRBaseConnector(DSMRConnectorListener connectorListener) {
6869
* @param inputStream input stream to read data from
6970
* @throws IOException throws exception in case input stream is null
7071
*/
71-
protected void open(@Nullable InputStream inputStream) throws IOException {
72+
protected void open(@Nullable final InputStream inputStream) throws IOException {
7273
if (inputStream == null) {
7374
throw new IOException("Inputstream is null");
7475
}
@@ -91,7 +92,7 @@ protected void close() {
9192
if (inputStream != null) {
9293
try {
9394
inputStream.close();
94-
} catch (IOException ioe) {
95+
} catch (final IOException ioe) {
9596
logger.debug("Failed to close reader", ioe);
9697
}
9798
}
@@ -104,12 +105,12 @@ protected void close() {
104105
protected void handleDataAvailable() {
105106
try {
106107
synchronized (readLock) {
107-
BufferedInputStream localInputStream = inputStream;
108+
final BufferedInputStream localInputStream = inputStream;
108109

109110
if (localInputStream != null) {
110111
int bytesAvailable = localInputStream.available();
111112
while (bytesAvailable > 0) {
112-
int bytesAvailableRead = localInputStream.read(buffer, 0,
113+
final int bytesAvailableRead = localInputStream.read(buffer, 0,
113114
Math.min(bytesAvailable, buffer.length));
114115

115116
if (open && bytesAvailableRead > 0) {
@@ -122,8 +123,9 @@ protected void handleDataAvailable() {
122123
}
123124
}
124125
}
125-
} catch (IOException e) {
126-
dsmrConnectorListener.handleErrorEvent(DSMRConnectorErrorEvent.READ_ERROR);
126+
} catch (final IOException e) {
127+
dsmrConnectorListener.handleErrorEvent(DSMRConnectorErrorEvent.READ_ERROR,
128+
Optional.ofNullable(e.getMessage()).orElse(""));
127129
logger.debug("Exception on read data", e);
128130
}
129131
}

‎bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/connector/DSMRConnectorErrorEvent.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
*/
1313
package org.openhab.binding.dsmr.internal.device.connector;
1414

15+
import java.util.Locale;
16+
1517
import org.eclipse.jdt.annotation.NonNullByDefault;
1618

1719
/**
@@ -26,12 +28,13 @@ public enum DSMRConnectorErrorEvent {
2628
IN_USE,
2729
INTERNAL_ERROR,
2830
NOT_COMPATIBLE,
31+
PARSE_ERROR,
2932
READ_ERROR;
3033

3134
/**
3235
* @return the event details
3336
*/
3437
public String getEventDetails() {
35-
return "@text/error.connector." + name().toLowerCase();
38+
return "@text/binding.dsmr.error.connector." + name().toLowerCase(Locale.ROOT);
3639
}
3740
}

‎bundles/org.openhab.binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/connector/DSMRConnectorListener.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,9 @@ public interface DSMRConnectorListener {
2626
* Callback for {@link DSMRConnectorErrorEvent} events.
2727
*
2828
* @param portEvent {@link DSMRConnectorErrorEvent} that has occurred
29+
* @param message Additional error message
2930
*/
30-
public void handleErrorEvent(DSMRConnectorErrorEvent portEvent);
31+
void handleErrorEvent(DSMRConnectorErrorEvent portEvent, String message);
3132

3233
/**
3334
* Handle data.

0 commit comments

Comments
 (0)