Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial import restructure #451

Merged
merged 72 commits into from Sep 30, 2020
Merged

Initial import restructure #451

merged 72 commits into from Sep 30, 2020

Conversation

P-T-I
Copy link
Member

@P-T-I P-T-I commented Aug 18, 2020

Took the liberty to redesign the (initial) import scripts and database populations. Structured logging and moved all print statements to the logging class. Initial import duration moved from several (4+) hours to less then an hour. Added update / populate progress monitoring via tqdm progress bars and (debug) logging. Fixed some typo's and wrong references.
Fixes #424
Fixes #435
Fixes #463

@adulau
Copy link
Member

adulau commented Sep 24, 2020

Maybe the best would have a complete set of CPE test search in travis and one with expected results? Just to be sure. Then we can merge it in the master/main branch and make a release. Your changes are really useful ;-) Thanks again.

@P-T-I
Copy link
Member Author

P-T-I commented Sep 24, 2020

Rebased branch

@P-T-I
Copy link
Member Author

P-T-I commented Sep 24, 2020

@adulau a complete set of tests would make sense, something like:

  • ./search.py -p cisco:ios:12.4 > /dev/null
  • ./search.py -p cisco:ios:12.4 -o json > /dev/null
  • ./search.py -p microsoft:windows_7 -o html > /dev/null
  • ./search.py -c CVE-2010-3333 > /dev/null
  • ./search_cpe.py -s wordpress -o json > /dev/null
  • ./search_cpe.py -s wordpress -o csv > /dev/null
  • ./search_cpe.py -s wordpress > /dev/null

Or would you like search_fulltext to be tested separately as well?

And the 2 tests (both for search.py and search_cpe.py) where to match the results to a expected outcome? With changing content that could be tricky; I'll look into it!

@adulau
Copy link
Member

adulau commented Sep 24, 2020

Yep and comparing the result to a set of known good output would be just perfect.

This was referenced Sep 25, 2020
@codecov-commenter
Copy link

codecov-commenter commented Sep 28, 2020

Codecov Report

Merging #451 into master will decrease coverage by 0.50%.
The diff coverage is 0.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #451      +/-   ##
==========================================
- Coverage   17.98%   17.47%   -0.51%     
==========================================
  Files          37       46       +9     
  Lines        3604     3994     +390     
==========================================
+ Hits          648      698      +50     
- Misses       2956     3296     +340     
Impacted Files Coverage Δ
lib/DownloadHandler.py 0.00% <0.00%> (ø)
lib/IJSONHandler.py 0.00% <0.00%> (ø)
lib/JSONFileHandler.py 0.00% <0.00%> (ø)
lib/LogHandler.py 0.00% <0.00%> (ø)
lib/PluginManager.py 0.00% <0.00%> (ø)
lib/Sources_process.py 0.00% <0.00%> (ø)
lib/XMLFileHandler.py 0.00% <0.00%> (ø)
lib/content_handlers.py 0.00% <0.00%> (ø)
lib/db_action.py 0.00% <0.00%> (ø)
lib/redis_q.py 0.00% <0.00%> (ø)
... and 27 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f9c93c...ea18e88. Read the comment docs.

@adulau
Copy link
Member

adulau commented Sep 29, 2020

I'm doing some final tests Today before the final merge ;-)

@P-T-I
Copy link
Member Author

P-T-I commented Sep 29, 2020

Awesome! Quick question; are the entries from exploit-db available somewhere in cve_search?

@adulau
Copy link
Member

adulau commented Sep 29, 2020

They were in VIA4CVE but not sure if they still provide the url with database.

@P-T-I
Copy link
Member Author

P-T-I commented Sep 29, 2020

How would that reference be named in the via4 collection?

@adulau
Copy link
Member

adulau commented Sep 29, 2020

FYI, https://cve-beta.circl.lu/ is now running this branch.

@adulau
Copy link
Member

adulau commented Sep 29, 2020

In my test, there is a missing field which is last-modified. Is that intended? It might break a lot of existing software relying on it. Could you add it? Beside that, everything looks good.

@P-T-I
Copy link
Member Author

P-T-I commented Sep 29, 2020

Huh? That's strange; no definitely not intended. Where are you missing the last-modified field? Which collection, cves? The only collection I found that has that field is the info collection.

@P-T-I
Copy link
Member Author

P-T-I commented Sep 29, 2020

I found the location where it was originally set; it's in the updateCVE method. I will correct it asap.

@P-T-I
Copy link
Member Author

P-T-I commented Sep 29, 2020

Done

This was referenced Sep 29, 2020
@adulau adulau merged commit b93eb03 into cve-search:master Sep 30, 2020
@adulau
Copy link
Member

adulau commented Sep 30, 2020

A huge thanks @P-T-I for the hard work 🥇 We will update the official API one with this version and then we will do a release of cve-search in the next days.

@P-T-I
Copy link
Member Author

P-T-I commented Sep 30, 2020

No problem; glad to help! I'll start working on the new cwe and capec releases. @adulau did you happen to take a look at the Travis settings? I believe parallel execution is disabled somehow?

@P-T-I P-T-I deleted the import_impr branch September 30, 2020 11:23
@P-T-I P-T-I mentioned this pull request Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants