Differential D18760

Don't accept invalid KNS EntryInternal results
ClosedPublic
Actions

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

Details

Reviewers
apol
ngraham
Group Reviewers
Discover Software Store
Commits
R134:400b0d7e7596: Don't accept invalid KNS EntryInternal results
R134:b11cdd776c93: Don't accept invalid KNS EntryInternal results
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.Feb 5 2019, 1:53 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 5 2019, 1:53 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
leinir requested review of this revision.Feb 5 2019, 1:53 PM
apol accepted this revision.Feb 5 2019, 4:25 PM
This revision is now accepted and ready to land.Feb 5 2019, 4:25 PM
apol requested changes to this revision.Feb 5 2019, 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.Feb 5 2019, 4:26 PM
leinir marked an inline comment as done.Feb 5 2019, 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.Feb 5 2019, 4:37 PM
leinir marked an inline comment as done.

Actually use the filtered list of entries

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

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

This revision is now accepted and ready to land.Feb 5 2019, 4:37 PM
leinir added a comment.Feb 5 2019, 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.

Closed by commit R134:b11cdd776c93: Don't accept invalid KNS EntryInternal results (authored by leinir). · Explain WhyFeb 5 2019, 4:43 PM
This revision was automatically updated to reflect the committed changes.
ngraham added a comment.Feb 5 2019, 5:19 PM

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

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

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

Revision Contents
Changeset List

Diff 50966

View Options

libdiscover/backends/KNSBackend/KNSBackend.cpp

Loading...