This is an automated email from the ASF dual-hosted git repository. cdutz pushed a commit to branch develop in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git
The following commit(s) were added to refs/heads/develop by this push: new 980c069 S7: changed byteLength and blockNumber from short to int new 7031908 Merge pull request #44 from timbo2k/too_short_range_for_offset 980c069 is described below commit 980c0695e3500df60b2fbe159aef6821de0d5e32 Author: Tim Mitsch <tim.mit...@tmbeng.com> AuthorDate: Wed Jan 16 21:05:24 2019 +0100 S7: changed byteLength and blockNumber from short to int --- .../org/apache/plc4x/java/s7/model/S7Field.java | 54 ++++++++++++++++++---- .../model/params/items/S7AnyVarParameterItem.java | 10 ++-- .../strategies/DefaultS7MessageProcessor.java | 12 ++--- .../java/org/apache/plc4x/java/issues/PLC4X56.java | 44 +++++++++++++++++- .../src/test/resources/example_with_strings.yml | 14 +++--- 5 files changed, 104 insertions(+), 30 deletions(-) diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/model/S7Field.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/model/S7Field.java index a460d6a..8f7e555 100644 --- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/model/S7Field.java +++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/model/S7Field.java @@ -28,10 +28,13 @@ import java.util.regex.Pattern; public class S7Field implements PlcField { + //byteOffset theoretically can reach up to 2097151 ... see checkByteOffset() below --> 7digits private static final Pattern ADDRESS_PATTERN = - Pattern.compile("^%(?<memoryArea>.)(?<transferSizeCode>[XBWD]?)(?<byteOffset>\\d{1,5})(.(?<bitOffset>[0-7]))?:(?<dataType>[a-zA-Z_]+)(\\[(?<numElements>\\d+)])?"); + Pattern.compile("^%(?<memoryArea>.)(?<transferSizeCode>[XBWD]?)(?<byteOffset>\\d{1,7})(.(?<bitOffset>[0-7]))?:(?<dataType>[a-zA-Z_]+)(\\[(?<numElements>\\d+)])?"); + + //blockNumber usually has its max hat around 64000 --> 5digits private static final Pattern DATA_BLOCK_ADDRESS_PATTERN = - Pattern.compile("^%DB(?<blockNumber>\\d{1,4}).DB(?<transferSizeCode>[XBWD]?)(?<byteOffset>\\d{1,5})(.(?<bitOffset>[0-7]))?:(?<dataType>[a-zA-Z_]+)(\\[(?<numElements>\\d+)])?"); + Pattern.compile("^%DB(?<blockNumber>\\d{1,5}).DB(?<transferSizeCode>[XBWD]?)(?<byteOffset>\\d{1,7})(.(?<bitOffset>[0-7]))?:(?<dataType>[a-zA-Z_]+)(\\[(?<numElements>\\d+)])?"); private static final String DATA_TYPE = "dataType"; private static final String TRANSFER_SIZE_CODE = "transferSizeCode"; @@ -43,12 +46,12 @@ public class S7Field implements PlcField { private final TransportSize dataType; private final MemoryArea memoryArea; - private final short blockNumber; - private final short byteOffset; + private final int blockNumber; + private final int byteOffset; private final short bitOffset; private final int numElements; - private S7Field(TransportSize dataType, MemoryArea memoryArea, short blockNumber, short byteOffset, short bitOffset, int numElements) { + private S7Field(TransportSize dataType, MemoryArea memoryArea, int blockNumber, int byteOffset, short bitOffset, int numElements) { this.dataType = dataType; this.memoryArea = memoryArea; this.blockNumber = blockNumber; @@ -65,11 +68,11 @@ public class S7Field implements PlcField { return memoryArea; } - public short getBlockNumber() { + public int getBlockNumber() { return blockNumber; } - public short getByteOffset() { + public int getByteOffset() { return byteOffset; } @@ -92,8 +95,11 @@ public class S7Field implements PlcField { TransportSize dataType = TransportSize.valueOf(matcher.group(DATA_TYPE)); MemoryArea memoryArea = MemoryArea.DATA_BLOCKS; String transferSizeCode = matcher.group(TRANSFER_SIZE_CODE); - short blockNumber = Short.parseShort(matcher.group(BLOCK_NUMBER)); - short byteOffset = Short.parseShort(matcher.group(BYTE_OFFSET)); + + int blockNumber = checkDatablockNumber(Integer.parseInt(matcher.group(BLOCK_NUMBER))); + + int byteOffset = checkByteOffset(Integer.parseInt(matcher.group(BYTE_OFFSET))); + short bitOffset = 0; if(matcher.group(BIT_OFFSET) != null) { bitOffset = Short.parseShort(matcher.group(BIT_OFFSET)); @@ -116,7 +122,9 @@ public class S7Field implements PlcField { TransportSize dataType = TransportSize.valueOf(matcher.group(DATA_TYPE)); MemoryArea memoryArea = MemoryArea.valueOfShortName(matcher.group(MEMORY_AREA)); String transferSizeCode = matcher.group(TRANSFER_SIZE_CODE); - short byteOffset = Short.parseShort(matcher.group(BYTE_OFFSET)); + + int byteOffset = checkByteOffset(Integer.parseInt(matcher.group(BYTE_OFFSET))); + short bitOffset = 0; if(matcher.group(BIT_OFFSET) != null) { bitOffset = Short.parseShort(matcher.group(BIT_OFFSET)); @@ -139,6 +147,32 @@ public class S7Field implements PlcField { } /** + * checks if DatablockNumber of S7Field is in valid range + * @param blockNumber given DatablockNumber + * @return given blockNumber if Ok, throws PlcInvalidFieldException otherwise + */ + private static int checkDatablockNumber(int blockNumber){ + //ToDo check the value or add reference - limit eventually depending on active S7 --> make a case selection + if(blockNumber>64000 || blockNumber<1){ + throw new PlcInvalidFieldException("Datablock numbers larger than 64000 or smaller than 1 are not supported."); + } + return blockNumber; + } + + /** + * checks if ByteOffset from S7Field is in valid range + * @param byteOffset given byteOffset + * @return given byteOffset if Ok, throws PlcInvalidFieldException otherwise + */ + private static int checkByteOffset(int byteOffset){ + //ToDo check the value or add reference + if(byteOffset>2097151 || byteOffset<0){ + throw new PlcInvalidFieldException("ByteOffset must be smaller than 2097151 and positive."); + } + return byteOffset; + } + + /** * correct the storage of "array"-like variables like STRING * @param numElements auto-detected numElements (1 if no numElements in brackets has been given, x if a specific number has been given) * @param dataType detected Transport-Size that represents the data-type diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/items/S7AnyVarParameterItem.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/items/S7AnyVarParameterItem.java index b1849f8..e9bc87c 100644 --- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/items/S7AnyVarParameterItem.java +++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/model/params/items/S7AnyVarParameterItem.java @@ -44,11 +44,11 @@ public class S7AnyVarParameterItem implements VarParameterItem { private final MemoryArea memoryArea; private final TransportSize dataType; private final int numElements; - private final short dataBlockNumber; - private final short byteOffset; + private final int dataBlockNumber; + private final int byteOffset; private final byte bitOffset; - public S7AnyVarParameterItem(SpecificationType specificationType, MemoryArea memoryArea, TransportSize dataType, int numElements, short dataBlockNumber, short byteOffset, byte bitOffset) { + public S7AnyVarParameterItem(SpecificationType specificationType, MemoryArea memoryArea, TransportSize dataType, int numElements, int dataBlockNumber, int byteOffset, byte bitOffset) { this.specificationType = specificationType; this.memoryArea = memoryArea; this.dataType = dataType; @@ -79,11 +79,11 @@ public class S7AnyVarParameterItem implements VarParameterItem { return numElements; } - public short getDataBlockNumber() { + public int getDataBlockNumber() { return dataBlockNumber; } - public short getByteOffset() { + public int getByteOffset() { return byteOffset; } diff --git a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/strategies/DefaultS7MessageProcessor.java b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/strategies/DefaultS7MessageProcessor.java index 439199a..41db08a 100644 --- a/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/strategies/DefaultS7MessageProcessor.java +++ b/plc4j/protocols/s7/src/main/java/org/apache/plc4x/java/s7/netty/strategies/DefaultS7MessageProcessor.java @@ -141,7 +141,7 @@ public class DefaultS7MessageProcessor implements S7MessageProcessor { (double) maxResponseSize / (double) s7AnyVarParameterItem.getDataType().getSizeInBytes()); int sizeMaxNumElementInBytes = maxNumElements * s7AnyVarParameterItem.getDataType().getSizeInBytes(); int remainingNumElements = s7AnyVarParameterItem.getNumElements(); - short curByteOffset = s7AnyVarParameterItem.getByteOffset(); + int curByteOffset = s7AnyVarParameterItem.getByteOffset(); while(remainingNumElements > 0) { int numCurElements = Math.min(remainingNumElements, maxNumElements); @@ -218,7 +218,7 @@ public class DefaultS7MessageProcessor implements S7MessageProcessor { if (varParameterItem instanceof S7AnyVarParameterItem) { S7AnyVarParameterItem s7AnyVarParameterItem = (S7AnyVarParameterItem) varParameterItem; - short byteOffset = s7AnyVarParameterItem.getByteOffset(); + int byteOffset = s7AnyVarParameterItem.getByteOffset(); if (s7AnyVarParameterItem.getDataType() == TransportSize.BOOL) { processBooleanWriteVarParameter(request, varParameter, varPayload, s7AnyVarParameterItem, varPayloadItem, byteOffset, compositeRequestMessage); @@ -235,8 +235,8 @@ public class DefaultS7MessageProcessor implements S7MessageProcessor { private void processBooleanWriteVarParameter(S7RequestMessage request, VarParameter varParameter, VarPayload varPayload, S7AnyVarParameterItem s7AnyVarParameterItem, VarPayloadItem varPayloadItem, - short byteOffset, S7CompositeRequestMessage compositeRequestMessage) { - short curParameterByteOffset = byteOffset; + int byteOffset, S7CompositeRequestMessage compositeRequestMessage) { + int curParameterByteOffset = byteOffset; byte curParameterBitOffset = s7AnyVarParameterItem.getBitOffset(); byte curPayloadBitOffset = 0; for (int i = 0; i < s7AnyVarParameterItem.getNumElements(); i++) { @@ -288,8 +288,8 @@ public class DefaultS7MessageProcessor implements S7MessageProcessor { private void processNonBooleanWriteVarParameter(S7RequestMessage request, VarParameter varParameter, VarPayload varPayload, S7AnyVarParameterItem s7AnyVarParameterItem, VarPayloadItem varPayloadItem, - short byteOffset, S7CompositeRequestMessage compositeRequestMessage) { - short curByteOffset = byteOffset; + int byteOffset, S7CompositeRequestMessage compositeRequestMessage) { + int curByteOffset = byteOffset; int payloadPosition = 0; for (int i = 0; i < s7AnyVarParameterItem.getNumElements(); i++) { int elementSize = s7AnyVarParameterItem.getDataType().getSizeInBytes(); diff --git a/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/issues/PLC4X56.java b/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/issues/PLC4X56.java index d5de122..15d7a42 100644 --- a/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/issues/PLC4X56.java +++ b/plc4j/protocols/s7/src/test/java/org/apache/plc4x/java/issues/PLC4X56.java @@ -19,10 +19,14 @@ package org.apache.plc4x.java.issues; +import org.apache.plc4x.java.api.exceptions.PlcInvalidFieldException; import org.apache.plc4x.java.s7.model.S7Field; import org.apache.plc4x.java.s7.netty.model.types.MemoryArea; import org.apache.plc4x.java.s7.netty.model.types.TransportSize; +import org.junit.Assert; +import org.junit.Rule; import org.junit.jupiter.api.Test; +import org.junit.rules.ExpectedException; import static org.hamcrest.core.IsEqual.equalTo; import static org.junit.Assert.assertThat; @@ -33,11 +37,47 @@ public class PLC4X56 { void name() { S7Field field = S7Field.of("%DB56.DBB100:SINT[25]"); assertThat(field.getMemoryArea(), equalTo(MemoryArea.DATA_BLOCKS)); - assertThat(field.getBlockNumber(), equalTo((short) 56)); - assertThat(field.getByteOffset(), equalTo((short) 100)); + assertThat(field.getBlockNumber(), equalTo(56)); + assertThat(field.getByteOffset(), equalTo(100)); assertThat(field.getBitOffset(), equalTo((short) 0)); assertThat(field.getDataType(), equalTo(TransportSize.SINT)); assertThat(field.getNumElements(), equalTo(25)); } + @Test + public void invalidBlockLengthThrowsException() throws Exception { + try { + S7Field field = S7Field.of("%DB2000.DBB8000000:SINT[25]"); + Assert.fail(); + } + catch (PlcInvalidFieldException e){ + e.printStackTrace(); + // exception thrown --> Test Ok + } + } + + @Test + public void invalidBlockNumber1ThrowsException() throws Exception { + try { + S7Field field = S7Field.of("%DB0.DBB800:SINT[25]"); + Assert.fail(); + } + catch (PlcInvalidFieldException e){ + e.printStackTrace(); + // exception thrown --> Test Ok + } + } + + @Test + public void invalidBlockNumber2ThrowsException() throws Exception { + try { + S7Field field = S7Field.of("%DB80000.DBB800:SINT[25]"); + Assert.fail(); + } + catch (PlcInvalidFieldException e){ + e.printStackTrace(); + // exception thrown --> Test Ok + } + } + } diff --git a/plc4j/utils/scraper/src/test/resources/example_with_strings.yml b/plc4j/utils/scraper/src/test/resources/example_with_strings.yml index 37309f0..dce108a 100644 --- a/plc4j/utils/scraper/src/test/resources/example_with_strings.yml +++ b/plc4j/utils/scraper/src/test/resources/example_with_strings.yml @@ -26,13 +26,13 @@ jobs: sources: - S7_TIM fields: - test1: '%DB810:DBW0:UINT' - test2: '%DB810:DBX4:STRING' - test3: '%DB810:DBX264:STRING' - test4: '%DB810:DBX524:STRING' - test5: '%DB810:DBX784:STRING' - test6: '%DB810:DBX1044:STRING' - test7: '%DB810:DBD0:REAL' + test1: '%DB810:DBW0:INT' + test2: '%DB810:DBX6:STRING' + test3: '%DB810:DBX266:STRING' + test4: '%DB810:DBX526:STRING' + test5: '%DB810:DBX786:STRING' + test6: '%DB810:DBX46806:STRING' + test7: '%DB810:DBD2:REAL' test8: '%DB811:DBX12:STRING' test9: '%DB811:DBX280:STRING' test10: '%DB811:DBB1000:BYTE[8]'