Skip to content

Commit 55bbbb4

Browse files
authored
fix(traceflow): report the output path for open flows when the extension is empty (#2237) (#2246)
TCP.submit guarded the open-flow fpout on a truthy extension, while _finalise checked `is not None`. A dumper registered with an empty extension writes `{root}/{label}`, so submit reported None for every open flow that _finalise later reported by path. Both now build fpout through TCP._output_path, which returns None only when the format writes no file. The Index.fpout docs say so. Test: test_an_empty_extension_reports_the_written_file_open_or_finalised reads fpout mid-capture and after finish.
1 parent 20c8b2f commit 55bbbb4

4 files changed

Lines changed: 54 additions & 4 deletions

File tree

‎docs/source/pcapkit/foundation/traceflow/tcp.rst‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,8 @@ Terminology
120120
121121
(tuple) index
122122
|--> (Info) data
123-
| |--> 'fpout' : (Optional[str]) output filename if exists
123+
| |--> 'fpout' : (Optional[str]) output filename, or None when
124+
| | the format writes no file
124125
| |--> 'index': (tuple) tuple of frame index, both directions,
125126
| | in capture order
126127
| | |--> (int) frame index

‎pcapkit/foundation/traceflow/data/tcp.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,8 @@ class Index(DeferredPacket, Info):
167167
#: :class:`~pcapkit.corekit.packet.DeferredPacket` to resolve.
168168
__deferred__ = Deferred
169169

170-
#: Output filename if exists.
170+
#: Path of the flow's output file, or :data:`None` when the output format
171+
#: writes no file. Open and finalised flows report the same path.
171172
fpout: 'Optional[str]'
172173
#: Tuple of frame index, **both directions**, in capture order.
173174
index: 'tuple[int, ...]'

‎pcapkit/foundation/traceflow/tcp.py‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,25 @@ def _make_segment(self, packet: 'Packet[_AT]') -> 'Any':
436436
timestamp=packet.timestamp, # capture timestamp
437437
)
438438

439+
def _output_path(self, label: 'str') -> 'Optional[str]':
440+
"""Path of a flow's output file, as an index entry reports it.
441+
442+
Arguments:
443+
label: the flow's label
444+
445+
Returns:
446+
The file :meth:`trace` writes the flow to, or :data:`None` when the
447+
output format writes no file at all. An *empty* extension still
448+
writes one, named after the label alone.
449+
450+
Both :meth:`submit` and :meth:`_finalise` report through this, so an
451+
open flow and a finalised one name the same file (:issue:`2237`).
452+
453+
"""
454+
if self._fdpext is None:
455+
return None
456+
return f'{self._fproot}/{label}{self._fdpext}'
457+
439458
@staticmethod
440459
def _ended(buffer: 'Buffer[_AT]') -> 'bool':
441460
"""Whether a flow's connection has been seen to end.
@@ -478,7 +497,7 @@ def _finalise(self, bufid: 'BufferID') -> 'Index':
478497
logger.debug('TCP flow %s finalised after %d frame(s) (%d forward, %d reverse)',
479498
label, len(buf.index), len(buf.forward), len(buf.reverse))
480499
index = Index(
481-
fpout=f'{self._fproot}/{label}{self._fdpext}' if self._fdpext is not None else None,
500+
fpout=self._output_path(label),
482501
index=tuple(buf.index),
483502
label=label,
484503
forward=tuple(buf.forward),
@@ -550,7 +569,7 @@ def submit(self) -> 'tuple[Index, ...]':
550569

551570
ret = [] # type: list[Index]
552571
for buf in self._buffer.values():
553-
ret.append(Index(fpout=f"{self._fproot}/{buf.label}{self._fdpext}" if self._fdpext else None,
572+
ret.append(Index(fpout=self._output_path(buf.label),
554573
index=tuple(buf.index),
555574
label=buf.label,
556575
forward=tuple(buf.forward),

‎tests/foundation/traceflow/test_tcp.py‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,35 @@ def test_tcp_trace_ipv6_label_and_no_extension_output_path(self) -> None:
552552
self.assertEqual(final_index.index, (1, 2))
553553
self.assertFalse(pathlib.Path(tempdir, f'{label}.None').exists())
554554

555+
def test_an_empty_extension_reports_the_written_file_open_or_finalised(self) -> None:
556+
"""An empty extension still writes a file, and every report names it (#2237).
557+
558+
The flow file is named after the label alone. :meth:`TCP.submit` reported
559+
:data:`None` for a flow still open while :meth:`TCP._finalise` reported the
560+
path, so one flow's ``fpout`` depended on when it was read.
561+
562+
"""
563+
from dictdumper import JSON
564+
565+
from pcapkit.foundation.traceflow.tcp import TCP
566+
567+
TCP.register_dumper('unit-noext', JSON, '')
568+
569+
with tempfile.TemporaryDirectory() as tempdir:
570+
trace = TCP(tempdir, 'unit-noext')
571+
label = trace.trace(self._packet(index=1, syn=True))
572+
expected = f'{tempdir}/{label}'
573+
self.assertTrue(pathlib.Path(expected).is_file())
574+
575+
# mid-capture: the flow is open, reported by ``submit``
576+
open_index, = trace.index
577+
self.assertEqual(open_index.fpout, expected)
578+
579+
# end of capture: the same flow, reported by ``_finalise``
580+
trace.finish()
581+
final_index, = trace.index
582+
self.assertEqual(final_index.fpout, expected)
583+
555584

556585
if __name__ == '__main__':
557586
unittest.main()

0 commit comments

Comments
 (0)