Mise à jour du client Delphi AMQP 1

· Fonctionnalités

L'implémentation du protocole AMQP 1.0 dans sgcWebSockets a fait l'objet d'une revue complète par rapport à la spécification OASIS AMQP 1.0. Cet article documente les 30 corrections appliquées dans 8 fichiers source, couvrant des bugs critiques, des fuites mémoire, la conformité à la spécification, la justesse de la machine à états, la gestion du heartbeat et des améliorations de la sécurité des threads.

Table des matières

  1. Aperçu
  2. Corrections de bugs critiques
  3. Corrections de fuites mémoire
  4. Corrections de conformité à la spécification
  5. Corrections de la machine à états et de la connexion
  6. Corrections du heartbeat et du timeout d'inactivité
  7. Corrections de la sécurité des threads
  8. Fichiers modifiés

1. Aperçu

Au total, 30 corrections ont été appliquées dans 6 fichiers source et 2 fichiers d'interface de l'implémentation AMQP 1.0. Les corrections sont catégorisées comme suit :

Catégorie Nombre Sévérité
Corrections de bugs critiques 6 Critique
Corrections de fuites mémoire 4 Critical
Conformité à la spécification 10 Haut
Machine à états et connexion 5 High
Heartbeat et timeout d'inactivité 3 High
Sécurité des threads 2 High

2. Corrections de bugs critiques

Ces corrections traitent des bugs causant des échecs immédiats à l'exécution, une corruption de protocole ou un comportement incorrect pendant la communication AMQP 1.0.

2.1 Mot-clé raise manquant dans une exception

Fichier : sgcAMQP1_Classes.pas
Un objet d'exception était créé mais jamais levé lorsqu'un type de trame invalide était rencontré. L'exception était silencieusement ignorée, permettant le traitement de trames corrompues sans détection.

Avant (cassé)

TsgcAMQP1Exception.CreateFmt(S_AMQP1_INVALID_FRAME_TYPE, [vByte]);

Après (corrigé)

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

2.2 WriteMap Map32 avec données manquantes et taille incorrecte

Fichier : sgcAMQP1_Classes.pas
Le chemin d'encodage Map32 omettait l'appel à WriteBytes pour les données réelles de la map, et le champ size utilisait un offset incorrect. Map32 utilise un champ de comptage de 4 octets (contrairement à Map8 qui utilise 1 octet), donc la taille doit inclure 4 octets supplémentaires.

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 Logique ContainsError inversée

Fichier : sgcAMQP1_Frames.pas
La méthode ContainsError dans TsgcAMQP1FrameRejected et TsgcAMQP1DescribedListError renvoyait True quand il n'y avait pas d'erreur et False quand il y avait une erreur. Cela provoquait la perte silencieuse des informations d'erreur et l'écriture d'octets nuls alors que les erreurs réelles auraient dû être sérialisées. Les branches DoWrite et DoWriteError ont également été inversées pour correspondre à la logique corrigée.

Before (Broken)

function TsgcAMQP1FrameRejected.ContainsError: Boolean;
begin
  if not Assigned(FError) then
    Result := True              // Faux : True quand pas d'erreur
  else
    Result := (Error.Condition = '') and (Error.Description = '') and
      (Error.Info = '');     // Faux : True quand tous vides
end;

After (Fixed)

function TsgcAMQP1FrameRejected.ContainsError: Boolean;
begin
  if not Assigned(FError) then
    Result := False             // Correct : False quand pas d'erreur
  else
    Result := (Error.Condition <> '') or (Error.Description <> '') or
      (Error.Info <> '');    // Correct : True quand un champ est défini
end;

2.4 Séparateurs à octets nuls de SASL PLAIN

Fichier : sgcAMQP1_Frames.pas
Le mécanisme SASL PLAIN requiert le format \0username\0password avec des séparateurs ($00) à octets nuls. L'implémentation n'initialisait pas le tableau d'octets à zéro, donc les positions des séparateurs contenaient des données aléatoires. L'authentification échouait contre tout broker AMQP 1.0 conforme aux standards.

After (Fixed)

SetLength(FInitialResponse, 2 + Length(oUser) + Length(oPassword));
FillChar(FInitialResponse[0], Length(FInitialResponse), 0);  // Remplir de zéros pour les séparateurs nuls

2.5 inherited Create manquant dans TsgcAMQP1Message

Fichier : sgcAMQP1_Message.pas
Le constructeur paramétré de TsgcAMQP1Message n'appelait pas inherited Create, ce qui signifie que la classe de base n'était jamais initialisée. Cela provoquait des violations d'accès ou un état corrompu lors de l'utilisation du constructeur de commodité.

After (Fixed)

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

2.6 Point-virgule manquant dans AmqpValue.DoRead

Fichier : sgcAMQP1_Frames.pas
Un point-virgule manquant dans TsgcAMQP1FrameAmqpValue.DoRead empêchait la compilation.

3. Corrections de fuites mémoire

Ces corrections traitent des problèmes de gestion de durée de vie d'objets qui faisaient s'accumuler la mémoire au fil du temps, en particulier pendant des connexions AMQP 1.0 longues avec de nombreux échanges de messages.

Correction Fichier Description
3.1 sgcAMQP1_Frames.pas FDefaultOutcome n'était pas libéré avant réaffectation lors de la lecture du descripteur Source. À chaque réception d'un nouveau default outcome, l'objet précédent était fuité.
3.2 sgcAMQP1_Session.pas Appel dupliqué à sgcFree(FCreditConsumed) dans le destructeur provoquant un potentiel double-free. Ligne dupliquée supprimée.
3.3 sgcAMQP1_Session.pas FOutgoingDeliveries manquait dans le destructeur de session. La liste de suivi des livraisons n'était jamais libérée à la destruction d'une session.
3.4 sgcAMQP1_Message.pas SetMessage et SetMessageAndFreeOnDestroy ne libéraient pas le message précédent quand FFreeMessageOnDestroy était activé. Les affectations répétées de message faisaient fuir la mémoire.

Correction 3.1 – FDefaultOutcome libéré avant réaffectation

sgcFree(FDefaultOutcome);  // Libérer l'instance précédente avant réaffectation
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

Correction 3.4 – SetMessage libère l'ancien 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. Corrections de conformité à la spécification

Ces corrections corrigent des écarts par rapport aux spécifications AMQP 1.0 Transport, Types et Messaging.

4.1 Champ 7 de la trame Begin lu dans la mauvaise propriété

Fichier : sgcAMQP1_Frames.pas
Le champ d'index 7 de la performative begin était lu dans DesiredCapabilities au lieu de Properties. Selon la spécification, les champs de begin sont : 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 Champs manquants pour Source et Target dans DoWrite

Fichier : sgcAMQP1_Frames.pas
La méthode DoWrite du descripteur Source ne sérialisait pas les champs default-outcome, outcomes et capabilities. Le descripteur Target omettait capabilities. Cela faisait que le broker utilisait les valeurs par défaut au lieu des valeurs négociées, pouvant entraîner une gestion incorrecte de l'état des livraisons.

4.3 AmqpSequence lu dans la mauvaise propriété

Fichier : sgcAMQP1_Message.pas
Lors de la lecture du corps du message, les données amqp-sequence étaient lues dans ApplicationData.AMQPValue au lieu de ApplicationData.AMQPSequence. Cela corrompait le corps du message pour tout message utilisant l'encodage amqp-sequence.

4.4 Outcome de TransactionalState non écrit

Fichier : sgcAMQP1_Frames.pas
L'état de livraison transactional-state ne sérialisait pas son champ outcome, requis lors du règlement des livraisons dans une transaction.

4.5 Le champ Last de Disposition ne distingue pas zéro de non défini

Fichiers : sgcAMQP1_Frames.pas, sgcAMQP1_Frames.intf, sgcAMQP1_Session.pas
La performative disposition a un champ optionnel last (delivery-id). Comme c'est un Cardinal, la valeur 0 est valide et ne peut pas être utilisée comme sentinelle pour « non défini ». Un nouveau drapeau booléen FLastAssigned et un setter SetLast ont été ajoutés pour suivre correctement si le champ a été explicitement défini. 

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

4.6 AmqpSequence sans propriété Value ni Read/Write

Fichiers : sgcAMQP1_Frames.pas, sgcAMQP1_Frames.intf
La classe TsgcAMQP1FrameAmqpSequence n'avait pas de propriété Value et avait des méthodes DoRead/DoWrite vides. Le type de corps amqp-sequence était totalement non fonctionnel.

4.7 Le champ Info de l'erreur est lu comme String au lieu de Map

Fichier : sgcAMQP1_Frames.pas
La spécification AMQP 1.0 définit le champ info du type error comme une map. Il était lu avec ReadString au lieu de ReadMap, causant des échecs de parsing quand les brokers envoyaient des informations d'erreur structurées.

4.8 Capabilities et Locales écrits comme String au lieu de Symbol

Fichier : sgcAMQP1_Frames.pas
La spécification AMQP 1.0 définit offered-capabilities, desired-capabilities, outgoing-locales et incoming-locales comme des tableaux de symbol. Ils étaient écrits avec WriteString au lieu de WriteSymbol dans les performatives open, begin et attach. Un broker conforme aux standards rejetterait ces trames car les types des champs sont incorrects.

4.9 Descripteur Accepted manquant dans le handler DefaultOutcome

Fichier : sgcAMQP1_Frames.pas
Le lecteur de default-outcome du descripteur Source ne gérait que released et rejected. L'outcome le plus courant, accepted, n'était pas géré. Quand un broker envoyait accepted comme default outcome, il était silencieusement ignoré.

5. Corrections de la machine à états et de la connexion

Ces corrections traitent de la machine à états de la connexion AMQP 1.0 et de la logique de traitement des trames.

Fix File Description
5.1 sgcAMQP1.pas La transition d'état amqp1csOpenReceived n'avait pas le else DoRaiseInvalidState que tous les autres états possédaient. Les transitions invalides étaient silencieusement ignorées au lieu de lever une erreur.
5.2 sgcAMQP1.pas Le message d'erreur de validation de taille de trame affichait RemoteMaxFrameSize mais aurait dû afficher LocalMaxFrameSize, puisque c'est la limite locale qui était vérifiée.
5.3 sgcAMQP1.pas FLastTimeRead était initialisé à 0 (époque Delphi : 1899-12-30) au lieu de Now. Cela provoquait une fausse détection immédiate du timeout d'inactivité au démarrage.
5.4 sgcAMQP1.pas La surcharge TBytes de Read ne mettait pas à jour FLastTimeRead := Now, contrairement à la surcharge TMemoryStream. Cela provoquait un suivi incohérent du heartbeat.
5.5 sgcAMQP1.pas La transition d'état Header reçu était conditionnelle alors qu'elle devrait toujours se déclencher. La machine à états doit transiter à chaque échange d'en-tête valide selon la spécification AMQP 1.0.

Correction 5.1 – Branche d'erreur manquante pour OpenReceived

amqp1csOpenReceived:
  begin
    if aState = amqp1csOpenSent then
      FConnectionState := amqp1csOpened
    else
      DoRaiseInvalidState;  // Ajouté : était manquant
  end;

6. Corrections du heartbeat et du timeout d'inactivité

La spécification AMQP 1.0 exige que lorsqu'un peer annonce un idle-timeout dans la performative open, l'autre peer doit envoyer des trames heartbeat à la moitié de l'intervalle annoncé. Ces corrections garantissent que le mécanisme de heartbeat fonctionne réellement.

Fix File Description
6.1 sgcAMQP1_Client.pas HeartBeat n'était jamais activé. Les deux branches de la vérification du timeout d'inactivité positionnaient HeartBeat.Enabled := False. Changé en True quand IdleTimeout > 0.
6.2 sgcAMQP1_Client.pas Disconnect ne désactivait pas le heartbeat et ne positionnait pas FConnected := False assez tôt. Réordonné pour empêcher le heartbeat de se déclencher pendant la fermeture.
6.3 sgcAMQP1.pas FLastTimeRead non mis à jour dans la surcharge Read sur TBytes (également mentionné dans la section Machine à états).

Correction 6.1 – HeartBeat activé

if oOpen.IdleTimeout > 0 then
begin
  HeartBeat.Interval := Trunc(oOpen.IdleTimeout / 2);
  HeartBeat.Enabled := True;   // Avant : False (heartbeat jamais démarré)
end
else
  HeartBeat.Enabled := False;

Correction 6.2 – Disconnect réordonné

procedure TsgcAMQP1_Client.Disconnect;
begin
  FConnected := False;           // Déplacé en premier : évite la condition de course du heartbeat
  DoStopIdleTimeout;
  HeartBeat.Enabled := False;    // Ajouté : arrête le heartbeat pendant la fermeture
  Clear;
  DoConnectionState(amqp1csEnd);
end;

7. Corrections de la sécurité des threads

Ces corrections traitent des conditions de course dans l'accès concurrent à des structures de données partagées.

7.1 TsgcAMQP1Deliveries.First() : vérification des bornes et verrouillage

Fichier : sgcAMQP1_Message.pas
La méthode First() accédait à Items[0] sans vérifier si la liste était vide et sans acquérir le verrou thread-safe. Dans un environnement multi-thread, un autre thread pouvait supprimer tous les éléments entre la vérification du count et l'accès, causant une exception d'index hors bornes.

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 Remplacement sûr d'objet par SetMessage

Fichier : sgcAMQP1_Message.pas
La méthode SetMessage vérifie maintenant que le nouveau message est différent de l'actuel avant de le libérer, prévenant un use-after-free lors de l'affectation du même objet message.

8. Fichiers modifiés

File Corrections Catégories
Source\sgcAMQP1_Classes.pas 2 Bugs critiques
Source\sgcAMQP1_Frames.pas 16 Bugs critiques, fuites mémoire, conformité à la spécification
Interfaces\sgcAMQP1_Frames.intf 2 Conformité à la spécification (Disposition LastAssigned, AmqpSequence Value)
Source\sgcAMQP1_Message.pas 4 Bugs critiques, fuites mémoire, sécurité des threads
Source\sgcAMQP1_Session.pas 3 Fuites mémoire, conformité à la spécification
Source\sgcAMQP1.pas 5 Machine à états, connexion, heartbeat
Source\sgcAMQP1_Client.pas 3 Heartbeat, sûreté du disconnect

Total : 30 corrections dans 8 fichiers, améliorant la justesse du protocole, la sécurité mémoire et la fiabilité de l'implémentation AMQP 1.0 par rapport à la spécification OASIS.