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

feat: Add User-Agent for mirroring #3183

Merged
merged 4 commits into from
Aug 2, 2023
Merged

feat: Add User-Agent for mirroring #3183

merged 4 commits into from
Aug 2, 2023

Conversation

terriko
Copy link
Contributor

@terriko terriko commented Jul 27, 2023

This adds a User-Agent header to cve-bin-tool, currently used only for aiohttp requests used in data_sources and mirroring. Here's what it looks like trying an experiment on localhost right now:

[terri@cedar ~]$ ls | nc -l -p 4888
GET //metadata.json HTTP/1.1
Host: 127.0.0.1:4888
User-Agent: cve-bin-tool/3.2.2dev0 (https://github.com/intel/cve-bin-tool/)
Accept: */*
Accept-Encoding: gzip, deflate

At this time, the PR doesn't cover any url loads using requests, only the ones using aiohttp

This adds a User-Agent header to cve-bin-tool, currently used only when
getting data from a mirror (as opposed to downloading directly from
other data_sources).

Signed-off-by: Terri Oda <terri.oda@intel.com>
There are still some requests-based url loads that are not covered.

Signed-off-by: Terri Oda <terri@toybox.ca>
@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2023

Codecov Report

Merging #3183 (a5cf257) into main (e2d1ef7) will decrease coverage by 2.13%.
The diff coverage is 64.28%.

@@            Coverage Diff             @@
##             main    #3183      +/-   ##
==========================================
- Coverage   81.40%   79.27%   -2.13%     
==========================================
  Files         716      714       -2     
  Lines       11114    10983     -131     
  Branches     1495     1271     -224     
==========================================
- Hits         9047     8707     -340     
- Misses       1681     1882     +201     
- Partials      386      394       +8     
Flag Coverage Δ
longtests ?
win-longtests 79.27% <64.28%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
cve_bin_tool/data_sources/rsd_source.py 0.00% <0.00%> (ø)
cve_bin_tool/data_sources/epss_source.py 69.86% <33.33%> (+0.41%) ⬆️
cve_bin_tool/nvd_api.py 19.17% <33.33%> (+0.42%) ⬆️
cve_bin_tool/data_sources/curl_source.py 98.00% <100.00%> (+0.04%) ⬆️
cve_bin_tool/data_sources/gad_source.py 74.22% <100.00%> (-10.75%) ⬇️
cve_bin_tool/data_sources/nvd_source.py 56.00% <100.00%> (+0.17%) ⬆️
cve_bin_tool/data_sources/osv_source.py 90.69% <100.00%> (+11.25%) ⬆️
cve_bin_tool/data_sources/redhat_source.py 67.76% <100.00%> (+4.84%) ⬆️
cve_bin_tool/fetch_json_db.py 49.27% <100.00%> (+0.37%) ⬆️
cve_bin_tool/version.py 94.44% <100.00%> (+0.32%) ⬆️

... and 19 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Signed-off-by: Terri Oda <terri@toybox.ca>
Signed-off-by: Terri Oda <terri@toybox.ca>
Copy link

@novafacing novafacing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, possibly worth extending this functionality in the future!

@@ -10,6 +10,10 @@

VERSION: str = "3.2.2dev0"

HTTP_HEADERS: dict = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only comment here is that this is such a good idea it may be worth adding an environment variable, command line arg, or configuration option to let the user optionally specify this value. The list of sources doesn't have anything I would anticipate adding user-agent checks, but it's possible corporate firewalls or some such thing may restrict outbound user agents.

@terriko
Copy link
Contributor Author

terriko commented Aug 2, 2023

Thanks all, let's get this merged!

@terriko terriko merged commit f1687d5 into intel:main Aug 2, 2023
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants