Update Apache_httpd importer from XML to JSON Advisory by AmitGupta7580 · Pull Request #425 · aboutcode-org/vulnerablecode

AmitGupta7580 · 2021-04-04T11:58:04Z

Resolve #423

Tasks :

Fetch data into JSON format.
Differentiate impacted and resolved packages.
Add severity of vulnerability.
Create Test Case class for Apache_httpd importer.

Sync Forked repo

Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

sbs2001

Thanks, this looks good to be merged with minor changes I suggested

vulnerabilities/importers/apache_httpd.py

vulnerabilities/tests/test_apache_httpd.py

pombredanne · 2021-04-14T13:18:25Z

vulnerabilities/importers/apache_httpd.py

+    def to_advisory(self, data):
+        cve = data["CVE_data_meta"]["ID"]
+        description = data["description"]["description_data"]
+        summary = next((item["value"] for item in description if item["lang"] == "eng"), "")


Using next here is a little cryptic. Would there be a more readable and explicit way to do this?
I reckon that description has this shape (and are you sure this is always present?):

"description": { "description_data": [ { "lang": "eng", "value": "For configurations using proxying...." } ] }

so what about this?

descriptions = data.get("description", {}).get("description_data", []) description = None for desc in descriptions: if desc.get("lang") == "eng": description = desc.get("value") break

description is always present, @AmitGupta7580 checked it.

vulnerabilities/importers/apache_httpd.py

Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

sbs2001

I guess this needs to rebased and refactored according to new model (which would 1-2 line changes IMHO) See the new importer now for reference

… model Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

AmitGupta7580 · 2021-04-22T16:00:28Z

Done 👍

vulnerabilities/importers/apache_httpd.py

vulnerabilities/tests/test_apache_httpd.py

…versions of Apache-httpd from Github API Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

sbs2001

@AmitGupta7580 thanks, this looks good with minor changes.

vulnerabilities/importers/apache_httpd.py

sbs2001 · 2021-04-28T04:32:56Z

vulnerabilities/importers/apache_httpd.py


+    def to_advisory(self, data):
+        cve = data["CVE_data_meta"]["ID"]
+        descriptions = data.get("description", {}).get("description_data", [])


Just curious, does this cascade all the way down. Could you point me to an example where descriptions == [] .

No, currently there is no example where descriptions == [].

I ran this code but got nothing.

descriptions = data.get("description", {}).get("description_data", []) if len(descriptions) == 0: print("empty descriptions : {}".format(cve)) description = None for desc in descriptions: if desc.get("lang") == "eng": description = desc.get("value") else: print("other language : {}".format(cve))

This is too defensive then, may mask issues, ideally we want to fail loudly.

Does descriptions = data["description"]["description_data"] work ? If yes then use that, otherwise try
descriptions = data["description"].get("description_data", [])

sbs2001 · 2021-04-28T04:34:33Z

vulnerabilities/importers/apache_httpd.py

+        descriptions = data.get("description", {}).get("description_data", [])
+        description = None
+        for desc in descriptions:
+            if desc.get("lang") == "eng":


When does this condition not get satisifed, ie does desc.get("lang") == None happen ?

I'm guessing that desc.get("lang") is never None ("lang" key is always present in desc) . If yes then use if desc["lang"] == "eng" : .....

vulnerabilities/importers/apache_httpd.py

sbs2001 · 2021-04-28T04:43:56Z

vulnerabilities/importers/apache_httpd.py

-        for info in issue:
-            if info.tag == "cve":
-                cve = info.attrib["name"]
+        versions = []


This name is misleading because really, it stores some version related data (which includes ranges and such).

Maybe rename this to version_data

sbs2001 · 2021-04-28T04:49:20Z

vulnerabilities/importers/apache_httpd.py

        )

-    return advisories
+    def to_version_ranges(self, versions):


…ode format Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

sbs2001 · 2021-04-29T04:50:18Z

vulnerabilities/importers/apache_httpd.py


+    def to_advisory(self, data):
+        cve = data["CVE_data_meta"]["ID"]
+        descriptions = data.get("description", {}).get("description_data", [])


This is too defensive then, may mask issues, ideally we want to fail loudly.

Does descriptions = data["description"]["description_data"] work ? If yes then use that, otherwise try
descriptions = data["description"].get("description_data", [])

sbs2001 · 2021-04-29T04:53:28Z

vulnerabilities/importers/apache_httpd.py

+        descriptions = data.get("description", {}).get("description_data", [])
+        description = None
+        for desc in descriptions:
+            if desc.get("lang") == "eng":


I'm guessing that desc.get("lang") is never None ("lang" key is always present in desc) . If yes then use if desc["lang"] == "eng" : .....

sbs2001 · 2021-04-29T04:54:37Z

vulnerabilities/importers/apache_httpd.py

+        impacts = data.get("impact", [])
+        for impact in impacts:
+            value = impact.get("other")
+            if value is not None:


Use if value: instead, they are the same but concise and easy to read.

Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

sbs2001 · 2021-05-05T16:53:17Z

vulnerabilities/importers/apache_httpd.py

-        if create_etag(data_src=self, url=self.url, etag_key="ETag"):
-            data = fetch_xml(self.url)
-            advisories = to_advisories(data)
+        if create_etag(data_src=self, url=self.base_url, etag_key="ETag"):


This isn't valid now because the provided endpoint doesn't provide etags in the headers. Remove this if statement and the emtpy list return below

Also the comments related to etags

Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

sbs2001 · 2021-05-08T15:22:38Z

vulnerabilities/importers/apache_httpd.py

+from vulnerabilities.data_source import VulnerabilitySeverity
+from vulnerabilities.package_managers import GitHubTagsAPI
+from vulnerabilities.severity_systems import scoring_systems
 from vulnerabilities.helpers import create_etag


Nit this isn't used anymore

sbs2001 · 2021-05-08T15:23:49Z

@AmitGupta7580 I'm squashing the commits and merging it, consider squashing from your side next time .

AmitGupta7580 added 8 commits April 4, 2021 13:22

Merge pull request #7 from nexB/main

a0e6be9

Sync Forked repo

Update Apache httpd importer from importing XML to JSON data

e9172cb

Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

Add severity system and test file for apache_httpd

4bd6a38

Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

Add new Severity System for Apache_httpd Severity

c25b48c

Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

Merge branch 'main' into update-apache-httpd-importer

5ac506d

Merge branch 'main' into update-apache-httpd-importer

9a7c438

Merge branch 'main' into update-apache-httpd-importer

7dba53f

Merge branch 'main' into update-apache-httpd-importer

db9ec0d

sbs2001 requested changes Apr 14, 2021

View reviewed changes

pombredanne requested changes Apr 14, 2021

View reviewed changes

Improve the format of apache_httpd importer

2bc3acc

Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

AmitGupta7580 requested review from pombredanne and sbs2001 April 15, 2021 09:53

sbs2001 requested changes Apr 22, 2021

View reviewed changes

AmitGupta7580 added 2 commits April 22, 2021 19:23

Merge branch 'main' into update-apache-httpd-importer

05b1ef6

rebased and refactored the apache_httpd importer according to the new…

a4b4633

… model Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

AmitGupta7580 requested a review from sbs2001 April 22, 2021 16:00

sbs2001 requested changes Apr 23, 2021

View reviewed changes

vulnerabilities/importers/apache_httpd.py Outdated Show resolved Hide resolved

vulnerabilities/importers/apache_httpd.py Outdated Show resolved Hide resolved

vulnerabilities/tests/test_apache_httpd.py Show resolved Hide resolved

Resolve version ranges into discrete versions and fetch all released …

28f44ef

…versions of Apache-httpd from Github API Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

AmitGupta7580 requested a review from sbs2001 April 26, 2021 05:53

sbs2001 requested changes Apr 28, 2021

View reviewed changes

AmitGupta7580 added 2 commits April 28, 2021 15:26

Resolve the problem of adding format string in etag and improve the c…

6537967

…ode format Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

Merge branch 'main' into update-apache-httpd-importer

f165d16

AmitGupta7580 requested a review from sbs2001 April 28, 2021 17:08

sbs2001 requested changes Apr 29, 2021

View reviewed changes

AmitGupta7580 added 2 commits April 29, 2021 18:52

Improve data extraction from JSON data

4157037

Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

Merge branch 'main' into update-apache-httpd-importer

996ef17

AmitGupta7580 requested a review from sbs2001 April 29, 2021 18:52

sbs2001 reviewed May 5, 2021

View reviewed changes

AmitGupta7580 added 2 commits May 6, 2021 23:26

Merge branch 'main' into update-apache-httpd-importer

5057e23

Removed Etag check

223cabe

Signed-off-by: AmitGupta7580 <amitgupta758000@gmail.com>

AmitGupta7580 requested a review from sbs2001 May 6, 2021 18:54

sbs2001 reviewed May 8, 2021

View reviewed changes

sbs2001 approved these changes May 8, 2021

View reviewed changes

sbs2001 merged commit d7d3206 into aboutcode-org:main May 8, 2021

AmitGupta7580 deleted the update-apache-httpd-importer branch May 9, 2021 06:52

Uh oh!

Conversation

AmitGupta7580 commented Apr 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbs2001 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sbs2001 left a comment

Choose a reason for hiding this comment

Uh oh!

AmitGupta7580 commented Apr 22, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sbs2001 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbs2001 May 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbs2001 commented May 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AmitGupta7580 commented Apr 4, 2021 •

edited

Loading

sbs2001 May 5, 2021 •

edited

Loading