Skip to content

Item::updateExpirationDate() emits malformed SQL and is silently a no-op #509

Description

@navjottomer

Severity: High — dormant unless a category has a non-zero expiration window (i_expiration_days > 0). An install whose categories are all "never expire" (0) never reaches the broken branch, which is why it can sit unnoticed.

Symptom

Item::newInstance()->updateExpirationDate($id, $days, true) with a positive $days returns false and does not change dt_expiration. The row keeps whatever expiration it already had.

Root cause

oc-includes/osclass/classes/model/Item.php:862-866 calls the low-level writer with a raw SET string and a positional WHERE array:

$this->dao->update(
    $this->getTableName(),
    sprintf('dt_expiration = %s', $dt_expiration),   // a raw string
    ['pk_i_id', $id]                                 // positional
);

Neither shape matches DBCommandClass:

  • DBCommandClass::set() wraps a non-array key as [$string => ''], and _update() renders each pair as "$k = $v" — so the SET clause becomes dt_expiration = DATE_ADD(...) = ''.
  • where(['pk_i_id', $id]) is iterated as index⇒value, becoming 0 = 'pk_i_id' AND 1 = <id>.

The statement that actually reaches MySQL:

UPDATE oc_t_item
SET dt_expiration = DATE_ADD(oc_t_item.dt_pub_date, INTERVAL 30 DAY) = ''
WHERE 0 = 'pk_i_id' AND 1 = 103

SET col = (expr = '') assigns a boolean/NULL to a DATETIME; under strict mode the write is rejected and update() returns false.

Impact

Any code that sets a listing's expiry through this method — including core's own ItemActions::add()/edit() when a category carries a real expiration window — fails to write the expiration. Item posting appears to succeed; the expiry is just never applied.

Suggested fix

Build the SET/WHERE the way the rest of the model does — pass an associative array of columns and a proper where clause — or add a first-class "expiry = pub + N days" writer rather than hand-assembling SQL fragments.


Found 2026-07-17 while building a downstream theme (internal ref: CORE-1). A theme-side renewal path currently routes around this by writing dt_expiration directly via Item::update().

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugimportantphpPull requests that update Php code

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions