chore(codeql): address pre-existing CodeQL alerts #5243

[matejk]

Summary

Resolves pre-existing CodeQL alerts surfaced by PR #5235. Addresses errors, warnings, and notices across Foundation, Net, XML, JSON, Data, Zip, Prometheus, and Util components.

Closes #5243

Changes

Errors fixed:

• Fix duplicate include guard in FTPSClientSessionTest.h
• Add null checks for socket.impl() in PollSet.cpp
• Use POCO_DATA_INVALID_ROW instead of -1 in test Extractor.h

Warnings fixed:

• Remove tautological ch != eof check in MessageHeader.cpp
• Fix sleepTime reset logic in SQLChannel.cpp for proper exponential backoff
• Add explicit static_cast<int>() for enum comparisons in ICMPv4PacketImpl.cpp
• Default operator= in HTTPAuthenticationParams (Rule of Zero)
• Use qualified calls in destructors/constructors: SplitterChannel::close(), SocketImpl::close(), FTPClientSession::receiveServerReadyReply()

Notices fixed:

• Remove commented-out code across 6 files
• Remove trailing semicolons from doc comments (8 files)
• Rename shadowing variables in MailMessage, AbstractConfiguration, XMLStreamParser, Compress
• Simplify complex condition in DOMImplementation::hasFeature()
• Convert trivial switch to if/else in XMLStreamParser
• Fix Q-encoding bounds check and improve error handling in MessageHeader.cpp
• Add error check for eventfd read() in PollSet.cpp

False positives suppressed with comments:

• Authentication bypass by spoofing (proxy rewriting, IP parsing)
• Cleartext transmission (base socket layer)
• XXE (features exist to disable)
• Local address stored in non-local memory (observer/DOM/SAX patterns)
• Float equality (intentional in dynamic type system)
• Raw arrays in interfaces (SAX specification API)

Test plan

• cmake -B build-dir -DENABLE_TESTS=ON && cmake --build build-dir — builds cleanly
• Foundation, XML, JSON, Util, Prometheus, Data, DataSQLite, Zip test suites all pass
• Net test suite individual tests pass (suite times out due to network tests)

In general, to fix this type of issue you want the comparison to express a condition that can be either true or false given the actual runtime range of the operands. If part of a compound condition is provably always true (for example, checking that an unsigned‑derived value is >= 0), that part should be removed or the type/range assumptions revisited.

For this specific case in ICMPv4PacketImpl::errorDescription (Net/src/ICMPv4PacketImpl.cpp), the intent is to ensure code is within the valid range for indexing the corresponding error‑description arrays. Because code is derived from an ICMP header code field, which is an 8‑bit non‑negative value, only the upper bound of the range check is meaningful. The best fix that preserves behaviour is to remove the redundant lower‑bound checks while keeping the upper‑bound checks. Concretely:

• In the DESTINATION_UNREACHABLE_TYPE case, change if (code >= static_cast<int>(NET_UNREACHABLE) && code < static_cast<int>(DESTINATION_UNREACHABLE_UNKNOWN)) to if (code < static_cast<int>(DESTINATION_UNREACHABLE_UNKNOWN)).
• In the REDIRECT_MESSAGE_TYPE case, similarly drop the code >= static_cast<int>(REDIRECT_NETWORK) part.
• In the TIME_EXCEEDED_TYPE case, drop the code >= static_cast<int>(TIME_TO_LIVE) part.

No new methods or imports are needed; we only simplify the if conditions in the shown function.

In general, when a comparison is always the same truth value, remove or adjust the redundant part so that the condition can actually distinguish valid and invalid ranges, or rewrite the logic in terms of clearly defined bounds. Here, the intent in the REDIRECT_MESSAGE_TYPE case is to use code as an index into REDIRECT_MESSAGE_CODE only when it is in the valid range of redirect codes and fall back to an “unknown” entry otherwise.

The best fix while preserving behavior is to keep only the upper‑bound check, because code is already known to be non‑negative: replace

if (code >= static_cast<int>(REDIRECT_NETWORK) && code < static_cast<int>(REDIRECT_MESSAGE_UNKNOWN))
    err << REDIRECT_MESSAGE_CODE[code];
else
    err << REDIRECT_MESSAGE_CODE[REDIRECT_MESSAGE_UNKNOWN];

with

if (code < static_cast<int>(REDIRECT_MESSAGE_UNKNOWN))
    err << REDIRECT_MESSAGE_CODE[code];
else
    err << REDIRECT_MESSAGE_CODE[REDIRECT_MESSAGE_UNKNOWN];

This leaves indexing logic unchanged for all non‑negative code values and removes the redundant condition that CodeQL flags. No new methods, imports, or definitions are needed; the change is localized to the REDIRECT_MESSAGE_TYPE case in Net/src/ICMPv4PacketImpl.cpp, around lines 229–235.

In general, to fix “comparison result is always the same” issues, you either (1) remove the redundant part of the condition if it truly adds no value, or (2) adjust the operands or types to match the intended semantics if the redundancy is accidental. Here, the pattern is repeated for several ICMP message types: “lower-bound check is defensive (enum values may change)” followed by a code >= ... && code < ... guard. For the TIME_EXCEEDED_TYPE case, CodeQL determined the lower bound is always satisfied, so the first comparison is dead code.

The safest fix that does not change observable behavior is to drop the always‑true lower‑bound comparison in this specific case and rely solely on the upper bound check, which is the one that actually protects the array indexing. Concretely, in Net/src/ICMPv4PacketImpl.cpp, within ICMPv4PacketImpl::errorDescription, in the case TIME_EXCEEDED_TYPE: block, replace:

		// lower-bound check is defensive (enum values may change)
		if (code >= static_cast<int>(TIME_TO_LIVE) && code < static_cast<int>(TIME_EXCEEDED_UNKNOWN))
			err << TIME_EXCEEDED_CODE[code];
		else
			err << TIME_EXCEEDED_CODE[TIME_EXCEEDED_UNKNOWN];

with:

		if (code < static_cast<int>(TIME_EXCEEDED_UNKNOWN))
			err << TIME_EXCEEDED_CODE[code];
		else
			err << TIME_EXCEEDED_CODE[TIME_EXCEEDED_UNKNOWN];

This keeps the same behavior for all possible code values (since the removed condition was always true under the current type/value assumptions) and removes the condition that the analyzer flagged. No additional methods, imports, or definitions are required.