| tsimonq2 | Hey everyone, so on https://dev.launchpad.net/PatchSubmission it says I should talk to someone here before making a change. | 03:54 |
|---|---|---|
| wgrant | tsimonq2: We're all in transit atm, but some time next week during EU day should work. | 03:55 |
| tsimonq2 | Was talking with the Security Team a few days ago and someone brought up bug 439470. Mind if I fix it? :P | 03:55 |
| mup | Bug #439470: Cannot attach currently-unknown CVEs via linkCVE() <api> <lp-bugs> <platform-want> <Launchpad itself:Triaged> <https://launchpad.net/bugs/439470> | 03:55 |
| tsimonq2 | wgrant: Ok | 03:55 |
| tsimonq2 | wgrant: I can link the bug again next week, unless people idle here and can see it then? | 03:56 |
| tsimonq2 | wgrant: Regardless, I have some preliminary code that I think works, I didn't fully read that first bullet point before working on it :P | 04:02 |
| tsimonq2 | Like you said, I'll ping sometime next week | 04:02 |
| cjwatson | tsimonq2: How were you planning to do it? The API takes a CVE reference today. | 04:02 |
| cjwatson | I mean, an object rather than a string. | 04:02 |
| tsimonq2 | cjwatson: So I played with it a bit, the best way I found was to edit lib/lp/bugs/model/cve.py and move the "add to db" logic from inText() to __getitem__() | 04:05 |
| tsimonq2 | cjwatson: That way, regardless on how you want the CVE to appear (either via bug commit or via the Launchpad interface), if the CVE is valid and exists in Mitre, add it to the database. | 04:06 |
| tsimonq2 | cjwatson: That (add to db if it's valid) was only done in inText() | 04:06 |
| cjwatson | Maybe. I'd be interested to see what test suite fallout exists from that, since that might indicate difficulties with that approach. | 04:06 |
| tsimonq2 | Sure, I'll play with that aspect of it a bit. | 04:07 |
| cjwatson | I agree that's one possibility, but it is pretty implicit. | 04:07 |
| tsimonq2 | cjwatson: Here's a diff: http://paste.ubuntu.com/25651103/ | 04:07 |
| cjwatson | I'm not going to think about diffs now | 04:07 |
| tsimonq2 | I'll do a runthrough of the tests and see what it does. | 04:07 |
| tsimonq2 | Ok | 04:08 |
| cjwatson | Also check whether that actually helps you over the API. | 04:08 |
| cjwatson | I don't remember whether the collection interface goes through __getitem__. | 04:09 |
| tsimonq2 | I think it will, because afair linkCVE over the API doesn't add new (valid) entries to the database. | 04:09 |
| cjwatson | That doesn't have much to do with my question :) | 04:09 |
| tsimonq2 | That addresses your first statement, though. I'm not entirely sure what you're referring to when you say "collection interface." | 04:11 |
| tsimonq2 | I *thought* it called ICveSet directly, like this: getUtility(ICveSet)['1999-8979'] | 04:12 |
| tsimonq2 | That in turn executes the logic I put in. | 04:12 |
| cjwatson | Right, but API clients use a webservice collection which isn't exactly identical to that. | 04:12 |
| cjwatson | Implementing it in __getitem__ only helps API clients if lp.cves[...] will get marshalled into something that ends up as the equivalent of getUtility(ICveSet)[...]. It would be logical for it to do so, but I don't remember whether that's actually the case right now. | 04:13 |
| tsimonq2 | Ah, I see. | 04:15 |
| tsimonq2 | I'll test it out and see if doing that adds the CVE properly. | 04:16 |
| tsimonq2 | cjwatson: Am I missing something, or do I need to compile a custom launchpadlib to be able to test this out? | 04:16 |
| cjwatson | Why would you need to do that? | 04:17 |
| cjwatson | (No compilation involved, to start with ...) | 04:17 |
| tsimonq2 | Good question... I just know that as an easy way to reference the API (in an interactive Python session would be a good way to test it out, no?) | 04:18 |
| cjwatson | If you're using the default launchpad.dev hostname, then you can use "dev" rather than "production"; otherwise you can pass a root URL | 04:19 |
| cjwatson | You'll probably need to run with LP_DISABLE_SSL_CERTIFICATE_VALIDATION=1 set in the environment unless you go to lots of effort to set up a hostname that Python will be willing to validate, but apart from that it works fine | 04:20 |
| tsimonq2 | Ok | 04:20 |
| tsimonq2 | Good to know. | 04:20 |
| cjwatson | It seems weird to me from a REST perspective that this means merely doing a GET on /bugs/cve/<unknown-sequence> will create a CVE placeholder. | 04:23 |
| cjwatson | Also AFAICS there's no way to do that from launchpadlib today; it would need some kind of equivalent of launchpadlib.launchpad.BugSet et al to allow looking up strings in that collection. | 04:24 |
| cjwatson | I would take a different approach: add an exported method to CveSet called something like "ensure" which returns a Cve object, inserting a placeholder into the DB if necessary. | 04:25 |
| cjwatson | That would make lookups of even existing Cve rows more convenient for API clients immediately, and would save having to make client changes. | 04:26 |
| cjwatson | As well as being more compliant with usual HTTP semantics. | 04:26 |
| cjwatson | (because the named-operation call to ensure() would be a POST) | 04:26 |
| tsimonq2 | Alright. | 04:28 |
| cjwatson | This is all somewhat dozy post-midnight derivation-from-code rather than actually trying it, but I think it's correct. | 04:29 |
| tsimonq2 | Ok. | 04:30 |
| cjwatson | And at any rate implementing an exported factory operation is the natural and usual way to do it and will definitely work, as opposed to an unusual way to do it which at best only might work. :-) | 04:30 |
| cjwatson | (you'll probably want @export_factory_operation rather than @export_write_operation) | 04:31 |
| tsimonq2 | Sure, makes sense. :) | 04:31 |
| tsimonq2 | Alright. | 04:31 |
| cjwatson | Anyway, that's your pre-implementation review, so I'm going to sleep :) | 04:32 |
| tsimonq2 | Ok, wfm, thanks cjwatson. :) | 04:32 |
| tsimonq2 | cjwatson: So I'm looking over the code and your review a bit more, I split the database placeholder entry code into another function that's called by __getitem__, because the way inText works is that it adds a placeholder to the database if it's a valid CVE and if it doesn't already exist, but since the problem is that the browser implementation only runs through __getitem__, we need it there as | 18:33 |
| tsimonq2 | well, no? | 18:33 |
| tsimonq2 | cjwatson: So the way I see it, it seems we can just have the function be called by __getitem__, and when inText reaches a point where something needs to be added to the db, it can just loop through __getitem__ and get the new Cve object returned | 18:34 |
| tsimonq2 | cjwatson: That's how my initial code worked, but it didn't split it into a different function, I'll have to play with @export_factory_operation to make it something that can be accessed via the db. Is that all OK? | 18:35 |
| tsimonq2 | s/accessed via the db/accessed via the API/ | 18:41 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!