From f2c0546c6962d4a31c27471ad58d4e2c407e6ba7 Mon Sep 17 00:00:00 2001 From: "David G. Young" Date: Mon, 6 Jul 2015 07:36:43 -0400 Subject: [PATCH] Make parser more tolerant per #229 - allow missing data fields - if an AltBeacon parser fails, don't crash with NPE --- .../org/altbeacon/beacon/BeaconParser.java | 89 ++++++++++--------- .../altbeacon/beacon/AltBeaconParserTest.java | 31 ++++++- .../altbeacon/beacon/BeaconParserTest.java | 19 ++++ 3 files changed, 94 insertions(+), 45 deletions(-) diff --git a/src/main/java/org/altbeacon/beacon/BeaconParser.java b/src/main/java/org/altbeacon/beacon/BeaconParser.java index 7e22b8d39..cf50d709e 100644 --- a/src/main/java/org/altbeacon/beacon/BeaconParser.java +++ b/src/main/java/org/altbeacon/beacon/BeaconParser.java @@ -407,7 +407,7 @@ protected Beacon fromScanData(byte[] bytesToProcess, int rssi, BluetoothDevice d bytesToHex(bytesToProcess)); } } - + parseFailed = true; beacon = null; } else { if (LogManager.isVerboseLoggingEnabled()) { @@ -416,63 +416,64 @@ protected Beacon fromScanData(byte[] bytesToProcess, int rssi, BluetoothDevice d } } - for (int i = 0; i < mIdentifierEndOffsets.size(); i++) { - int endIndex = mIdentifierEndOffsets.get(i) + startByte; - if (endIndex > pduToParse.getEndIndex()) { - parseFailed = true; - if (LogManager.isVerboseLoggingEnabled()) { - LogManager.d(TAG, "Cannot parse identifier "+i+" because PDU is too short. endIndex: " + endIndex + " PDU endIndex: " + pduToParse.getEndIndex()); + if (patternFound) { + for (int i = 0; i < mIdentifierEndOffsets.size(); i++) { + int endIndex = mIdentifierEndOffsets.get(i) + startByte; + if (endIndex > pduToParse.getEndIndex()) { + parseFailed = true; + if (LogManager.isVerboseLoggingEnabled()) { + LogManager.d(TAG, "Cannot parse identifier "+i+" because PDU is too short. endIndex: " + endIndex + " PDU endIndex: " + pduToParse.getEndIndex()); + } } - } - else { - Identifier identifier = Identifier.fromBytes(bytesToProcess, mIdentifierStartOffsets.get(i) + startByte, endIndex+1, mIdentifierLittleEndianFlags.get(i)); - identifiers.add(identifier); - } - } - for (int i = 0; i < mDataEndOffsets.size(); i++) { - int endIndex = mDataEndOffsets.get(i) + startByte; - if (endIndex > pduToParse.getEndIndex()) { - parseFailed = true; - if (LogManager.isVerboseLoggingEnabled()) { - LogManager.d(TAG, "Cannot parse data field "+i+" because PDU is too short. endIndex: " + endIndex + " PDU endIndex: " + pduToParse.getEndIndex()); + else { + Identifier identifier = Identifier.fromBytes(bytesToProcess, mIdentifierStartOffsets.get(i) + startByte, endIndex+1, mIdentifierLittleEndianFlags.get(i)); + identifiers.add(identifier); } } - else { - String dataString = byteArrayToFormattedString(bytesToProcess, mDataStartOffsets.get(i) + startByte, endIndex, mDataLittleEndianFlags.get(i)); - dataFields.add(Long.parseLong(dataString)); - } - } - - if (mPowerStartOffset != null) { - int endIndex = mPowerEndOffset + startByte; - int txPower = 0; - try { + for (int i = 0; i < mDataEndOffsets.size(); i++) { + int endIndex = mDataEndOffsets.get(i) + startByte; if (endIndex > pduToParse.getEndIndex()) { - parseFailed = true; if (LogManager.isVerboseLoggingEnabled()) { - LogManager.d(TAG, "Cannot parse power field because PDU is too short. endIndex: " + endIndex + " PDU endIndex: " + pduToParse.getEndIndex()); + LogManager.d(TAG, "Cannot parse data field "+i+" because PDU is too short. endIndex: " + endIndex + " PDU endIndex: " + pduToParse.getEndIndex()+". Setting value to 0"); } + dataFields.add(new Long(0l)); } else { - String powerString = byteArrayToFormattedString(bytesToProcess, mPowerStartOffset + startByte, mPowerEndOffset + startByte, false); - txPower = Integer.parseInt(powerString)+mDBmCorrection; - // make sure it is a signed integer - if (txPower > 127) { - txPower -= 256; - } - beacon.mTxPower = txPower; + String dataString = byteArrayToFormattedString(bytesToProcess, mDataStartOffsets.get(i) + startByte, endIndex, mDataLittleEndianFlags.get(i)); + dataFields.add(Long.parseLong(dataString)); } } - catch (NumberFormatException e1) { - // keep default value - } - catch (NullPointerException e2) { - // keep default value + + if (mPowerStartOffset != null) { + int endIndex = mPowerEndOffset + startByte; + int txPower = 0; + try { + if (endIndex > pduToParse.getEndIndex()) { + parseFailed = true; + if (LogManager.isVerboseLoggingEnabled()) { + LogManager.d(TAG, "Cannot parse power field because PDU is too short. endIndex: " + endIndex + " PDU endIndex: " + pduToParse.getEndIndex()); + } + } + else { + String powerString = byteArrayToFormattedString(bytesToProcess, mPowerStartOffset + startByte, mPowerEndOffset + startByte, false); + txPower = Integer.parseInt(powerString)+mDBmCorrection; + // make sure it is a signed integer + if (txPower > 127) { + txPower -= 256; + } + beacon.mTxPower = txPower; + } + } + catch (NumberFormatException e1) { + // keep default value + } + catch (NullPointerException e2) { + // keep default value + } } } } - if (parseFailed) { beacon = null; } diff --git a/src/test/java/org/altbeacon/beacon/AltBeaconParserTest.java b/src/test/java/org/altbeacon/beacon/AltBeaconParserTest.java index c74ff0bd4..0e38fa418 100644 --- a/src/test/java/org/altbeacon/beacon/AltBeaconParserTest.java +++ b/src/test/java/org/altbeacon/beacon/AltBeaconParserTest.java @@ -2,7 +2,10 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import org.altbeacon.beacon.logging.LogManager; +import org.altbeacon.beacon.logging.Loggers; import org.robolectric.RobolectricTestRunner; import org.junit.runner.RunWith; @@ -31,7 +34,6 @@ public static byte[] hexStringToByteArray(String s) { } return data; } - @Test public void testRecognizeBeacon() { BeaconManager.setDebug(true); @@ -59,4 +61,31 @@ public void testDetectsAlternateBeconType() { Beacon beacon = parser.fromScanData(bytes, -55, null); assertNotNull("Beacon should be not null if parsed successfully", beacon); } + @Test + public void testParseWrongFormatReturnsNothing() { + BeaconManager.setDebug(true); + org.robolectric.shadows.ShadowLog.stream = System.err; + LogManager.d("XXX", "testParseWrongFormatReturnsNothing start"); + byte[] bytes = hexStringToByteArray("02011a1aff1801ffff2f234454cf6d4a0fadf2f4911ba9ffa600010002c509"); + AltBeaconParser parser = new AltBeaconParser(); + Beacon beacon = parser.fromScanData(bytes, -55, null); + LogManager.d("XXX", "testParseWrongFormatReturnsNothing end"); + assertNull("Beacon should be null if not parsed successfully", beacon); + } + + @Test + public void testParsesBeaconMissingDataField() { + BeaconManager.setDebug(true); + org.robolectric.shadows.ShadowLog.stream = System.err; + byte[] bytes = hexStringToByteArray("02011a1aff1801beac2f234454cf6d4a0fadf2f4911ba9ffa600010002c5"); + AltBeaconParser parser = new AltBeaconParser(); + Beacon beacon = parser.fromScanData(bytes, -55, null); + assertEquals("mRssi should be as passed in", -55, beacon.getRssi()); + assertEquals("uuid should be parsed", "2f234454-cf6d-4a0f-adf2-f4911ba9ffa6", beacon.getIdentifier(0).toString()); + assertEquals("id2 should be parsed", "1", beacon.getIdentifier(1).toString()); + assertEquals("id3 should be parsed", "2", beacon.getIdentifier(2).toString()); + assertEquals("txPower should be parsed", -59, beacon.getTxPower()); + assertEquals("manufacturer should be parsed", 0x118 ,beacon.getManufacturer()); + assertEquals("missing data field zero should be zero", new Long(0l), beacon.getDataFields().get(0)); + } } \ No newline at end of file diff --git a/src/test/java/org/altbeacon/beacon/BeaconParserTest.java b/src/test/java/org/altbeacon/beacon/BeaconParserTest.java index c2d3f8847..598157b82 100644 --- a/src/test/java/org/altbeacon/beacon/BeaconParserTest.java +++ b/src/test/java/org/altbeacon/beacon/BeaconParserTest.java @@ -98,6 +98,25 @@ public void testRecognizeBeacon() { assertEquals("manufacturer should be parsed", 0x118 ,beacon.getManufacturer()); } + @Test + public void testParsesBeaconMissingDataField() { + LogManager.setLogger(Loggers.verboseLogger()); + org.robolectric.shadows.ShadowLog.stream = System.err; + byte[] bytes = hexStringToByteArray("02011a1aff1801beac2f234454cf6d4a0fadf2f4911ba9ffa600010002c5"); + BeaconParser parser = new BeaconParser(); + parser.setBeaconLayout("m:2-3=beac,i:4-19,i:20-21,i:22-23,p:24-24,d:25-25"); + Beacon beacon = parser.fromScanData(bytes, -55, null); + assertEquals("mRssi should be as passed in", -55, beacon.getRssi()); + assertEquals("uuid should be parsed", "2f234454-cf6d-4a0f-adf2-f4911ba9ffa6", beacon.getIdentifier(0).toString()); + assertEquals("id2 should be parsed", "1", beacon.getIdentifier(1).toString()); + assertEquals("id3 should be parsed", "2", beacon.getIdentifier(2).toString()); + assertEquals("txPower should be parsed", -59, beacon.getTxPower()); + assertEquals("manufacturer should be parsed", 0x118 ,beacon.getManufacturer()); + assertEquals("missing data field zero should be zero", new Long(0l), beacon.getDataFields().get(0)); + + } + + @Test public void testRecognizeBeaconWithFormatSpecifyingManufacturer() { LogManager.setLogger(Loggers.verboseLogger());