Add apache_httpd improver #1102
Conversation
TG1999
force-pushed
the
apache_httpd_add_improver
branch
from
af94a5d to
3926ed1
Compare
AyanSinhaMahapatra
left a comment
AyanSinhaMahapatra
left a comment
There was a problem hiding this comment.
Thanks @TG1999 ! Looks good to me, just posted some questions/comments on code style/structure which is not required for merge btw.
| { | ||
| "system": "apache_httpd", | ||
| "value": "moderate", | ||
| "scoring_elements": "" |
There was a problem hiding this comment.
Shouldn't we have a null/none instead of a empty string when there is no value? This wouldn't matter because if "": would produce the same as if None: but it's good practice AFAIK :P
| } | ||
|
|
||
|
|
||
| class ApacheHTTPDImprover(Improver): |
There was a problem hiding this comment.
Why not have these classes as seperate files in the improvers directory? I only see the oval improver has it's own file, is that only because of it's size? We would probably want to be consistent here and have the improvers in their respective directory unless that's inconvenient/not possible because of something.
| if not link.endswith("json"): | ||
| continue | ||
| links.append(urllib.parse.urljoin(url, link)) | ||
| return links |
There was a problem hiding this comment.
See line 87 above in the importer class, there is a lot of dict/key usage, often several levels like:
for vendor in data["affects"]["vendor"]["vendor_data"] and then again for products in vendor["product"]["product_data"].
Shouldn't we probably use something like the AdvisoryData classes (or a special subclass for that) which would model the advisory data directly, and then use those classes everywhere instead of dicts. This has some advantages:
- code would be more pythonic and readable (and better looking)
- we can and should test for the advisory format and handle keyerrors gracefully (idk if the data format changes that much at all, but it could in future?)
There was a problem hiding this comment.
Also see https://github.com/nexB/vulnerablecode/pull/1102/files#diff-aa2cf137c50d3fb0242d00e7fbe786cc8e3d8c3d22c32b8afedde439965f2fb9R49, any reason we are using SPDX version of the license expression with spdx_license_expression instead of the scancode license expression version?
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
TG1999
force-pushed
the
apache_httpd_add_improver
branch
from
3926ed1 to
136d868
Compare
3 participants
Signed-off-by: Tushar Goel tushar.goel.dav@gmail.com