Don't accept invalid KNS EntryInternal results
ClosedPublic

Authored by leinir on Tue, Feb 5, 1:53 PM.

Details

Summary

In certain cases we might end up with invalid results being returned by a KNS source. These entries should be considered undesirable, and consequently filtered out.

Diff Detail

Repository
R134 Discover Software Store
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
leinir created this revision.Tue, Feb 5, 1:53 PM
Restricted Application added a project: Plasma. · View Herald TranscriptTue, Feb 5, 1:53 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
leinir requested review of this revision.Tue, Feb 5, 1:53 PM
apol accepted this revision.Tue, Feb 5, 4:25 PM
This revision is now accepted and ready to land.Tue, Feb 5, 4:25 PM
apol requested changes to this revision.Tue, Feb 5, 4:26 PM
apol added inline comments.
libdiscover/backends/KNSBackend/KNSBackend.cpp
213

first arg should be filtered.

This revision now requires changes to proceed.Tue, Feb 5, 4:26 PM
leinir marked an inline comment as done.Tue, Feb 5, 4:36 PM
leinir added inline comments.
libdiscover/backends/KNSBackend/KNSBackend.cpp
213

Oh dear! Yes, it should.

leinir updated this revision to Diff 50965.Tue, Feb 5, 4:37 PM
leinir marked an inline comment as done.

Actually use the filtered list of entries

apol accepted this revision.Tue, Feb 5, 4:37 PM

Good, now I just wonder why KNS would give us poised entries...

This revision is now accepted and ready to land.Tue, Feb 5, 4:37 PM
leinir added a comment.Tue, Feb 5, 4:41 PM
In D18760#405894, @apol wrote:

Good, now I just wonder why KNS would give us poised entries...

Indeed, that would want to be looked at. Essentially, it just forwards the result of the list job from Attica, which in the case of empty results are just... also empty EntryInternal instances. Basically, we just never had a server which did things like that before, so we're now finding some slightly odd bugs that've always been there, caused by little assumptions about the way the server works ;) Not entirely sure if the fix wants to be in Attica or in KNewStuff... Probably Attica, though.

This revision was automatically updated to reflect the committed changes.

Is this a good candidate for going into 5.14 too? Or even 5.12?

This comment was removed by ngraham.
apol added a comment.Tue, Feb 5, 8:20 PM

I'd actually like to be sure that it's fixing any issues before backporting to 5.12.