Skip to content

snmp: keep printing results after an ICMP answer - #5128

Open
KernelClint wants to merge 1 commit into
secdev:masterfrom
KernelClint:snmpget-continue-after-icmp
Open

snmp: keep printing results after an ICMP answer#5128
KernelClint wants to merge 1 commit into
secdev:masterfrom
KernelClint:snmpget-continue-after-icmp

Conversation

@KernelClint

Copy link
Copy Markdown
Contributor

snmpget() can query several targets in one call and prints what each returns. Some answers are
not SNMP at all: Scapy matches an ICMP error to the probe that provoked it, so an unreachable host
or router appears in the answered set.

The result loop at
scapy/layers/snmp.py:290-317
prints that error and then returns from the function rather than moving to the next answer.
Everything before the ICMP answer in the iteration has already printed; everything after it is
discarded. The results were collected — they are simply never shown.

The change continues instead of returning:

         if ICMP in r.answer:
             print("%s: %s" % (r.answer.src, r.answer.sprintf("%ICMP.type%")))
-            return
+            continue

The diagnostic still prints, so nothing about the error case is lost.

The added regression builds a multi-target result set with an ICMP answer ahead of a valid SNMP
answer and asserts both appear in the output. Restoring the return makes it fail.

Performance was measured on one computer, before and after the fix: a representative call took
113.5 µs before and 114.3 µs after. Repeat runs moved by about 2%, so that difference is smaller
than the test can distinguish.

AI-Assisted: yes (GPT-5.6-Cyber)
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.64%. Comparing base (b3bbcc8) to head (e5ab25f).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5128      +/-   ##
==========================================
+ Coverage   80.63%   80.64%   +0.01%     
==========================================
  Files         390      390              
  Lines       96936    96936              
==========================================
+ Hits        78168    78178      +10     
+ Misses      18768    18758      -10     
Files with missing lines Coverage Δ
scapy/layers/snmp.py 89.06% <100.00%> (+4.68%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant