ItemListJobImpl silently throws away items that fail client side validation
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
storage-framework (Ubuntu) |
Fix Released
|
Critical
|
Michi Henning |
Bug Description
I spent ages trying to work out why my roots() method was appearing to succeed but never emitting the itemsReceived() signal on the ItemJob.
Inside the class, you have:
try
{
}
catch (StorageError const& exc)
{
// Bad metadata received from provider, validate_() or make_item() have logged it.
}
So items that fail the validation silently disappear with no indication that there is a problem. Unlike what the comment says, there was no logging and the ItemJob didn't transition to the Error state. This is not okay.
And later on, it seems the itemsReceived signal is only conditionally emitted:
if (!items.isEmpty())
{
Q_EMIT public_
}
Would it be a problem to always emit this signal? Having it conditional like this made me chase my tail trying to work out if there was a problem with the way I'd connected to the signal, or if it might be getting emitted after statusChanged.
Related branches
- unity-api-1-bot: Approve (continuous-integration)
- James Henstridge: Approve
-
Diff: 537 lines (+161/-118)6 files modifiedsrc/qt/internal/AccountImpl.cpp (+2/-1)
src/qt/internal/ItemImpl.cpp (+10/-8)
src/qt/internal/ItemListJobImpl.cpp (+4/-6)
src/qt/internal/MultiItemListJobImpl.cpp (+1/-4)
src/qt/internal/validate.cpp (+75/-63)
tests/remote-client/remote-client_test.cpp (+69/-36)
Changed in storage-framework (Ubuntu): | |
importance: | Undecided → Critical |
assignee: | nobody → Michi Henning (michihenning) |
status: | New → In Progress |
Changed in storage-framework (Ubuntu): | |
status: | In Progress → Fix Committed |
Changed in storage-framework (Ubuntu): | |
status: | Fix Committed → Fix Released |