@soniah can I ask you to look at it?

What issue does this fix?

What issue does this fix?

I’m currently out of office and shortly for now I can mention what I remember

• unmarshall was not returning anything for zero value while snmp requires value even if zero

Tommorow I will ask my developer for technical justification of this change and some snmp spec refering to it.

With this change my snmp simulator with your library successfully discover my devices (represented by *.snmprec files)

I think I understand the first part, always returning a single empty byte. But I'm not sure about the second half.

I think I understand the first part, always returning a single empty byte. But I'm not sure about the second half.

I asked AI to explain as I cannot repeat exactly my college (I will ask him tommorow to confirm)

🔧 The Issue

In helper.go, the unmarshalInt64 implementation had a subtle bug: when decoding a value of zero, it was not returning 0 properly. Instead, due to how length or payload were interpreted, it returned an error or a nil value. The correct behavior—as per ASN.1 BER encoding—is to return 0 when the integer data is exactly zero.

⸻

✅ What the Fixed Version Does
1. Detects “zero-length” or empty payloads
If the BER length indicates zero (no payload), the code now returns the integer 0. That’s the correct representation for 0.
2. Properly parses byte payload
Reads each byte of the payload (1 to 8 bytes, inclusive), shifting bits and building the integer.
3. Handles sign extension correctly
If the first payload byte has its highest bit set (e.g. 0x80), the code ensures the integer is interpreted as a signed value, adjusting the result to negative as intended.
4. Supports 8-bit and up to 64-bit integers
The loop reads up to 8 bytes, so small values (like a single zero byte 0x00) or full 8-byte values (e.g. 0x00…80) work correctly.

I'm pretty sure the second half of this is unnecessary, as the binary.BigEndian function does the necessary bit handling.

If this is not correct, tests need to be updated in order to prove the change does the correct thing.

I'm pretty sure the second half of this is unnecessary, as the binary.BigEndian function does the necessary bit handling.

If this is not correct, tests need to be updated in order to prove the change does the correct thing.

I will check tommorow if it works without this part however this won’t be a proof that it won’t break some other scenario. I will ask my college to comment that.

By the way … yes, tests are crutial :-) I’m a QA/QC engineer and write a bit some tests software.

Current implementation did not handle case properly when Counter64 value was 0 or 255(The MSB is negative):

func marshalUint64(v interface{}) []byte {
	bs := make([]byte, 8)
	source := v.(uint64)
	binary.BigEndian.PutUint64(bs, source) // will panic on failure
	// truncate leading zeros. Cleaner technique?
	return bytes.TrimLeft(bs, "\x00")
}

This implementation for 0 returns nil.

Problem with arraises in marshall.go when writing to buffer:

intBytes := marshalUint64(pdu.Value)
tmpBuf.WriteByte(byte(len(intBytes)))
tmpBuf.Write(intBytes)

the len(nil) is 0, but actuall value is sent so length byte is missing:
e.g.
[48 12 6 8 43 6 1 2 1 1 1 0 70 0]
[48 13 6 8 43 6 1 2 1 1 1 0 70 1 0]

Lets look into 0 and 255 case in new implementation:

• 0:

value := uint64(0)

bs := make([]byte, 8)  // [0, 0, 0, 0, 0, 0, 0, 0]

binary.BigEndian.PutUint64(bs, 0)  // [0, 0, 0, 0, 0, 0, 0, 0]

trimmed := bytes.TrimLeft(bs, "\x00")  // []
// This DOES get trimmed! All zeros are removed.

if len(trimmed) == 0 {  // true, length is 0
    return []byte{0}
}

// Result: [0] ----- previously it would be nil

• 255:

value := uint64(255)

bs := make([]byte, 8)  // [0, 0, 0, 0, 0, 0, 0, 0]

binary.BigEndian.PutUint64(bs, 255)  // [0, 0, 0, 0, 0, 0, 0, 255]

trimmed := bytes.TrimLeft(bs, "\x00")  // []
// This DOES get trimmed! All zeros are removed.

if len(trimmed) == 0 {  // true, length is 0
    return []byte{0}
}
//this condition now is true -> we prepend leading 0 so it is not treated as negative according to BER
if trimmed[0]&0x80 > 0 {
		trimmed = append([]byte{0}, trimmed...)
	}
	return trimmed

// Result: [0, 255] ----- previously it would be [255]

Function parseUint64 checks for leading zeros assumming the size can be up to 9 bytes.

As mentioned earlier, the change works as follows:

• Encode the value in big endian format (using PutUint64).
• Remove leading zero bytes on the left side (TrimLeft)—these bytes do not provide any information and should be removed according to BER number encoding rules.
• If all bytes were zero (it means input value is 0) the array is empty, just return 0.
• If the remaining bytes would represent a negative value (i.e., the leftmost bit is 1), prepend an additional zero byte to ensure the value is interpreted as positive. This is necessary to adjust the sign, since Counter64 is stored as a 64-bit unsigned integer and should always be positive (see below). BER encoding uses endian encoding, but there is no special treatment for unsigned numbers. To ensure the number is positive, it must not start with the leftmost bit set to 1. This is why the final condition prepends zero if necessary.

Counter64 ::=
    [APPLICATION 6]
        IMPLICIT INTEGER (0..18446744073709551615)