Skip to content

NULL Line/Font Attributes and Foreground Colour references are rejected, and from_iop silently truncates the pool #47

Description

@Arjan-Woltjer

Summary

Object::read rejects any object whose Line Attributes, Font Attributes or (Input Boolean) Foreground Colour reference is the NULL object ID (0xFFFF), returning ParseError::UnexpectedNullObjectId. ISO 11783-6 allows NULL for all of these, and real-world pools use it. Because ObjectPool::from_iop / extend_with_iop loop with while let Ok(o) = Object::read(..), the first such object silently ends the parse and every object after it is dropped — with no error surfaced.

The user-visible symptom in AgIsoTerminalDesigner (which builds from daan/terminal-designer-changes) is a pool that loads with no Working Set, because vendors such as John Deere put the Working Set object last.

Where

src/object_pool/reader.rs (at 5274565, the head of daan/terminal-designer-changes; main has the same code):

object field line
InputBoolean foreground_colour 500
InputString font_attributes 522
InputNumber font_attributes 547
OutputString font_attributes 602
OutputNumber font_attributes 625
OutputLine line_attributes 649
OutputRectangle line_attributes 668
OutputEllipse line_attributes 688
OutputPolygon line_attributes 712

each of the form Self::read_u16(data)?.try_into()? → ObjectId::new → Err(UnexpectedNullObjectId) on 0xFFFF.

Then src/object_pool/object_pool.rs:85:

while let Ok(o) = Object::read(&mut data) {

which treats any ParseError the same as end-of-data.

Evidence that NULL is legal here

  • ISO 11783-6 (2004 edition, Table for Output Polygon, bytes 8-9): "Object ID of a line attributes object to use for the line attributes", range 0-65534, 65535 — NULL explicitly allowed.
  • AgIsoStack++ (isobus_virtual_terminal_objects.cpp) accepts NULL for every one of these fields: OutputLine/OutputRectangle/OutputEllipse/OutputPolygon::get_is_valid — "Verify the line attributes is a line attribute or NULL_OBJECT_ID"; OutputString/OutputNumber/InputNumber::get_is_valid — "Verify that the font attributes is a font attribute or NULL_OBJECT_ID"; InputBoolean::get_is_valid — "Verify that the foreground colour is a font attribute or NULL_OBJECT_ID". The C++ deserializer reads the same pools completely.
  • A real terminal accepts them. The pools below were captured off a live ISOBUS; the John Deere VT answered End of Object Pool with error bitmask 0x00 for each.

Real-world pools that trigger it

Captured with a CANedge3 on a John Deere tractor (2026-09-11) and reassembled byte-exact (the ECU's Get Memory request declared 327 501 bytes; 327 501 were received; the same pool was uploaded twice and both copies are byte-identical):

pool size objects NULL refs the reader rejects reader stops at objects kept Working Set
StarFire GNSS receiver (0x1C) 327 501 B 9 619 line attrs: 109 OutputLine, 63 OutputRectangle, 40 OutputEllipse, 1 OutputPolygon; font attrs: 3 OutputString, 4 InputString; foreground colour: 38 of 38 InputBoolean offset 0x29FD0 (object 1 640, OutputLine id 1042) 1 638 0 (it is object 9 618, at 99.6 %)
Tractor ECU (0xF0) 21 643 B 47 line attrs: 1 OutputPolygon offset 0x4718 (object 31) 30 0 (it is at 99.5 %)

I have not attached the vendor pools. Here is a 31-byte reproducer that has nothing vendor-specific in it — an Output Line with NULL line attributes, followed by an ordinary Working Set and Data Mask:

01 00 0d ff ff 70 00 01 00 00 00              Output Line, id 1, line attributes NULL
00 00 00 f0 01 02 00 00 00 01 65 6e           Working Set, id 0, active mask 2, language "en"
02 00 01 f0 ff ff 00 00                       Data Mask, id 2

At 5274565:

Object::read  -> Err(UnexpectedNullObjectId) at offset 0, after 5 bytes
ObjectPool::from_iop -> 0 objects, 0 WorkingSet(s)

Relationship to #40

#40 (Fix objects that can have a NULL object id as attribute) makes fill_attributes, variable_reference, soft_key_mask, list items etc. nullable and is largely already on daan/terminal-designer-changes — but it leaves line_attributes, font_attributes and foreground_colour as non-nullable ObjectId (its object.rs diff still pushes them with refs.push(o.line_attributes)), so the pools above fail with or without it.

Proposed fix

  1. Make the nine fields NullableObjectId and read them with .into(); collect them with push_nullable_id in Object::referenced_objects. write_u16 already accepts NullableObjectId, so the writer needs no change and NULL round-trips as 0xFFFF.
  2. Add ObjectPool::try_from_iop / try_extend_with_iop that return the first ParseError instead of swallowing it, leaving from_iop lenient as before. Callers that want to tell "the pool ended" from "the pool could not be read" then can.

With (1), all four pools above parse to their last byte (9 619 / 47 / 105 / 26 objects) and the Working Sets are found. PR follows.

This is a type change on public struct fields, so AgIsoTerminalDesigner needs a small companion change (it already handles fill_attributes the same way); I'll open that there and link it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions