Skip to content

Commit 5d345e2

Browse files
authored
Remove possibility of setFilter and clearFilter shadowing in derived devices. (#2005)
* Updates the `IFilterableDevice` interface to utilize a NVI pattern. This prevents issues such as shadowing overloads when multiple overloads are available and only 1 is overloaded. For example: `setFilter` previously required `using::setFilter` declaration on every class that included an override it. The change consolidates all device specific filter update code in `doUpdateFilter` hook. The methods `setFiltter` (+overloads) / `clearFilter` are no longer virtual and can be overridden. Their implementation is now forwarded through `doUpdateFilter`. * Updated PcapNgFileReaderDevice. * Fix inconsistent override warning. * Update `doUpdateFilter` to account for possible valid `""` filter string; * Fix * Fix * Typo.
1 parent 24cc309 commit 5d345e2

File tree

7 files changed

+74
-55
lines changed

7 files changed

+74
-55
lines changed

Pcap++/header/Device.h

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,22 +53,39 @@ namespace pcpp
5353
/// received
5454
/// @param[in] filter The filter to be set in PcapPlusPlus' GeneralFilter format
5555
/// @return True if filter set successfully, false otherwise
56-
virtual bool setFilter(GeneralFilter& filter)
56+
bool setFilter(GeneralFilter& filter)
5757
{
5858
std::string filterAsString;
5959
filter.parseToString(filterAsString);
60-
return setFilter(filterAsString);
60+
return doUpdateFilter(&filterAsString);
6161
}
6262

6363
/// Set a filter for the device. When implemented by the device, only packets that match the filter will be
64-
/// received
64+
/// processed.
6565
/// @param[in] filterAsString The filter to be set in Berkeley Packet Filter (BPF) syntax
6666
/// (http://biot.com/capstats/bpf.html)
6767
/// @return True if filter set successfully, false otherwise
68-
virtual bool setFilter(std::string filterAsString) = 0;
68+
bool setFilter(std::string const& filterAsString)
69+
{
70+
return doUpdateFilter(&filterAsString);
71+
}
6972

7073
/// Clear the filter currently set on the device
7174
/// @return True if filter was removed successfully or if no filter was set, false otherwise
72-
virtual bool clearFilter() = 0;
75+
bool clearFilter()
76+
{
77+
return doUpdateFilter(nullptr);
78+
}
79+
80+
protected:
81+
/// @brief Updates the filter on the device with a BPF string.
82+
///
83+
/// Only packets that match the filter should be processed by the device after this method is called.
84+
/// A nullptr should disable any existing filter on the device.
85+
///
86+
/// @param filterAsString A pointer to a string representing the filter in BPF syntax
87+
/// (http://biot.com/capstats/bpf.html).
88+
/// @return True if the operation was successful, false otherwise.
89+
virtual bool doUpdateFilter(std::string const* filterAsString) = 0;
7390
};
7491
} // namespace pcpp

Pcap++/header/PcapDevice.h

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -164,20 +164,7 @@ namespace pcpp
164164
PCPP_DEPRECATED("Prefer GeneralFilter::matches(...) method directly.")
165165
static bool matchPacketWithFilter(GeneralFilter& filter, RawPacket* rawPacket);
166166

167-
// implement abstract methods
168-
169-
using IFilterableDevice::setFilter;
170-
171-
/// Set a filter for the device. When implemented by the device, only packets that match the filter will be
172-
/// received. Please note that when the device is closed the filter is reset so when reopening the device you
173-
/// need to call this method again in order to reactivate the filter
174-
/// @param[in] filterAsString The filter to be set in Berkeley Packet Filter (BPF) syntax
175-
/// (http://biot.com/capstats/bpf.html)
176-
/// @return True if filter set successfully, false otherwise
177-
bool setFilter(std::string filterAsString) override;
178-
179-
/// Clear the filter currently set on device
180-
/// @return True if filter was removed successfully or if no filter was set, false otherwise
181-
bool clearFilter() override;
167+
protected:
168+
bool doUpdateFilter(std::string const* filterAsString) override;
182169
};
183170
} // namespace pcpp

Pcap++/header/PcapFileDevice.h

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -363,21 +363,18 @@ namespace pcpp
363363
/// @param[out] rawPacket A reference for an empty RawPacket where the packet will be written
364364
/// @return True if a packet was read successfully. False will be returned if the file isn't opened (also, an
365365
/// error log will be printed) or if reached end-of-file
366-
bool getNextPacket(RawPacket& rawPacket);
366+
bool getNextPacket(RawPacket& rawPacket) override;
367367

368368
/// Open the file name which path was specified in the constructor in a read-only mode
369369
/// @return True if file was opened successfully or if file is already opened. False if opening the file failed
370370
/// for some reason (for example: file path does not exist)
371-
bool open();
372-
373-
/// Set a filter for PcapNG reader device. Only packets that match the filter will be received
374-
/// @param[in] filterAsString The filter to be set in Berkeley Packet Filter (BPF) syntax
375-
/// (http://biot.com/capstats/bpf.html)
376-
/// @return True if filter set successfully, false otherwise
377-
bool setFilter(std::string filterAsString);
371+
bool open() override;
378372

379373
/// Close the pacp-ng file
380-
void close();
374+
void close() override;
375+
376+
protected:
377+
bool doUpdateFilter(std::string const* filter) override;
381378
};
382379

383380
/// @class PcapNgFileWriterDevice
@@ -476,11 +473,8 @@ namespace pcpp
476473
/// Flush and close the pcap-ng file
477474
void close() override;
478475

479-
/// Set a filter for PcapNG writer device. Only packets that match the filter will be persisted
480-
/// @param[in] filterAsString The filter to be set in Berkeley Packet Filter (BPF) syntax
481-
/// (http://biot.com/capstats/bpf.html)
482-
/// @return True if filter set successfully, false otherwise
483-
bool setFilter(std::string filterAsString) override;
476+
protected:
477+
bool doUpdateFilter(std::string const* filterAsString) override;
484478

485479
private:
486480
/// @struct PcapNgMetadata

Pcap++/header/PfRingDevice.h

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -315,15 +315,12 @@ namespace pcpp
315315
return m_DeviceOpened;
316316
}
317317

318-
using IFilterableDevice::setFilter;
318+
protected:
319+
bool doUpdateFilter(std::string const* filterAsString) override;
319320

320-
/// Sets a BPF filter to the device
321-
/// @param[in] filterAsString The BPF filter in string format
322-
bool setFilter(std::string filterAsString);
323-
324-
/// Remove a filter if currently set
325-
/// @return True if filter was removed successfully or if no filter was set, false otherwise
326-
bool clearFilter();
321+
private:
322+
bool removeFilterForAllChannels();
323+
bool setFilterForAllChannels(std::string const& filterAsString);
327324
};
328325

329326
} // namespace pcpp

Pcap++/src/PcapDevice.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ namespace pcpp
133133
return stats;
134134
}
135135

136-
bool IPcapDevice::setFilter(std::string filterAsString)
136+
bool IPcapDevice::doUpdateFilter(std::string const* filterAsString)
137137
{
138138
PCPP_LOG_DEBUG("Filter to be set: '" << filterAsString << "'");
139139
if (!isOpened())
@@ -142,12 +142,14 @@ namespace pcpp
142142
return false;
143143
}
144144

145-
return m_PcapDescriptor.setFilter(filterAsString);
146-
}
147-
148-
bool IPcapDevice::clearFilter()
149-
{
150-
return m_PcapDescriptor.clearFilter();
145+
if (filterAsString == nullptr || filterAsString->empty())
146+
{
147+
return m_PcapDescriptor.clearFilter();
148+
}
149+
else
150+
{
151+
return m_PcapDescriptor.setFilter(*filterAsString);
152+
}
151153
}
152154

153155
bool IPcapDevice::matchPacketWithFilter(GeneralFilter& filter, RawPacket* rawPacket)

Pcap++/src/PcapFileDevice.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -624,9 +624,14 @@ namespace pcpp
624624
return getNextPacket(rawPacket, temp);
625625
}
626626

627-
bool PcapNgFileReaderDevice::setFilter(std::string filterAsString)
627+
bool PcapNgFileReaderDevice::doUpdateFilter(std::string const* filterAsString)
628628
{
629-
return m_BpfWrapper.setFilter(filterAsString);
629+
if (filterAsString == nullptr)
630+
{
631+
return m_BpfWrapper.setFilter("");
632+
}
633+
634+
return m_BpfWrapper.setFilter(*filterAsString);
630635
}
631636

632637
void PcapNgFileReaderDevice::close()
@@ -876,9 +881,14 @@ namespace pcpp
876881
PCPP_LOG_DEBUG("File writer closed for file '" << m_FileName << "'");
877882
}
878883

879-
bool PcapNgFileWriterDevice::setFilter(std::string filterAsString)
884+
bool PcapNgFileWriterDevice::doUpdateFilter(std::string const* filterAsString)
880885
{
881-
return m_BpfWrapper.setFilter(filterAsString);
886+
if (filterAsString == nullptr)
887+
{
888+
return m_BpfWrapper.setFilter("");
889+
}
890+
891+
return m_BpfWrapper.setFilter(*filterAsString);
882892
}
883893

884894
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Pcap++/src/PfRingDevice.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,14 +366,26 @@ namespace pcpp
366366
return SystemCores::IdToSystemCore[sched_getcpu()];
367367
}
368368

369-
bool PfRingDevice::setFilter(std::string filterAsString)
369+
bool PfRingDevice::doUpdateFilter(std::string const* filterAsString)
370370
{
371371
if (!m_DeviceOpened)
372372
{
373373
PCPP_LOG_ERROR("Device not opened");
374374
return false;
375375
}
376376

377+
if (filterAsString == nullptr || filterAsString->empty())
378+
{
379+
return removeFilterForAllChannels();
380+
}
381+
else
382+
{
383+
return setFilterForAllChannels(*filterAsString);
384+
}
385+
}
386+
387+
bool PfRingDevice::setFilterForAllChannels(std::string const& filterAsString)
388+
{
377389
for (pfring* rxChannel : m_PfRingDescriptors)
378390
{
379391
int res = pfring_set_bpf_filter(rxChannel, (char*)filterAsString.c_str());
@@ -394,7 +406,7 @@ namespace pcpp
394406
return true;
395407
}
396408

397-
bool PfRingDevice::clearFilter()
409+
bool PfRingDevice::removeFilterForAllChannels()
398410
{
399411
if (!m_IsFilterCurrentlySet)
400412
return true;

0 commit comments

Comments
 (0)