AMQP 1 Client Delphi Update

· Recursos

The AMQP 1.0 protocol implementation em sgcWebSockets has undergone um comprehensive review against the OASIS AMQP 1.0 specification. Este artigo documenta o 30 fixes applied across 8 source files, covering critical bugs, memory leaks, specification compliance, state machine correctness, heartbeat handling, e thread safety improvements.

Sumário

  1. Overview
  2. Critical Bug Fixes
  3. Memory Leak Fixes
  4. Specification Compliance Fixes
  5. State Machine & Conexão Fixes
  6. Heartbeat & Idle Timeout Fixes
  7. Thread Safety Fixes
  8. Files Modified

1. Overview

A total de 30 fixes were applied across 6 source files e 2 interface files no AMQP 1.0 implementation. The fixes are categorized como segue:

Category Count Severity
Critical Bug Fixes 6 Critical
Memory Leak Fixes 4 Critical
Specification Compliance 10 High
State Machine & Conexão 5 High
Heartbeat & Idle Timeout 3 High
Thread Safety 2 High

2. Critical Bug Fixes

These fixes address bugs that would cause immediate runtime failures, protocol corruption, ou incorrect behavior during AMQP 1.0 communication.

2.1 Missing raise Keyword em Exception

File: sgcAMQP1_Classes.pas
An exception object was created but nunca raised when um invalid frame type was encountered. The exception was silently discarded, allowing corrupt frames para be processed sem detection.

Before (Broken)

TsgcAMQP1Exception.CreateFmt(S_AMQP1_INVALID_FRAME_TYPE, [vByte]);

After (Fixed)

raise TsgcAMQP1Exception.CreateFmt(S_AMQP1_INVALID_FRAME_TYPE, [vByte]);

2.2 WriteMap Map32 Missing Data e Wrong Size

File: sgcAMQP1_Classes.pas
The Map32 encoding path was missing o WriteBytes call para o actual map data, e o size field used um incorrect offset. Map32 uses um 4-byte count field (unlike Map8 which uses 1 byte), so o size must include 4 extra bytes.

Before (Broken)

else
begin
  WriteUByte(Ord(amqp1DataMap32));
  _WriteUInt32(vSize + 1);
  _WriteUInt32(oJSON.Count * 2);
  // Missing: WriteBytes(oArray.Bytes);
end;

After (Fixed)

else
begin
  WriteUByte(Ord(amqp1DataMap32));
  _WriteUInt32(vSize + 4);
  _WriteUInt32(oJSON.Count * 2);
  WriteBytes(oArray.Bytes);
end;

2.3 Inverted ContainsError Logic

File: sgcAMQP1_Frames.pas
The ContainsError method em both TsgcAMQP1FrameRejected and TsgcAMQP1DescribedListError returned True when there was no error and False when there was um erro. This caused error information para be silently dropped e null bytes para be written when actual errors should foram serialized. The DoWrite and DoWriteError branches were also swapped para match o corrected logic.

Before (Broken)

function TsgcAMQP1FrameRejected.ContainsError: Boolean;
begin
  if not Assigned(FError) then
    Result := True              // Wrong: True when no error
  else
    Result := (Error.Condition = '') and (Error.Description = '') and
      (Error.Info = '');     // Wrong: True when all empty
end;

After (Fixed)

function TsgcAMQP1FrameRejected.ContainsError: Boolean;
begin
  if not Assigned(FError) then
    Result := False             // Correct: False when no error
  else
    Result := (Error.Condition <> '') or (Error.Description <> '') or
      (Error.Info <> '');    // Correct: True when any field set
end;

2.4 SASL PLAIN Null Byte Separators

File: sgcAMQP1_Frames.pas
The SASL PLAIN mechanism requer o formato \0username\0password com null byte ($00) separators. The implementation was not initializing o byte array para zero, so separator positions contained garbage data. Autenticação would fail against any standards-compliant AMQP 1.0 broker.

After (Fixed)

SetLength(FInitialResponse, 2 + Length(oUser) + Length(oPassword));
FillChar(FInitialResponse[0], Length(FInitialResponse), 0);  // Zero-fill for null separators

2.5 Missing inherited Create em TsgcAMQP1Message

File: sgcAMQP1_Message.pas
The parameterized constructor de TsgcAMQP1Message did not call inherited Create, meaning o base class was nunca initialized. This would cause access violations ou corrupt state when using o convenience constructor.

After (Fixed)

constructor TsgcAMQP1Message.Create(const aValue: string);
begin
  inherited Create;
  ApplicationData.ValueType := amqp1adtAmqpValue;
  ApplicationData.AMQPValue.Value := aValue;
end;

2.6 Missing Semicolon em AmqpValue.DoRead

File: sgcAMQP1_Frames.pas
A missing semicolon em TsgcAMQP1FrameAmqpValue.DoRead would prevent compilation.

3. Memory Leak Fixes

These fixes address object lifetime management issues that would cause memory para accumulate over time, particularly during long-running AMQP 1.0 conexões com many message exchanges.

Fix File Description
3.1 sgcAMQP1_Frames.pas FDefaultOutcome was not freed before reassignment when reading o Source descriptor. Each time um novo default outcome was received, o previous object was leaked.
3.2 sgcAMQP1_Session.pas Duplicate sgcFree(FCreditConsumed) call no destructor caused um potential double-free. Removed o duplicate line.
3.3 sgcAMQP1_Session.pas FOutgoingDeliveries was missing do session destructor. The delivery tracking list was nunca freed when um session was destroyed.
3.4 sgcAMQP1_Message.pas SetMessage e SetMessageAndFreeOnDestroy did not free o previous message when FFreeMessageOnDestroy was enabled. Repeated message assignments leaked memory.

Fix 3.1 – FDefaultOutcome Freed Before Reassignment

sgcFree(FDefaultOutcome);  // Free previous instance before reassignment
if oDescriptor.Code = amqp1dcptReleased then
  FDefaultOutcome := TsgcAMQP1FrameReleased.Create
else if oDescriptor.Code = amqp1dcptAccepted then
  FDefaultOutcome := TsgcAMQP1FrameAccepted.Create
else if oDescriptor.Code = amqp1dcptRejected then
  FDefaultOutcome := TsgcAMQP1FrameRejected.Create

Fix 3.4 – SetMessage Frees Old Message

procedure TsgcAMQP1Delivery.SetMessage(const aMessage: TsgcAMQP1Message);
begin
  if FFreeMessageOnDestroy and Assigned(F_Message) and (F_Message <> aMessage) then
    sgcFree(F_Message);
  FFreeMessageOnDestroy := False;
  F_Message := aMessage;
end;

4. Specification Compliance Fixes

These fixes correct deviations do AMQP 1.0 Transport, Types, and Messaging specifications.

4.1 Begin Frame Field 7 Read Into Wrong Property

File: sgcAMQP1_Frames.pas
Field index 7 do begin performative was being read into DesiredCapabilities instead of Properties. Per o spec, begin fields are: remote-channel(0), next-outgoing-id(1), incoming-window(2), outgoing-window(3), handle-max(4), offered-capabilities(5), desired-capabilities(6), properties(7).

4.2 Source e Target Missing Fields em DoWrite

File: sgcAMQP1_Frames.pas
The Source descriptor's DoWrite method was not serializing the default-outcome, outcomes, and capabilities fields. The Target descriptor was missing capabilities. This caused o broker to use defaults instead do negotiated values, potentially leading para incorrect delivery state handling.

4.3 AmqpSequence Read Into Wrong Property

File: sgcAMQP1_Message.pas
When reading um mensagem body, amqp-sequence data was being read into ApplicationData.AMQPValue instead of ApplicationData.AMQPSequence. This corrupted a mensagem body para any message using o amqp-sequence encoding.

4.4 TransactionalState Outcome Not Written

File: sgcAMQP1_Frames.pas
The transactional-state delivery state was not serializing its outcome field, which is required when settling deliveries within um transaction.

4.5 Disposition Last Field Cannot Distinguish Zero From Unset

Files: sgcAMQP1_Frames.pas, sgcAMQP1_Frames.intf, sgcAMQP1_Session.pas
The disposition performative has um optional last field (delivery-id). Since it is um Cardinal, o valor 0 is valid e cannot be used como um sentinel para “not set.” A new FLastAssigned boolean flag e SetLast setter were added to properly track whether o field was explicitly set. 

procedure TsgcAMQP1FrameDisposition.SetLast(const Value: Cardinal);
begin
  FLast := Value;
  FLastAssigned := True;
end;

4.6 AmqpSequence Missing Value Property e Read/Write

Files: sgcAMQP1_Frames.pas, sgcAMQP1_Frames.intf
The TsgcAMQP1FrameAmqpSequence class had no Value property e empty DoRead/DoWrite métodos. The amqp-sequence body type was completely non-functional.

4.7 Error Info Field Read como String Instead de Map

File: sgcAMQP1_Frames.pas
The AMQP 1.0 spec defines o info field do error type como a map. It was being read with ReadString instead of ReadMap, causing parsing failures when brokers send structured error information.

4.8 Capabilities e Locales Written como String Instead de Symbol

File: sgcAMQP1_Frames.pas
The AMQP 1.0 spec defines offered-capabilities, desired-capabilities, outgoing-locales, and incoming-locales como arrays of symbol. They were being written with WriteString instead of WriteSymbol no open, begin, and attach performatives. A standards-compliant broker would reject these frames como having incorrect field types.

4.9 Missing Accepted Descriptor em DefaultOutcome Handler

File: sgcAMQP1_Frames.pas
The Source descriptor's default-outcome reader somente handled released and rejected. The most common outcome, accepted, was not handled. When um broker sent accepted como o padrão outcome, it was silently ignored.

5. State Machine & Conexão Fixes

These fixes address o AMQP 1.0 conexão state machine e frame processing logic.

Fix File Description
5.1 sgcAMQP1.pas The amqp1csOpenReceived state transition was missing o else DoRaiseInvalidState that all other states had. Invalid transitions were silently ignored instead de raising um erro.
5.2 sgcAMQP1.pas Frame size validation mensagem de erro displayed RemoteMaxFrameSize but should have shown LocalMaxFrameSize, since o local limit is what was being checked.
5.3 sgcAMQP1.pas FLastTimeRead was initialized para 0 (Delphi epoch: 1899-12-30) instead de Now. This caused immediate false idle timeout detection em startup.
5.4 sgcAMQP1.pas The TBytes overload de Read did not update FLastTimeRead := Now, unlike o TMemoryStream overload. This caused inconsistent heartbeat tracking.
5.5 sgcAMQP1.pas Header received state transition was conditional when it should sempre trigger. The state machine must transition em every valid header exchange per o AMQP 1.0 spec.

Fix 5.1 – OpenReceived Missing Error Branch

amqp1csOpenReceived:
  begin
    if aState = amqp1csOpenSent then
      FConnectionState := amqp1csOpened
    else
      DoRaiseInvalidState;  // Added: was missing
  end;

6. Heartbeat & Idle Timeout Fixes

The AMQP 1.0 specification requer que when um peer advertises an idle-timeout no open performative, o other peer must send heartbeat frames em half o advertised interval. These fixes ensure o heartbeat mechanism actually functions.

Fix File Description
6.1 sgcAMQP1_Client.pas HeartBeat was nunca enabled. Both branches do idle timeout check set HeartBeat.Enabled := False. Changed para True when IdleTimeout > 0.
6.2 sgcAMQP1_Client.pas Disconnect did not disable heartbeat ou set FConnected := False early enough. Reordered para prevent heartbeat firing during teardown.
6.3 sgcAMQP1.pas FLastTimeRead not updated no TBytes Read overload (also listed em State Machine section).

Fix 6.1 – HeartBeat Enabled

if oOpen.IdleTimeout > 0 then
begin
  HeartBeat.Interval := Trunc(oOpen.IdleTimeout / 2);
  HeartBeat.Enabled := True;   // Was: False (heartbeat never started)
end
else
  HeartBeat.Enabled := False;

Fix 6.2 – Desconectar Reordered

procedure TsgcAMQP1_Client.Disconnect;
begin
  FConnected := False;           // Moved first: prevents heartbeat race
  DoStopIdleTimeout;
  HeartBeat.Enabled := False;    // Added: stop heartbeat during teardown
  Clear;
  DoConnectionState(amqp1csEnd);
end;

7. Thread Safety Fixes

These fixes address race conditions em concurrent access para shared data structures.

7.1 TsgcAMQP1Deliveries.Primeiro() Bounds Check e Locking

File: sgcAMQP1_Message.pas
The First() method accessed Items[0] sem checking if um lista was empty and sem acquiring o thread-safe lock. In um multi-threaded environment, another thread could remover all items entre o count check e o access, causing um index-out-of-bounds exception.

After (Fixed)

function TsgcAMQP1Deliveries.First: TsgcAMQP1Delivery;
var
  oList: TList;
begin
  result := nil;
  oList := LockList;
  Try
    if oList.Count > 0 then
      result := TsgcAMQP1Delivery(oList[0]);
  Finally
    UnLockList;
  End;
end;

7.2 SetMessage Safe Object Replacement

File: sgcAMQP1_Message.pas
The SetMessage method now checks that o novo message is different do current one before freeing, preventing use-after-free when assigning o same message object.

8. Files Modified

File Fixes Categories
Source\sgcAMQP1_Classes.pas 2 Critical bugs
Source\sgcAMQP1_Frames.pas 16 Critical bugs, memory leaks, spec compliance
Interfaces\sgcAMQP1_Frames.intf 2 Spec compliance (Disposition LastAssigned, AmqpSequence Value)
Source\sgcAMQP1_Message.pas 4 Critical bugs, memory leaks, thread safety
Source\sgcAMQP1_Session.pas 3 Memory leaks, spec compliance
Source\sgcAMQP1.pas 5 State machine, conexão, heartbeat
Source\sgcAMQP1_Client.pas 3 Heartbeat, desconectar safety

Total: 30 fixes across 8 files, improving protocol correctness, memory safety, e reliability do AMQP 1.0 implementation against o OASIS specification.