From e6f3487656be90391037ee09cccd9141bb82a422 Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Wed, 7 Oct 2026 11:54:08 +0200 Subject: [PATCH 1/4] Don't abort the SDR repository walk on OEM or undecodable records Fixes #76. populateSensorRecord threw IllegalArgumentException for any SDR record type it does not model, and the runners only advanced nextRecId after a successful decode, so a single unknown record killed both FRU and sensor collection. IPMI 2.0 reserves C0h-FFh for OEM records and vendors use values above C0h (19 of 85 records are D0h on a GIGABYTE ME62-GE0-00). - SensorRecord: map every record type >= C0h to OemRecord. - OemRecord: tolerate records shorter than the C0h layout. - AbstractIpmiRunner: advance nextRecId before decoding, log and skip a record that fails to decode, and bound the header copy in the chunked path by the record size. Verified live: chassis, 2 FRUs and 62 sensors now collected from the GIGABYTE BMC that previously failed in under 200 ms; Lenovo IMM unchanged. Co-Authored-By: Claude Fable 5.1 --- .../client/runner/AbstractIpmiRunner.java | 44 ++++++++++++------- .../coding/commands/sdr/record/OemRecord.java | 12 +++-- .../commands/sdr/record/SensorRecord.java | 7 +++ 3 files changed, 45 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunner.java b/src/main/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunner.java index 66b084b..7bd6cba 100644 --- a/src/main/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunner.java +++ b/src/main/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunner.java @@ -40,14 +40,18 @@ import org.metricshub.ipmi.core.coding.security.CipherSuite; import org.metricshub.ipmi.core.common.TypeConverter; import org.metricshub.ipmi.core.connection.Connection; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * This abstract class implements common features required by FRUs, Sensor and Chassis Status runners. - * - * @param Represent the data type managed by the runner + * + * @param Represent the data type managed by the runner */ public abstract class AbstractIpmiRunner implements AutoCloseable, Callable { + private static final Logger LOGGER = LoggerFactory.getLogger(AbstractIpmiRunner.class); + private static final int DEFAULT_LOCAL_UDP_PORT = 0; /** @@ -192,13 +196,11 @@ protected SensorRecord getSensorData(int reservationId) throws Exception { GetSdrResponseData data = (GetSdrResponseData) connector.sendMessage(handle, new GetSdr(IpmiVersion.V20, handle.getCipherSuite(), AuthenticationType.RMCPPlus, reservationId, nextRecId)); - // If getting whole record succeeded we create SensorRecord from - // received data... - SensorRecord sensorDataToPopulate = SensorRecord.populateSensorRecord(data.getSensorRecordData()); - - // ... and update the ID of the next record + // Advance to the next record first, so that a record we cannot + // decode never stalls the whole repository walk nextRecId = data.getNextRecordId(); - return sensorDataToPopulate; + + return decodeRecord(data.getSensorRecordData()); } catch (IPMIException e) { @@ -236,7 +238,7 @@ protected SensorRecord getSensorViaChunks(int reservationId) throws Exception { byte[] bytes = new byte[recSize]; - System.arraycopy(data.getSensorRecordData(), 0, bytes, 0, data.getSensorRecordData().length); + System.arraycopy(data.getSensorRecordData(), 0, bytes, 0, Math.min(recSize, data.getSensorRecordData().length)); // We get the rest of the record in chunks (watch out for // exceeding the record size, since this will result in BMC's @@ -257,13 +259,25 @@ protected SensorRecord getSensorViaChunks(int reservationId) throws Exception { read += bytesToRead; } - // Finally we populate the sensor record with the gathered - // data... - SensorRecord sensorDataToPopulate = SensorRecord.populateSensorRecord(bytes); - - // ... and update the ID of the next record + // Advance to the next record, then decode the gathered data nextRecId = data.getNextRecordId(); - return sensorDataToPopulate; + return decodeRecord(bytes); + } + + /** + * Decode a raw SDR record. A record the library cannot model (unknown or malformed type) is logged and skipped + * instead of aborting the whole repository walk. + * + * @param recordData Raw bytes of the SDR record + * @return {@link SensorRecord} instance or null if the record cannot be decoded + */ + private SensorRecord decodeRecord(byte[] recordData) { + try { + return SensorRecord.populateSensorRecord(recordData); + } catch (RuntimeException e) { + LOGGER.warn("Skipping undecodable SDR record before id {} on {}: {}", nextRecId, ipmiConfiguration.getHostname(), e.getMessage()); + return null; + } } } diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/OemRecord.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/OemRecord.java index 4552c00..70d648c 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/OemRecord.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/OemRecord.java @@ -37,17 +37,23 @@ public class OemRecord extends SensorRecord { protected void populateTypeSpecficValues(byte[] recordData, SensorRecord record) { + // Vendor-specific record types (C1h-FFh) don't necessarily follow the + // C0h layout (3-byte manufacturer ID + data), so guard every offset. byte[] buffer = new byte[4]; - System.arraycopy(recordData, 5, buffer, 0, 3); + if (recordData.length >= 8) { + System.arraycopy(recordData, 5, buffer, 0, 3); + } buffer[3] = 0; setManufacturerId(TypeConverter.littleEndianByteArrayToInt(buffer)); - byte[] data = new byte[recordData.length - 8]; + byte[] data = new byte[Math.max(0, recordData.length - 8)]; - System.arraycopy(recordData, 8, data, 0, data.length); + if (data.length > 0) { + System.arraycopy(recordData, 8, data, 0, data.length); + } setOemData(data); } diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorRecord.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorRecord.java index 284165b..17cd9dd 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorRecord.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorRecord.java @@ -91,6 +91,13 @@ public static SensorRecord populateSensorRecord(byte[] recordData) { sensorRecord = new OemRecord(); break; default: + // IPMI 2.0 Table 43-1 reserves C0h-FFh for OEM records; vendors use + // values above C0h (e.g. NVIDIA/GIGABYTE emit D0h). Never abort the + // SDR repository walk on a record type we don't model. + if (TypeConverter.byteToInt(recType) >= TypeConverter.byteToInt(RecordTypes.OEM_RECORD)) { + sensorRecord = new OemRecord(); + break; + } throw new IllegalArgumentException("Invalid record type: " + recType); } From 255b812764c32701c78eb982a188927118f7fbfd Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Wed, 7 Oct 2026 12:06:17 +0200 Subject: [PATCH 2/4] Keep the whole payload of vendor-defined SDR record types in OemRecord Only the C0h OEM record defines a 3-byte manufacturer ID followed by OEM data. Record types C1h-FFh, which populateSensorRecord now also maps to OemRecord, have a vendor-defined layout: interpreting bytes 5-7 as a manufacturer ID fabricated an ID and dropped the first three payload bytes. For those types the manufacturer ID stays 0 and getOemData() returns the entire type-specific payload. Co-Authored-By: Claude Fable 5.1 --- .../coding/commands/sdr/record/OemRecord.java | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/OemRecord.java b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/OemRecord.java index 70d648c..2c4d6f3 100644 --- a/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/OemRecord.java +++ b/src/main/java/org/metricshub/ipmi/core/coding/commands/sdr/record/OemRecord.java @@ -29,6 +29,16 @@ */ public class OemRecord extends SensorRecord { + /** + * Size of the common SDR record header (record ID, SDR version, record type, record length). + */ + private static final int HEADER_LENGTH = 5; + + /** + * Size of the manufacturer ID field in a C0h OEM record. + */ + private static final int MANUFACTURER_ID_LENGTH = 3; + private int manufacturerId; private byte[] oemData; @@ -37,22 +47,23 @@ public class OemRecord extends SensorRecord { protected void populateTypeSpecficValues(byte[] recordData, SensorRecord record) { - // Vendor-specific record types (C1h-FFh) don't necessarily follow the - // C0h layout (3-byte manufacturer ID + data), so guard every offset. - byte[] buffer = new byte[4]; - - if (recordData.length >= 8) { - System.arraycopy(recordData, 5, buffer, 0, 3); + // Only the C0h OEM record has a defined layout (3-byte manufacturer ID + // followed by OEM data). Vendor-defined record types C1h-FFh carry an + // unknown layout, so keep their whole type-specific payload and leave + // the manufacturer ID at 0 (unknown). + int dataOffset = HEADER_LENGTH; + + if (recordData[3] == RecordTypes.OEM_RECORD && recordData.length >= HEADER_LENGTH + MANUFACTURER_ID_LENGTH) { + byte[] buffer = new byte[4]; + System.arraycopy(recordData, HEADER_LENGTH, buffer, 0, MANUFACTURER_ID_LENGTH); + setManufacturerId(TypeConverter.littleEndianByteArrayToInt(buffer)); + dataOffset += MANUFACTURER_ID_LENGTH; } - buffer[3] = 0; - - setManufacturerId(TypeConverter.littleEndianByteArrayToInt(buffer)); - - byte[] data = new byte[Math.max(0, recordData.length - 8)]; + byte[] data = new byte[Math.max(0, recordData.length - dataOffset)]; if (data.length > 0) { - System.arraycopy(recordData, 8, data, 0, data.length); + System.arraycopy(recordData, dataOffset, data, 0, data.length); } setOemData(data); From 7aac625fb69a4d4b2fe1a1d5fce932d9ef64cb78 Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Wed, 7 Oct 2026 12:55:00 +0200 Subject: [PATCH 3/4] Add AGENTS.md with project instructions for AI agents, imported by CLAUDE.md Same convention as jawk: CLAUDE.md only imports AGENTS.md. The instructions describe this repository as it is today (Java 8 target, license headers, no formatter yet, ungated quality reports and the workaround for SpotBugs on recent JDKs) plus the IPMI-specific lessons from the October 2026 review: never abort an SDR or FRU walk on an unknown record, keep BMC credentials out of the repository, and expect dropped UDP replies when testing against real hardware. Co-Authored-By: Claude Fable 5.1 --- AGENTS.md | 37 +++++++++++++++++++++++++++++++++++++ CLAUDE.md | 1 + 2 files changed, 38 insertions(+) create mode 100644 AGENTS.md create mode 100644 CLAUDE.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..8c91e08 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,37 @@ +# Instructions for AI Agents + +## Code format + +The project has not adopted the MetricsHub Eclipse formatter profile yet (see issue #118). Until it does, match the style of the file you are editing: the `org.metricshub.ipmi.client` packages use tabs, the `org.metricshub.ipmi.core` packages (forked from the Verax IPMI library) use 4 spaces. Do not reformat code you are not otherwise changing. Once `formatter-maven-plugin` is configured, simply run `mvn formatter:format` before committing. + +All files must include the proper LGPL-3 license header. When you add a new file, run `mvn license:update-file-header` before committing (and before building, since the build fails if a file lacks the header). + +All public methods must have proper Javadoc. Check the output of Maven to identify issues with Javadoc and fix these issues. + +The library targets **Java 8** (`maven.compiler.release` is 8): no `var`, records, switch expressions, text blocks or APIs newer than Java 8 in `src/main`. + +## Build + +The project uses Maven to build. A full build is performed with `mvn verify site` (or `mvn clean verify site` when applicable). CI runs the same on JDK 17. + +@codex, please don't try to use `mvnw` (Maven Wrapper). Maven is already installed and runs perfectly well. + +## Test + +Whenever required, when you add code or when you modify code that is not covered with unit tests, add the corresponding unit tests. All tests must pass with `mvn test`. Don't use the `-q` (silent) option, as you want to see the result of successful tests. Tests are run with the Maven surefire plugin and results are stored in the ./target/surefire-reports directory. + +Unit tests must not depend on a real BMC. Exercising the RMCP+ session code against real hardware is done with throw-away harnesses outside the repository; never commit hostnames, user names or passwords of test systems. When testing against a real BMC, keep in mind that BMCs drop UDP replies under concurrent sessions: repeat a failing request before blaming the library, set a short per-message timeout with `IpmiConnector.setTimeout(handle, ms)`, and end harness `main` methods with `System.exit(0)` because the library leaves non-daemon threads behind after a timeout. + +## Code quality reports + +Code quality reports (checkstyle, pmd/cpd, spotbugs) are generated by `mvn verify site` into ./target/checkstyle-result.xml, ./target/pmd.xml, ./target/cpd.xml and ./target/spotbugsXml.xml. They are not yet gated (issues #114, #115, #116, #117 track the clean-up). Do not add new violations: check the reports for the files you changed before committing and submitting your code. On JDK 21+ the SpotBugs plugin version inherited from the parent POM cannot read the JDK class files; run `mvn com.github.spotbugs:spotbugs-maven-plugin:4.10.4.1:spotbugs` instead. + +## Documentation + +Always make sure that public API changes are properly documented in src/site/markdown/*.md and that README.md is always up-to-date. The published documentation is https://metricshub.org/ipmi-java (see issue #113 for the planned overhaul). + +## IPMI specifics + +- The protocol implementation lives in `org.metricshub.ipmi.core` (RMCP+, RAKP, SDR/FRU/SEL coders, connection state machine); the MetricsHub-facing API is `org.metricshub.ipmi.client` (`IpmiClient`, `IpmiClientConfiguration`, the runners and `IpmiResultConverter`). +- Decoders must never abort a whole SDR repository walk or FRU decode on a record they do not model: skip, log and continue (IPMI 2.0 reserves SDR record types 0xC0-0xFF for OEM use and vendors do emit them). +- Byte-level parsing must follow the IPMI 2.0 specification (section numbers are cited in the code and in the issues); when a finding is "by the spec" but not reproduced on real hardware, say so in the commit or PR. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..43c994c --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md From a602881af991d8a420287d8c426071aa9bb58206 Mon Sep 17 00:00:00 2001 From: Bertrand Martin Date: Wed, 7 Oct 2026 13:12:44 +0200 Subject: [PATCH 4/4] Fall back to chunked reads on truncated Get SDR replies; add OEM record tests Some BMCs answer a whole-record Get SDR with a success code but fewer bytes than the record length declared in the SDR header. Advancing to the next record before decoding would have turned such a record into a silent skip, so the runner now detects the short reply and uses the existing chunked read while the record ID is still known. The chunked path also tolerates a BMC returning fewer bytes than requested instead of throwing ArrayIndexOutOfBounds. New unit tests cover the OEM decoding path (vendor-defined types keep their whole payload, C0h parses the manufacturer ID, short records do not throw, reserved types are still rejected), the runner's skip behaviour for undecodable records, and the truncation check. AGENTS.md now states that the LGPL header requirement applies to the Java sources under src/main/java only, which is what the license plugin checks; tests, Markdown and resources carry no header. Co-Authored-By: Claude Fable 5.1 --- AGENTS.md | 2 +- .../client/runner/AbstractIpmiRunner.java | 50 +++++++++-- .../client/runner/AbstractIpmiRunnerTest.java | 74 +++++++++++++++++ .../commands/sdr/record/SensorRecordTest.java | 82 +++++++++++++++++++ 4 files changed, 201 insertions(+), 7 deletions(-) create mode 100644 src/test/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunnerTest.java create mode 100644 src/test/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorRecordTest.java diff --git a/AGENTS.md b/AGENTS.md index 8c91e08..e7a49bf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,7 +4,7 @@ The project has not adopted the MetricsHub Eclipse formatter profile yet (see issue #118). Until it does, match the style of the file you are editing: the `org.metricshub.ipmi.client` packages use tabs, the `org.metricshub.ipmi.core` packages (forked from the Verax IPMI library) use 4 spaces. Do not reformat code you are not otherwise changing. Once `formatter-maven-plugin` is configured, simply run `mvn formatter:format` before committing. -All files must include the proper LGPL-3 license header. When you add a new file, run `mvn license:update-file-header` before committing (and before building, since the build fails if a file lacks the header). +All Java source files under `src/main/java` must include the proper LGPL-3 license header (the `license-maven-plugin` check covers `main/java/**/*.java` only; tests, Markdown and resources carry no header). When you add a new source file, run `mvn license:update-file-header` before committing (and before building, since the build fails if a source file lacks the header). All public methods must have proper Javadoc. Check the output of Maven to identify issues with Javadoc and fix these issues. diff --git a/src/main/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunner.java b/src/main/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunner.java index 7bd6cba..77ade3f 100644 --- a/src/main/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunner.java +++ b/src/main/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunner.java @@ -196,6 +196,13 @@ protected SensorRecord getSensorData(int reservationId) throws Exception { GetSdrResponseData data = (GetSdrResponseData) connector.sendMessage(handle, new GetSdr(IpmiVersion.V20, handle.getCipherSuite(), AuthenticationType.RMCPPlus, reservationId, nextRecId)); + // Some BMCs answer with success but return fewer bytes than the + // record length declared in the header: fall back to the chunked + // read while the current record ID is still known + if (isTruncated(data.getSensorRecordData())) { + return getSensorViaChunks(reservationId); + } + // Advance to the next record first, so that a record we cannot // decode never stalls the whole repository walk nextRecId = data.getNextRecordId(); @@ -231,14 +238,22 @@ protected SensorRecord getSensorViaChunks(int reservationId) throws Exception { GetSdrResponseData data = (GetSdrResponseData) connector.sendMessage(handle, new GetSdr(IpmiVersion.V20, handle.getCipherSuite(), AuthenticationType.RMCPPlus, reservationId, nextRecId, 0, INITIAL_CHUNK_SIZE)); + byte[] header = data.getSensorRecordData(); + if (header == null || header.length < HEADER_SIZE) { + LOGGER.warn("Skipping SDR record {} on {}: BMC returned {} byte(s) instead of the {}-byte header", nextRecId, + ipmiConfiguration.getHostname(), header == null ? 0 : header.length, HEADER_SIZE); + nextRecId = data.getNextRecordId(); + return null; + } + // The record size is 5th byte of the record. It does not take // into account the size of the header, so we need to add it. - int recSize = TypeConverter.byteToInt(data.getSensorRecordData()[4]) + HEADER_SIZE; - int read = INITIAL_CHUNK_SIZE; + int recSize = TypeConverter.byteToInt(header[HEADER_SIZE - 1]) + HEADER_SIZE; + int read = Math.min(header.length, recSize); byte[] bytes = new byte[recSize]; - System.arraycopy(data.getSensorRecordData(), 0, bytes, 0, Math.min(recSize, data.getSensorRecordData().length)); + System.arraycopy(header, 0, bytes, 0, read); // We get the rest of the record in chunks (watch out for // exceeding the record size, since this will result in BMC's @@ -253,10 +268,19 @@ protected SensorRecord getSensorViaChunks(int reservationId) throws Exception { GetSdrResponseData part = (GetSdrResponseData) connector.sendMessage(handle, new GetSdr(IpmiVersion.V20, handle.getCipherSuite(), AuthenticationType.RMCPPlus, reservationId, nextRecId, read, bytesToRead)); + byte[] chunk = part.getSensorRecordData(); + int got = chunk == null ? 0 : Math.min(bytesToRead, chunk.length); + if (got == 0) { + LOGGER.warn("Skipping SDR record {} on {}: BMC returned no data at offset {}", nextRecId, + ipmiConfiguration.getHostname(), read); + nextRecId = data.getNextRecordId(); + return null; + } + // Append the new bytes - System.arraycopy(part.getSensorRecordData(), 0, bytes, read, bytesToRead); + System.arraycopy(chunk, 0, bytes, read, got); - read += bytesToRead; + read += got; } // Advance to the next record, then decode the gathered data @@ -272,7 +296,7 @@ protected SensorRecord getSensorViaChunks(int reservationId) throws Exception { * @param recordData Raw bytes of the SDR record * @return {@link SensorRecord} instance or null if the record cannot be decoded */ - private SensorRecord decodeRecord(byte[] recordData) { + SensorRecord decodeRecord(byte[] recordData) { try { return SensorRecord.populateSensorRecord(recordData); } catch (RuntimeException e) { @@ -280,4 +304,18 @@ private SensorRecord decodeRecord(byte[] recordData) { return null; } } + + /** + * Detect a whole-record GetSdr response that is shorter than the record length declared in its own header. Some BMCs + * answer such requests with a success completion code and a truncated body instead of "Cannot return number of + * requested data bytes". + * + * @param recordData Raw bytes returned for a whole-record GetSdr request + * @return true if the header is missing or fewer bytes than declared were returned + */ + static boolean isTruncated(byte[] recordData) { + return recordData == null + || recordData.length < HEADER_SIZE + || recordData.length < TypeConverter.byteToInt(recordData[HEADER_SIZE - 1]) + HEADER_SIZE; + } } diff --git a/src/test/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunnerTest.java b/src/test/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunnerTest.java new file mode 100644 index 0000000..acd1099 --- /dev/null +++ b/src/test/java/org/metricshub/ipmi/client/runner/AbstractIpmiRunnerTest.java @@ -0,0 +1,74 @@ +package org.metricshub.ipmi.client.runner; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; +import org.metricshub.ipmi.client.IpmiClientConfiguration; +import org.metricshub.ipmi.core.coding.commands.sdr.record.FullSensorRecord; +import org.metricshub.ipmi.core.coding.commands.sdr.record.OemRecord; + +class AbstractIpmiRunnerTest { + + /** + * Minimal concrete runner: no session, no connector, just the decoding helpers under test. + */ + private static final AbstractIpmiRunner RUNNER = new AbstractIpmiRunner( + new IpmiClientConfiguration("bmc", "user", new char[0], null, false, 1)) { + @Override + public Void call() { + return null; + } + }; + + private static byte[] record(int type, byte[] payload) { + byte[] raw = new byte[5 + payload.length]; + raw[2] = 0x51; + raw[3] = (byte) type; + raw[4] = (byte) payload.length; + System.arraycopy(payload, 0, raw, 5, payload.length); + return raw; + } + + @Test + void decodeRecordReturnsOemRecordForVendorDefinedType() { + assertInstanceOf(OemRecord.class, RUNNER.decodeRecord(record(0xd0, new byte[] {1, 2, 3}))); + } + + @Test + void decodeRecordSkipsRecordsItCannotDecodeInsteadOfThrowing() { + // reserved (non-OEM) record type + assertNull(RUNNER.decodeRecord(record(0x05, new byte[] {1}))); + // shorter than the SDR header + assertNull(RUNNER.decodeRecord(new byte[] {0x00})); + // full sensor record whose body is missing: decoder runs out of bytes + assertNull(RUNNER.decodeRecord(record(0x01, new byte[] {1, 2, 3}))); + } + + @Test + void decodeRecordStillDecodesValidRecords() { + // A Full Sensor Record (type 01h) of minimal valid size: 48 bytes, the last one being the ID string type/length + byte[] payload = new byte[43]; + payload[0] = 0x20; // sensor owner: BMC + payload[7] = 0x01; // sensor type: temperature + payload[8] = 0x01; // event/reading type: threshold + payload[16] = 0x01; // base unit: degrees C + payload[42] = (byte) 0xc0; // 8-bit ASCII, zero-length name + assertInstanceOf(FullSensorRecord.class, RUNNER.decodeRecord(record(0x01, payload))); + } + + @Test + void isTruncatedDetectsWholeRecordResponsesShorterThanDeclared() { + byte[] complete = record(0xd0, new byte[] {1, 2, 3, 4, 5}); + assertFalse(AbstractIpmiRunner.isTruncated(complete)); + + byte[] truncated = new byte[complete.length - 2]; + System.arraycopy(complete, 0, truncated, 0, truncated.length); + assertTrue(AbstractIpmiRunner.isTruncated(truncated), "declared length 5 but only 3 payload bytes returned"); + + assertTrue(AbstractIpmiRunner.isTruncated(new byte[] {0x00, 0x00, 0x51}), "header itself is incomplete"); + assertTrue(AbstractIpmiRunner.isTruncated(null)); + } +} diff --git a/src/test/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorRecordTest.java b/src/test/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorRecordTest.java new file mode 100644 index 0000000..7d513f3 --- /dev/null +++ b/src/test/java/org/metricshub/ipmi/core/coding/commands/sdr/record/SensorRecordTest.java @@ -0,0 +1,82 @@ +package org.metricshub.ipmi.core.coding.commands.sdr.record; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; + +class SensorRecordTest { + + /** + * Build a raw SDR record: 2-byte record ID (LE), SDR version 0x51, record type, record length, then the payload. + */ + private static byte[] record(int id, int type, byte[] payload) { + byte[] raw = new byte[5 + payload.length]; + raw[0] = (byte) (id & 0xff); + raw[1] = (byte) ((id >> 8) & 0xff); + raw[2] = 0x51; + raw[3] = (byte) type; + raw[4] = (byte) payload.length; + System.arraycopy(payload, 0, raw, 5, payload.length); + return raw; + } + + @Test + void vendorDefinedRecordTypeIsDecodedAsOemRecordWithWholePayload() { + // Record type D0h as emitted by GIGABYTE/NVIDIA BMCs: vendor-defined layout, no manufacturer ID field + byte[] payload = {0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f}; + + SensorRecord record = SensorRecord.populateSensorRecord(record(0x0004, 0xd0, payload)); + + OemRecord oem = assertInstanceOf(OemRecord.class, record); + assertEquals(4, oem.getId()); + assertEquals(0, oem.getManufacturerId(), "manufacturer ID is unknown for vendor-defined record types"); + assertArrayEquals(payload, oem.getOemData(), "the complete type-specific payload must be preserved"); + } + + @Test + void lastOemRecordTypeIsAccepted() { + byte[] payload = {0x01, 0x02}; + + OemRecord oem = assertInstanceOf(OemRecord.class, SensorRecord.populateSensorRecord(record(0x0010, 0xff, payload))); + + assertArrayEquals(payload, oem.getOemData()); + } + + @Test + void standardOemRecordTypeC0ParsesManufacturerIdAndData() { + // C0h layout: 3-byte manufacturer ID (LE, 0x000157 = 343) followed by OEM data + byte[] payload = {0x57, 0x01, 0x00, 0x21, 0x22}; + + OemRecord oem = assertInstanceOf(OemRecord.class, SensorRecord.populateSensorRecord(record(0x0020, 0xc0, payload))); + + assertEquals(343, oem.getManufacturerId()); + assertArrayEquals(new byte[] {0x21, 0x22}, oem.getOemData()); + } + + @Test + void shortOemRecordDoesNotThrow() { + // C0h record without even a complete manufacturer ID: no ID is fabricated and the bytes are kept as payload + OemRecord oem = assertInstanceOf(OemRecord.class, SensorRecord.populateSensorRecord(record(0x0030, 0xc0, new byte[] {0x57}))); + + assertEquals(0, oem.getManufacturerId()); + assertArrayEquals(new byte[] {0x57}, oem.getOemData()); + + // header only + OemRecord empty = assertInstanceOf(OemRecord.class, SensorRecord.populateSensorRecord(record(0x0031, 0xc0, new byte[0]))); + assertEquals(0, empty.getOemData().length); + } + + @Test + void unknownStandardRecordTypeIsRejected() { + // 05h is reserved by IPMI 2.0 and not an OEM type: callers are expected to skip it + assertThrows(IllegalArgumentException.class, () -> SensorRecord.populateSensorRecord(record(0x0040, 0x05, new byte[] {0x00}))); + } + + @Test + void recordShorterThanHeaderIsRejected() { + assertThrows(IllegalArgumentException.class, () -> SensorRecord.populateSensorRecord(new byte[] {0x00, 0x00, 0x51})); + } +}