124 lines
4.4 KiB
Markdown
124 lines
4.4 KiB
Markdown
---
|
|
id: KB-PLUGIN-014
|
|
title: getFromDBByCrit() não limpa $fields em falha — reutilizar a mesma instância vaza estado entre iterações
|
|
domain: plugin-dev
|
|
tags:
|
|
- glpi
|
|
- glpi11
|
|
- commondbtm
|
|
- getfromdbbycrit
|
|
- bug
|
|
- iteration
|
|
- state-leak
|
|
status: active
|
|
severity: critical
|
|
created_at: 2026-05-10
|
|
updated_at: 2026-05-10
|
|
applies_to:
|
|
- GLPI 11.0.x
|
|
---
|
|
|
|
# `getFromDBByCrit()` vaza `$fields` entre iterações quando reutilizado
|
|
|
|
## Sintoma
|
|
|
|
Loop que itera sobre uma lista de itens (ex: plugins, entidades, qualquer `CommonDBTM`) reutilizando a mesma instância da classe via `getFromDBByCrit()` — itens que **não existem** no banco aparecem misteriosamente com dados do **item anterior** que foi encontrado.
|
|
|
|
No caso real do Mindplace: dois plugins (`splititil` e `tilesections`). `splititil` estava instalado, `tilesections` não. Ao renderizar a lista, `tilesections` apareceu com o **estado e ID do `splititil`** — clicar "Habilitar" no `splititil` parecia habilitar o `tilesections` (porque o render reaproveitava os fields).
|
|
|
|
## Causa
|
|
|
|
`CommonDBTM::getFromDBByCrit()` retorna `false` quando não encontra o item, **mas não limpa `$this->fields`**. A próxima chamada a `$obj->isNewItem()` ou acesso a `$obj->fields['...']` retorna o estado da **iteração anterior**.
|
|
|
|
```php
|
|
// CommonDBTM::getFromDBByCrit (resumido)
|
|
public function getFromDBByCrit(array $crit) {
|
|
$iter = $DB->request([
|
|
'FROM' => $this::getTable(),
|
|
'WHERE' => $crit,
|
|
]);
|
|
if (count($iter) == 1) {
|
|
$row = $iter->current();
|
|
return $this->getFromDB($row['id']); // sucesso: carrega fields
|
|
}
|
|
return false; // ← FALHA: fields NÃO são limpos!
|
|
}
|
|
```
|
|
|
|
## Código defeituoso
|
|
|
|
```php
|
|
// ❌ ERRADO — vaza estado entre iterações
|
|
$plugin_obj = new Plugin();
|
|
foreach ($plugins as &$p) {
|
|
$plugin_obj->getFromDBByCrit(['directory' => $p['key']]);
|
|
if ($plugin_obj->isNewItem()) {
|
|
// tilesections (não instalado) entra aqui só na PRIMEIRA iteração
|
|
// se for o primeiro a falhar. Senão, herda fields do anterior.
|
|
$p['state'] = Plugin::NOTINSTALLED;
|
|
} else {
|
|
$p['state'] = (int) $plugin_obj->fields['state']; // ← fields do plugin ERRADO
|
|
}
|
|
}
|
|
```
|
|
|
|
## Padrões corretos
|
|
|
|
### Opção 1 — Nova instância por iteração (mais defensivo)
|
|
|
|
```php
|
|
foreach ($plugins as &$p) {
|
|
$plugin_obj = new Plugin(); // ← fresh instance, fields zerados
|
|
$found = $plugin_obj->getFromDBByCrit(['directory' => $p['key']]);
|
|
if (!$found) {
|
|
$p['state'] = Plugin::NOTINSTALLED;
|
|
continue;
|
|
}
|
|
$p['state'] = (int) $plugin_obj->fields['state'];
|
|
}
|
|
```
|
|
|
|
### Opção 2 — Usar o valor de retorno (mais idiomático)
|
|
|
|
```php
|
|
$plugin_obj = new Plugin();
|
|
foreach ($plugins as &$p) {
|
|
if (!$plugin_obj->getFromDBByCrit(['directory' => $p['key']])) {
|
|
$p['state'] = Plugin::NOTINSTALLED;
|
|
continue;
|
|
}
|
|
$p['state'] = (int) $plugin_obj->fields['state'];
|
|
}
|
|
```
|
|
|
|
A Opção 2 é mais idiomática, mas **só funciona** se você nunca acessar `$plugin_obj->fields` no branch de falha. Se houver qualquer chance de o código acessar `$plugin_obj` depois de uma falha, prefira a Opção 1.
|
|
|
|
## Como nunca usar `isNewItem()` após `getFromDBByCrit()`
|
|
|
|
`isNewItem()` retorna `$this->fields['id'] <= 0`. Como `getFromDBByCrit()` não limpa `fields`, **`isNewItem()` mente** após um miss. Use sempre o valor de retorno do próprio `getFromDBByCrit()`:
|
|
|
|
```php
|
|
// ❌
|
|
$obj->getFromDBByCrit($crit);
|
|
if ($obj->isNewItem()) { ... } // não confiável!
|
|
|
|
// ✅
|
|
if (!$obj->getFromDBByCrit($crit)) { ... }
|
|
```
|
|
|
|
## Comportamento de métodos relacionados
|
|
|
|
| Método | Limpa `$fields` em falha? |
|
|
|---|---|
|
|
| `getFromDB($id)` | ✅ sim — `fields = []` |
|
|
| `getFromDBByCrit($crit)` | ❌ não |
|
|
| `getFromDBByQuery($query)` | ❌ não (em geral) |
|
|
| `find($criteria)` | n/a — retorna array, não muda estado |
|
|
|
|
Quando em dúvida, **sempre instancie um objeto novo** por iteração. O overhead é desprezível e elimina toda uma classe de bugs sutis.
|
|
|
|
## Onde isso pegou na vida real
|
|
|
|
`docker/glpi/plugins/mindplace/src/MarketplaceView.php::showPage()` — bug surgiu quando o catálogo do Mindplace passou a ter mais de um plugin. O segundo plugin (não instalado) "herdava" o estado do primeiro (instalado), causando comportamento inexplicável: clicar Habilitar num plugin parecia ativar outro.
|
|
|
|
Corrigido em `mindplace v1.0.7` instanciando `new Plugin()` por iteração.
|