Contact support

We reply by e-mail, usually within two days.

Google reCAPTCHA checks this submission against abuse; data is sent to Google. The script loads only once you open this form.

← All posts

The check that is always true

· Lukas Wojcik

There are 23 UPDATE statements in this service. Exactly one of them checks whether it changed anything. That check is:

'success' => $n->rowCount() >= 0

rowCount() returns 0 when no row matched. Zero is greater than or equal to zero. The check passes whatever happens.

23 UPDATE statements, one of them looks at the result rowCount() >= 0 — passes even when nothing matched rowCount() > 0 anywhere in the codebase: 0 times counters with a row in the table it updates: 4 of 2,218 nothing is broken today — for a reason that is not this line

What the statement is for

It saves a checkbox: whether the owner of a counter wants a weekly summary by e-mail. The address lives in a separate table, and a row appears there only after somebody has clicked the confirmation link in a mail they asked for. Four counters have such a row. There are 2,218 active ones.

So for 99.8% of counters the UPDATE matches nothing. The endpoint answers success: true and the setting is not saved, because there is nothing to save it in.

The comment above it is correct

Three lines higher, in the same function:

"Only where a confirmed return path exists. Without one there is no address the letter could go to — and the checkbox would have promised something that does not happen."

That is exactly right. Somebody thought about this case, understood it, and wrote down the reasoning. Then the line underneath used >= where the reasoning called for >.

This is worth looking at squarely, because the usual explanation — nobody thought about it — is available and wrong. The thinking is in the file. What failed is one character, in a place where the wrong version and the right version look identical at a glance and behave identically in every test that has a row to update.

Nobody is being lied to

Here is the part that would be easy to leave out, and leaving it out would make this post more dramatic and less true.

The page does not render that checkbox unless the row exists. One screen away, in the code that draws the owner's settings box, the same table is queried first, and the whole block is skipped when the query comes back empty. So a counter owner without a confirmed address never sees the control, never clicks it, and never gets the false confirmation.

The broken check is reachable only by calling the endpoint directly with a valid token. Somebody doing that gets success: true and no saved setting. That is a real defect and a narrow one.

Why it is still worth writing down

The feature is safe because of a guard nobody wrote down as the guard. The line that exists to make it safe does not make it safe. The thing that does is a rendering condition in a different function, whose comment explains why the checkbox is hidden — not that anything depends on it staying hidden.

Remove or restructure that rendering condition — a reasonable thing to do while redesigning a settings panel — and the defect becomes visible instantly, with nothing anywhere connecting the two changes. The safety is real, and it is accidental, and accidental safety is the kind that disappears during unrelated work.

The same codebase does it correctly one file over. The webring's equivalent function asks whether the row exists, returns false when it does not, and never issues the update at all. That table currently has zero rows, so that function returns false every time it is called, which is the correct answer.

The general shape

An UPDATE that matches no rows is not an error in any database. It is a successful statement that did nothing, and every layer above it will report success unless something asks. Twenty-two of the statements here do not ask, and for most of them that is fine: they update a row that the request already proved exists.

The one that needed to ask, asked in a way that cannot fail. rowCount() >= 0 is not a weak check, it is the absence of a check wearing the costume of one — and it is worse than no check, because the next person to read the function sees a result being inspected and stops looking.

The phrase rowCount() > 0 appears zero times in this codebase. That is the number that turned a one-character typo into something worth a post: not that it was written wrong once, but that there was no correct instance anywhere to compare it against.

Fixed on 28 August 2026. The endpoint now asks whether the row exists before updating, the way the webring function one file over already did, and answers false when it does not. The obvious one-character fix — comparing against greater-than-zero instead — was measured first and rejected: the driver reports changed rows rather than matched rows, so setting the checkbox to the value it already holds changes nothing, and that version would have reported failure for an operation that was fine. Both versions are wrong, in opposite directions.

Advertisement