From e3fd6ced99e74cfb68deb01ccd52e3a4c0d87083 Mon Sep 17 00:00:00 2001 From: alireza787b Date: Tue, 28 Jul 2026 05:22:15 +0000 Subject: [PATCH 01/11] fix(VideoManager): redact stream URLs in diagnostics --- src/Settings/VideoSettings.cc | 8 +- src/Utilities/Network/QGCNetworkHelper.cc | 25 ++++++ src/Utilities/Network/QGCNetworkHelper.h | 5 ++ src/VideoManager/VideoManager.cc | 13 ++- .../GStreamer/GstSourceFactory.cc | 16 ++-- .../GStreamer/GstVideoReceiver.cc | 89 +++++++++++-------- .../GStreamer/GstVideoReceiver.h | 1 + .../Utilities/Network/QGCNetworkHelperTest.cc | 19 ++++ test/Utilities/Network/QGCNetworkHelperTest.h | 1 + 9 files changed, 125 insertions(+), 52 deletions(-) diff --git a/src/Settings/VideoSettings.cc b/src/Settings/VideoSettings.cc index ae43d03c6c1a..c4f799e613af 100644 --- a/src/Settings/VideoSettings.cc +++ b/src/Settings/VideoSettings.cc @@ -243,22 +243,22 @@ bool VideoSettings::streamConfigured(void) } //-- If UDP, check for URL if(vSource == videoSourceUDPH264 || vSource == videoSourceUDPH265) { - qCDebug(VideoSettingsLog) << "Testing configuration for UDP Stream:" << udpUrl()->rawValue().toString(); + qCDebug(VideoSettingsLog) << "Testing configuration for UDP Stream"; return !udpUrl()->rawValue().toString().isEmpty(); } //-- If RTSP, check for URL if(vSource == videoSourceRTSP) { - qCDebug(VideoSettingsLog) << "Testing configuration for RTSP Stream:" << rtspUrl()->rawValue().toString(); + qCDebug(VideoSettingsLog) << "Testing configuration for RTSP Stream"; return !rtspUrl()->rawValue().toString().isEmpty(); } //-- If TCP, check for URL if(vSource == videoSourceTCP) { - qCDebug(VideoSettingsLog) << "Testing configuration for TCP Stream:" << tcpUrl()->rawValue().toString(); + qCDebug(VideoSettingsLog) << "Testing configuration for TCP Stream"; return !tcpUrl()->rawValue().toString().isEmpty(); } //-- If MPEG-TS, check for URL if(vSource == videoSourceMPEGTS) { - qCDebug(VideoSettingsLog) << "Testing configuration for MPEG-TS Stream:" << udpUrl()->rawValue().toString(); + qCDebug(VideoSettingsLog) << "Testing configuration for MPEG-TS Stream"; return !udpUrl()->rawValue().toString().isEmpty(); } //-- If Herelink Air unit, good to go diff --git a/src/Utilities/Network/QGCNetworkHelper.cc b/src/Utilities/Network/QGCNetworkHelper.cc index 5545ee2cc389..17046bd960cc 100644 --- a/src/Utilities/Network/QGCNetworkHelper.cc +++ b/src/Utilities/Network/QGCNetworkHelper.cc @@ -342,6 +342,31 @@ QUrl urlWithoutQuery(const QUrl& url) return url.adjusted(QUrl::RemoveQuery | QUrl::RemoveFragment); } +QString redactedUrlForLogging(const QUrl& url) +{ + if (!url.isValid() || url.scheme().isEmpty()) { + return QStringLiteral(""); + } + + QUrl redactedUrl(url); + const bool hadPath = !redactedUrl.path().isEmpty() && (redactedUrl.path() != QLatin1String("/")); + redactedUrl.setUserInfo(QString()); + redactedUrl.setPath(hadPath ? QString() : redactedUrl.path()); + redactedUrl.setQuery(QString()); + redactedUrl.setFragment(QString()); + + QString displayUrl = redactedUrl.toDisplayString(QUrl::FullyEncoded); + if (hadPath) { + displayUrl += QStringLiteral("/"); + } + return displayUrl; +} + +QString redactedUrlForLogging(const QString& url) +{ + return redactedUrlForLogging(QUrl(url)); +} + // ============================================================================ // Request Configuration // ============================================================================ diff --git a/src/Utilities/Network/QGCNetworkHelper.h b/src/Utilities/Network/QGCNetworkHelper.h index cb92f2d03a3d..1d3faf7aeaf2 100644 --- a/src/Utilities/Network/QGCNetworkHelper.h +++ b/src/Utilities/Network/QGCNetworkHelper.h @@ -139,6 +139,11 @@ QUrl buildUrl(const QString& baseUrl, const QList>& para /// Get URL without query string and fragment QUrl urlWithoutQuery(const QUrl& url); +/// Return a URL suitable for diagnostics without user info, path, query, or fragment secrets. +/// Invalid and relative input is not echoed. +QString redactedUrlForLogging(const QUrl& url); +QString redactedUrlForLogging(const QString& url); + // ============================================================================ // Request Configuration // ============================================================================ diff --git a/src/VideoManager/VideoManager.cc b/src/VideoManager/VideoManager.cc index 72d47e207f7d..624923aae7b2 100644 --- a/src/VideoManager/VideoManager.cc +++ b/src/VideoManager/VideoManager.cc @@ -7,6 +7,7 @@ #include "QGCCameraManager.h" #include "QGCCorePlugin.h" #include "QGCLoggingCategory.h" +#include "QGCNetworkHelper.h" #include "QGCVideoStreamInfo.h" #include "SettingsManager.h" #include "SubtitleWriter.h" @@ -593,7 +594,8 @@ bool VideoManager::_updateAutoStream(VideoReceiver *receiver) return false; } - qCDebug(VideoManagerLog) << QString("Configure stream (%1):").arg(receiver->name()) << pInfo->uri(); + qCDebug(VideoManagerLog) << QString("Configure stream (%1):").arg(receiver->name()) + << QGCNetworkHelper::redactedUrlForLogging(pInfo->uri()); QString source, url; switch (pInfo->type()) { @@ -651,7 +653,7 @@ bool VideoManager::_updateVideoUri(VideoReceiver *receiver, const QString &uri) return false; } - qCDebug(VideoManagerLog) << "New Video URI" << uri; + qCDebug(VideoManagerLog) << "New Video URI" << QGCNetworkHelper::redactedUrlForLogging(uri); receiver->setUri(uri); @@ -899,13 +901,16 @@ void VideoManager::_initVideoReceiver(VideoReceiver *receiver, QQuickWindow *win }); (void) connect(receiver, &VideoReceiver::onStopComplete, this, [this, receiver](VideoReceiver::STATUS status) { - qCDebug(VideoManagerLog) << "Stop complete" << receiver->name() << receiver->uri() << ", status:" << status; + qCDebug(VideoManagerLog) << "Stop complete" << receiver->name() + << QGCNetworkHelper::redactedUrlForLogging(receiver->uri()) + << ", status:" << status; receiver->setStarted(false); if (status == VideoReceiver::STATUS_INVALID_URL) { qCDebug(VideoManagerLog) << "Invalid video URL. Not restarting"; } else { QTimer::singleShot(1000, receiver, [this, receiver]() { - qCDebug(VideoManagerLog) << "Restarting video receiver" << receiver->name() << receiver->uri(); + qCDebug(VideoManagerLog) << "Restarting video receiver" << receiver->name() + << QGCNetworkHelper::redactedUrlForLogging(receiver->uri()); _startReceiver(receiver); }); } diff --git a/src/VideoManager/VideoReceiver/GStreamer/GstSourceFactory.cc b/src/VideoManager/VideoReceiver/GStreamer/GstSourceFactory.cc index d5a161ab464b..04163fa862e8 100644 --- a/src/VideoManager/VideoReceiver/GStreamer/GstSourceFactory.cc +++ b/src/VideoManager/VideoReceiver/GStreamer/GstSourceFactory.cc @@ -7,6 +7,7 @@ #include "GStreamerHelpers.h" #include "QGCLoggingCategory.h" +#include "QGCNetworkHelper.h" QGC_LOGGING_CATEGORY(GstSourceFactoryLog, "Video.GStreamer.GstSourceFactory") @@ -295,7 +296,8 @@ void linkPad(GstElement* element, GstPad* pad, gpointer data) GstElement* buildRtspSource(const QString& uri, const QUrl& sourceUrl, const Config& config, guint latencyMs) { if (!GStreamer::isValidRtspUri(uri.toUtf8().constData())) { - qCCritical(GstSourceFactoryLog) << "Invalid RTSP URI:" << sourceUrl.toDisplayString(QUrl::RemoveUserInfo); + qCCritical(GstSourceFactoryLog) << "Invalid RTSP URI:" + << QGCNetworkHelper::redactedUrlForLogging(sourceUrl); return nullptr; } @@ -336,12 +338,14 @@ GstElement* buildTcpSource(const QUrl& sourceUrl) { const int port = sourceUrl.port(); if (!validPort(port)) { - qCCritical(GstSourceFactoryLog) << "Invalid TCP port" << port << "in" << sourceUrl.toDisplayString(QUrl::RemoveUserInfo); + qCCritical(GstSourceFactoryLog) << "Invalid TCP port" << port << "in" + << QGCNetworkHelper::redactedUrlForLogging(sourceUrl); return nullptr; } const QString host = sourceUrl.host(); if (host.isEmpty()) { - qCCritical(GstSourceFactoryLog) << "Missing host in TCP URI" << sourceUrl.toDisplayString(QUrl::RemoveUserInfo); + qCCritical(GstSourceFactoryLog) << "Missing host in TCP URI" + << QGCNetworkHelper::redactedUrlForLogging(sourceUrl); return nullptr; } @@ -359,7 +363,8 @@ GstElement* buildUdpSource(const QUrl& sourceUrl, bool isUdpH264, bool isUdpH265 { const int port = sourceUrl.port(); if (!validPort(port)) { - qCCritical(GstSourceFactoryLog) << "Invalid UDP port" << port << "in" << sourceUrl.toDisplayString(QUrl::RemoveUserInfo); + qCCritical(GstSourceFactoryLog) << "Invalid UDP port" << port << "in" + << QGCNetworkHelper::redactedUrlForLogging(sourceUrl); return nullptr; } @@ -528,7 +533,8 @@ GstElement* create(const QString& uri, const Config& config) const bool isTcpMPEGTS = (scheme == QLatin1String("tcp")); if (!isRtsp && !isUdpH264 && !isUdpH265 && !isUdpMPEGTS && !isTcpMPEGTS) { - qCWarning(GstSourceFactoryLog) << "Unsupported URI scheme:" << scheme << "in" << sourceUrl.toDisplayString(QUrl::RemoveUserInfo); + qCWarning(GstSourceFactoryLog) << "Unsupported URI scheme:" << scheme << "in" + << QGCNetworkHelper::redactedUrlForLogging(sourceUrl); return nullptr; } diff --git a/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.cc b/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.cc index 395626a6a525..3a2767e62935 100644 --- a/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.cc +++ b/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.cc @@ -15,6 +15,7 @@ #include "GStreamerHelpers.h" #include "GstSourceFactory.h" #include "QGCLoggingCategory.h" +#include "QGCNetworkHelper.h" #include "QGCQVideoSinkController.h" #include @@ -96,6 +97,11 @@ GstVideoReceiver::~GstVideoReceiver() qCDebug(GstVideoReceiverLog) << this; } +QString GstVideoReceiver::_redactedUri() const +{ + return QGCNetworkHelper::redactedUrlForLogging(_uri); +} + void GstVideoReceiver::start(uint32_t timeout) { if (_needDispatch()) { @@ -104,7 +110,7 @@ void GstVideoReceiver::start(uint32_t timeout) } if (_pipeline) { - qCDebug(GstVideoReceiverLog) << "Already running!" << _uri; + qCDebug(GstVideoReceiverLog) << "Already running!" << _redactedUri(); emit onStartComplete(STATUS_INVALID_STATE); return; } @@ -118,7 +124,8 @@ void GstVideoReceiver::start(uint32_t timeout) _timeout = timeout; _buffer = lowLatency() ? -1 : 0; - qCDebug(GstVideoReceiverLog) << "Starting" << _uri << ", lowLatency" << lowLatency() << ", timeout" << _timeout; + qCDebug(GstVideoReceiverLog) << "Starting" << _redactedUri() << ", lowLatency" << lowLatency() + << ", timeout" << _timeout; // GST_DEBUG_BIN_TO_DOT_FILE is a no-op unless GST_DEBUG_DUMP_DOT_DIR is set; surface that // once per process so field debugging doesn't require re-reading the source. @@ -292,7 +299,7 @@ void GstVideoReceiver::start(uint32_t timeout) emit onStartComplete(STATUS_FAIL); } else { GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-started"); - qCDebug(GstVideoReceiverLog) << "Started" << _uri; + qCDebug(GstVideoReceiverLog) << "Started" << _redactedUri(); // _watchdogTimer lives on `this` (GUI thread); the emit runs synchronously on the // worker thread, so the timer start has to be queued separately or QObject warns. @@ -313,7 +320,7 @@ void GstVideoReceiver::stop() return; } - qCDebug(GstVideoReceiverLog) << "Stopping" << _uri; + qCDebug(GstVideoReceiverLog) << "Stopping" << _redactedUri(); // Bump the epoch synchronously (atomic — no GUI thread needed) so any in-flight reconnect lambda // is superseded before this stop() returns; cross-callsite QueuedConnection FIFO is not guaranteed. @@ -428,18 +435,18 @@ void GstVideoReceiver::stop() if (_streaming) { _streaming = false; - qCDebug(GstVideoReceiverLog) << "Streaming stopped" << _uri; + qCDebug(GstVideoReceiverLog) << "Streaming stopped" << _redactedUri(); emit streamingChanged(_streaming); } else { - qCDebug(GstVideoReceiverLog) << "Streaming did not start" << _uri; + qCDebug(GstVideoReceiverLog) << "Streaming did not start" << _redactedUri(); } } - qCDebug(GstVideoReceiverLog) << "Stopped" << _uri; + qCDebug(GstVideoReceiverLog) << "Stopped" << _redactedUri(); if (const HwBuffers::PathStats hwStats = HwBuffers::formatPathStats(true); hwStats.totalDelivered > 0) { qCInfo(GstVideoReceiverLog).noquote() - << "HW path stats" << _uri << hwStats.line + HwBuffers::takeExtraPathStats(); + << "HW path stats" << _redactedUri() << hwStats.line + HwBuffers::takeExtraPathStats(); } emit onStopComplete(STATUS_OK); @@ -448,7 +455,7 @@ void GstVideoReceiver::stop() void GstVideoReceiver::startDecoding(void *sink) { if (!sink) { - qCCritical(GstVideoReceiverLog) << "VideoSink is NULL" << _uri; + qCCritical(GstVideoReceiverLog) << "VideoSink is NULL" << _redactedUri(); return; } @@ -457,10 +464,10 @@ void GstVideoReceiver::startDecoding(void *sink) return; } - qCDebug(GstVideoReceiverLog) << "Starting decoding" << _uri; + qCDebug(GstVideoReceiverLog) << "Starting decoding" << _redactedUri(); if (!_widget) { - qCDebug(GstVideoReceiverLog) << "Video Widget is NULL" << _uri; + qCDebug(GstVideoReceiverLog) << "Video Widget is NULL" << _redactedUri(); emit onStartDecodingComplete(STATUS_FAIL); return; } @@ -470,7 +477,7 @@ void GstVideoReceiver::startDecoding(void *sink) } if (_videoSink || _decoding) { - qCDebug(GstVideoReceiverLog) << "Already decoding!" << _uri; + qCDebug(GstVideoReceiverLog) << "Already decoding!" << _redactedUri(); emit onStartDecodingComplete(STATUS_INVALID_STATE); return; } @@ -478,7 +485,7 @@ void GstVideoReceiver::startDecoding(void *sink) GstElement *videoSink = GST_ELEMENT(sink); GstPad *pad = gst_element_get_static_pad(videoSink, "sink"); if (!pad) { - qCCritical(GstVideoReceiverLog) << "Unable to find sink pad of video sink" << _uri; + qCCritical(GstVideoReceiverLog) << "Unable to find sink pad of video sink" << _redactedUri(); emit onStartDecodingComplete(STATUS_FAIL); return; } @@ -502,7 +509,7 @@ void GstVideoReceiver::startDecoding(void *sink) _ensureVideoSinkInPipeline(); if (!_addDecoder(_decoderValve)) { - qCCritical(GstVideoReceiverLog) << "_addDecoder() failed" << _uri; + qCCritical(GstVideoReceiverLog) << "_addDecoder() failed" << _redactedUri(); _shutdownDecodingBranch(); emit onStartDecodingComplete(STATUS_FAIL); return; @@ -512,7 +519,7 @@ void GstVideoReceiver::startDecoding(void *sink) "drop", FALSE, nullptr); - qCDebug(GstVideoReceiverLog) << "Decoding started" << _uri; + qCDebug(GstVideoReceiverLog) << "Decoding started" << _redactedUri(); emit onStartDecodingComplete(STATUS_OK); } @@ -524,14 +531,14 @@ void GstVideoReceiver::stopDecoding() return; } - qCDebug(GstVideoReceiverLog) << "Stopping decoding" << _uri; + qCDebug(GstVideoReceiverLog) << "Stopping decoding" << _redactedUri(); // Gate on _videoSink (set by startDecoding) instead of _decoding (which only flips on // first sink-buffer probe). Without this, stopDecoding() called between // onStartDecodingComplete(OK) and the first frame returns STATUS_INVALID_STATE and // leaves the decoder/sink branch live. if (!_pipeline || !_videoSink) { - qCDebug(GstVideoReceiverLog) << "Not decoding!" << _uri; + qCDebug(GstVideoReceiverLog) << "Not decoding!" << _redactedUri(); emit onStopDecodingComplete(STATUS_INVALID_STATE); return; } @@ -557,25 +564,25 @@ void GstVideoReceiver::startRecording(const QString &videoFile, FILE_FORMAT form return; } - qCDebug(GstVideoReceiverLog) << "Starting recording" << _uri; + qCDebug(GstVideoReceiverLog) << "Starting recording" << _redactedUri(); if (!_pipeline) { - qCDebug(GstVideoReceiverLog) << "Streaming is not active!" << _uri; + qCDebug(GstVideoReceiverLog) << "Streaming is not active!" << _redactedUri(); emit onStartRecordingComplete(STATUS_INVALID_STATE); return; } if (_recording) { - qCDebug(GstVideoReceiverLog) << "Already recording!" << _uri; + qCDebug(GstVideoReceiverLog) << "Already recording!" << _redactedUri(); emit onStartRecordingComplete(STATUS_INVALID_STATE); return; } - qCDebug(GstVideoReceiverLog) << "New video file:" << videoFile << _uri; + qCDebug(GstVideoReceiverLog) << "New video file:" << videoFile << _redactedUri(); GstPad* probepad = gst_element_get_static_pad(_recorderValve, "src"); if (!probepad) { - qCCritical(GstVideoReceiverLog) << "gst_element_get_static_pad() failed" << _uri; + qCCritical(GstVideoReceiverLog) << "gst_element_get_static_pad() failed" << _redactedUri(); emit onStartRecordingComplete(STATUS_FAIL); return; } @@ -651,7 +658,7 @@ void GstVideoReceiver::startRecording(const QString &videoFile, FILE_FORMAT form _recordingOutput = videoFile; _recording = true; - qCDebug(GstVideoReceiverLog) << "Recording started" << _uri; + qCDebug(GstVideoReceiverLog) << "Recording started" << _redactedUri(); emit onStartRecordingComplete(STATUS_OK); emit recordingChanged(_recording); } @@ -663,10 +670,10 @@ void GstVideoReceiver::stopRecording() return; } - qCDebug(GstVideoReceiverLog) << "Stopping recording" << _uri; + qCDebug(GstVideoReceiverLog) << "Stopping recording" << _redactedUri(); if (!_pipeline || !_recording) { - qCDebug(GstVideoReceiverLog) << "Not recording!" << _uri; + qCDebug(GstVideoReceiverLog) << "Not recording!" << _redactedUri(); emit onStopRecordingComplete(STATUS_INVALID_STATE); return; } @@ -696,7 +703,7 @@ void GstVideoReceiver::takeScreenshot(const QString &imageFile) return; } - qCDebug(GstVideoReceiverLog) << "taking screenshot" << _uri; + qCDebug(GstVideoReceiverLog) << "taking screenshot" << _redactedUri(); // FIXME: record screenshot here emit onTakeScreenshotComplete(STATUS_NOT_IMPLEMENTED); @@ -719,7 +726,7 @@ void GstVideoReceiver::_watchdog() if (++_statsTickCounter >= 10) { _statsTickCounter = 0; if (const HwBuffers::PathStats hwStats = HwBuffers::formatPathStats(false); hwStats.totalDelivered > 0) { - qCDebug(GstVideoReceiverLog).noquote() << "HW path live" << _uri << hwStats.line; + qCDebug(GstVideoReceiverLog).noquote() << "HW path live" << _redactedUri() << hwStats.line; } } @@ -730,7 +737,7 @@ void GstVideoReceiver::_watchdog() qint64 elapsed = now - lastSourceFrameTime; if (elapsed > _timeout) { - qCDebug(GstVideoReceiverLog) << "Stream timeout, no frames for" << elapsed << _uri; + qCDebug(GstVideoReceiverLog) << "Stream timeout, no frames for" << elapsed << _redactedUri(); GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-watchdog-timeout"); emit timeout(); _scheduleReconnect("source watchdog"); @@ -746,7 +753,7 @@ void GstVideoReceiver::_watchdog() elapsed = now - lastVideoFrameTime; if (elapsed > (_timeout * 2)) { - qCDebug(GstVideoReceiverLog) << "Video decoder timeout, no frames for" << elapsed << _uri; + qCDebug(GstVideoReceiverLog) << "Video decoder timeout, no frames for" << elapsed << _redactedUri(); GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-watchdog-timeout"); emit timeout(); _scheduleReconnect("decoder watchdog"); @@ -787,7 +794,8 @@ void GstVideoReceiver::_scheduleReconnect(const char *reason) const quint64 epoch = _reconnectEpoch.load(std::memory_order_relaxed); const int attempts = next; qCInfo(GstVideoReceiverLog) << "Scheduling reconnect #" << attempts - << "in" << delaySec << "s after" << reason << uri; + << "in" << delaySec << "s after" << reason + << QGCNetworkHelper::redactedUrlForLogging(uri); QTimer::singleShot(delaySec * 1000, this, [this, epoch, attempts, reconnectTimeout, uri]() { if (epoch != _reconnectEpoch.load(std::memory_order_relaxed)) return; // superseded by stop() // _pipeline is mutated by the worker under _pipelineMutex; a bare deref here (GUI @@ -796,7 +804,8 @@ void GstVideoReceiver::_scheduleReconnect(const char *reason) const bool pipelineUp = (livePipeline != nullptr); if (livePipeline) gst_object_unref(livePipeline); if (uri.isEmpty() || pipelineUp) return; // pipeline already came back - qCInfo(GstVideoReceiverLog) << "Reconnecting (attempt" << attempts << ")" << uri; + qCInfo(GstVideoReceiverLog) << "Reconnecting (attempt" << attempts << ")" + << QGCNetworkHelper::redactedUrlForLogging(uri); start(reconnectTimeout); }); }, Qt::QueuedConnection); @@ -996,7 +1005,7 @@ void GstVideoReceiver::_onNewSourcePad(GstPad *pad) if (!_streaming) { _streaming = true; - qCDebug(GstVideoReceiverLog) << "Streaming started" << _uri; + qCDebug(GstVideoReceiverLog) << "Streaming started" << _redactedUri(); emit streamingChanged(_streaming); } @@ -1023,7 +1032,7 @@ void GstVideoReceiver::_onNewSourcePad(GstPad *pad) "drop", FALSE, nullptr); - qCDebug(GstVideoReceiverLog) << "Decoding started" << _uri; + qCDebug(GstVideoReceiverLog) << "Decoding started" << _redactedUri(); } void GstVideoReceiver::_logDecodebin3SelectedCodec(GstElement *decodebin3) @@ -1083,7 +1092,7 @@ void GstVideoReceiver::_logDecodebin3SelectedCodec(GstElement *decodebin3) void GstVideoReceiver::_onNewDecoderPad(GstPad *pad) { - qCDebug(GstVideoReceiverLog) << "_onNewDecoderPad" << _uri; + qCDebug(GstVideoReceiverLog) << "_onNewDecoderPad" << _redactedUri(); GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-with-new-decoder-pad"); @@ -1189,7 +1198,8 @@ bool GstVideoReceiver::_addVideoSink(GstPad *pad) QSize videoSize; do { if (!_decoderValve) { - qCCritical(GstVideoReceiverLog) << "Unable to determine video size - _decoderValve is NULL" << _uri; + qCCritical(GstVideoReceiverLog) << "Unable to determine video size - _decoderValve is NULL" + << _redactedUri(); break; } @@ -1208,7 +1218,8 @@ bool GstVideoReceiver::_addVideoSink(GstPad *pad) const GstStructure *structure = gst_caps_get_structure(valveSrcPadCaps, 0); if (!structure) { - qCCritical(GstVideoReceiverLog) << "Unable to determine video size - structure is NULL" << _uri; + qCCritical(GstVideoReceiverLog) << "Unable to determine video size - structure is NULL" + << _redactedUri(); gst_clear_object(&valveSrcPad); break; } @@ -1254,10 +1265,10 @@ void GstVideoReceiver::_noteTeeFrame() } const quint64 sourceFrames = _sourceFrameCount.fetch_add(1, std::memory_order_relaxed) + 1; if (sourceFrames == 1) { - qCInfo(GstVideoReceiverLog).noquote() << "Source receiving frames (tee):" << _uri; + qCInfo(GstVideoReceiverLog).noquote() << "Source receiving frames (tee):" << _redactedUri(); } else if ((sourceFrames % 300) == 0) { qCDebug(GstVideoReceiverLog).noquote() - << "Source flow: teeFrames=" << sourceFrames << "decoding=" << _decoding << _uri; + << "Source flow: teeFrames=" << sourceFrames << "decoding=" << _decoding << _redactedUri(); } } @@ -1596,7 +1607,7 @@ gboolean GstVideoReceiver::_onBusMessage(GstBus * /* bus */, GstMessage *msg, gp gst_query_unref(q); const QString decName = pThis->decoderName(); qCDebug(GstVideoReceiverLog).noquote() - << "Pipeline PLAYING:" << pThis->_uri + << "Pipeline PLAYING:" << pThis->_redactedUri() << "decoder:" << (decName.isEmpty() ? QStringLiteral("(pending)") : decName) << "min-latency:" << (min / 1000000) << "ms" << "max-latency:" << (max / 1000000) << "ms"; diff --git a/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.h b/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.h index d31621959600..d95ba7f292a3 100644 --- a/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.h +++ b/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.h @@ -89,6 +89,7 @@ private slots: private: friend class GStreamerTest; + QString _redactedUri() const; GstElement *_makeDecoder(); static GstElement* _makeFileSink(const QString& videoFile, FILE_FORMAT format, const GstCaps* inputCaps); diff --git a/test/Utilities/Network/QGCNetworkHelperTest.cc b/test/Utilities/Network/QGCNetworkHelperTest.cc index 589b8fb8cc24..95f5659572ef 100644 --- a/test/Utilities/Network/QGCNetworkHelperTest.cc +++ b/test/Utilities/Network/QGCNetworkHelperTest.cc @@ -343,6 +343,25 @@ void QGCNetworkHelperTest::_testUrlWithoutQuery() QVERIFY(result.fragment().isEmpty()); } +void QGCNetworkHelperTest::_testRedactedUrlForLogging() +{ + const QString sensitiveUrl = + QStringLiteral("https://pilot:secret@example.com:8443/video%20feed?token=abc123#session"); + const QString redacted = QGCNetworkHelper::redactedUrlForLogging(sensitiveUrl); + + QCOMPARE(redacted, QStringLiteral("https://example.com:8443/")); + QVERIFY(!redacted.contains(QStringLiteral("pilot"))); + QVERIFY(!redacted.contains(QStringLiteral("secret"))); + QVERIFY(!redacted.contains(QStringLiteral("abc123"))); + QVERIFY(!redacted.contains(QStringLiteral("video"))); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("udp://0.0.0.0:5600")), + QStringLiteral("udp://0.0.0.0:5600")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("https://example.com/?token=abc123")), + QStringLiteral("https://example.com/")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("token-only-value")), + QStringLiteral("")); +} + // ============================================================================ // Request Configuration Tests // ============================================================================ diff --git a/test/Utilities/Network/QGCNetworkHelperTest.h b/test/Utilities/Network/QGCNetworkHelperTest.h index 4b14265c6178..c61eb1e1a8f2 100644 --- a/test/Utilities/Network/QGCNetworkHelperTest.h +++ b/test/Utilities/Network/QGCNetworkHelperTest.h @@ -38,6 +38,7 @@ private slots: void _testBuildUrlFromMap(); void _testBuildUrlFromList(); void _testUrlWithoutQuery(); + void _testRedactedUrlForLogging(); // Request configuration tests void _testDefaultUserAgent(); From 0cc1840ddb27bdf64b55561b0c80ebca5e025856 Mon Sep 17 00:00:00 2001 From: alireza787b Date: Tue, 28 Jul 2026 06:09:54 +0000 Subject: [PATCH 02/11] test(Utilities): cover redacted URL edge cases --- test/Utilities/Network/QGCNetworkHelperTest.cc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/Utilities/Network/QGCNetworkHelperTest.cc b/test/Utilities/Network/QGCNetworkHelperTest.cc index 95f5659572ef..d9be2807cd77 100644 --- a/test/Utilities/Network/QGCNetworkHelperTest.cc +++ b/test/Utilities/Network/QGCNetworkHelperTest.cc @@ -321,7 +321,8 @@ void QGCNetworkHelperTest::_testBuildUrlFromMap() void QGCNetworkHelperTest::_testBuildUrlFromList() { QList> params = { - {"key1", "value1"}, {"key1", "value2"}, // Duplicate key allowed with list + {"key1", "value1"}, + {"key1", "value2"}, // Duplicate key allowed with list }; QUrl url = QGCNetworkHelper::buildUrl("http://example.com/api", params); QVERIFY(url.isValid()); @@ -358,6 +359,14 @@ void QGCNetworkHelperTest::_testRedactedUrlForLogging() QStringLiteral("udp://0.0.0.0:5600")); QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("https://example.com/?token=abc123")), QStringLiteral("https://example.com/")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("https://[2001:db8::1]:8443/video?token=abc123")), + QStringLiteral("https://[2001:db8::1]:8443/")); + + const QString encodedUserInfo = QGCNetworkHelper::redactedUrlForLogging( + QStringLiteral("https://pilot%40ops:secret%2Fvalue@example.com/video?token=abc123")); + QCOMPARE(encodedUserInfo, QStringLiteral("https://example.com/")); + QVERIFY(!encodedUserInfo.contains(QStringLiteral("pilot"))); + QVERIFY(!encodedUserInfo.contains(QStringLiteral("secret"))); QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("token-only-value")), QStringLiteral("")); } From 068cf729fa911851342a847974142aa73698704b Mon Sep 17 00:00:00 2001 From: alireza787b Date: Tue, 28 Jul 2026 09:38:34 +0000 Subject: [PATCH 03/11] test(Utilities): run URL redaction checks in CI --- test/Utilities/Network/CMakeLists.txt | 3 ++ .../Utilities/Network/QGCNetworkHelperTest.cc | 27 ----------------- test/Utilities/Network/QGCNetworkHelperTest.h | 1 - .../Network/QGCNetworkRedactionTest.cc | 30 +++++++++++++++++++ .../Network/QGCNetworkRedactionTest.h | 11 +++++++ 5 files changed, 44 insertions(+), 28 deletions(-) create mode 100644 test/Utilities/Network/QGCNetworkRedactionTest.cc create mode 100644 test/Utilities/Network/QGCNetworkRedactionTest.h diff --git a/test/Utilities/Network/CMakeLists.txt b/test/Utilities/Network/CMakeLists.txt index af4c50eba070..ce0c1a5d072e 100644 --- a/test/Utilities/Network/CMakeLists.txt +++ b/test/Utilities/Network/CMakeLists.txt @@ -7,8 +7,11 @@ target_sources(${CMAKE_PROJECT_NAME} PRIVATE QGCNetworkHelperTest.cc QGCNetworkHelperTest.h + QGCNetworkRedactionTest.cc + QGCNetworkRedactionTest.h ) target_include_directories(${CMAKE_PROJECT_NAME} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) add_qgc_test(QGCNetworkHelperTest LABELS Unit Utilities Network) +add_qgc_test(QGCNetworkRedactionTest LABELS Unit Utilities) diff --git a/test/Utilities/Network/QGCNetworkHelperTest.cc b/test/Utilities/Network/QGCNetworkHelperTest.cc index d9be2807cd77..481d627928d5 100644 --- a/test/Utilities/Network/QGCNetworkHelperTest.cc +++ b/test/Utilities/Network/QGCNetworkHelperTest.cc @@ -344,33 +344,6 @@ void QGCNetworkHelperTest::_testUrlWithoutQuery() QVERIFY(result.fragment().isEmpty()); } -void QGCNetworkHelperTest::_testRedactedUrlForLogging() -{ - const QString sensitiveUrl = - QStringLiteral("https://pilot:secret@example.com:8443/video%20feed?token=abc123#session"); - const QString redacted = QGCNetworkHelper::redactedUrlForLogging(sensitiveUrl); - - QCOMPARE(redacted, QStringLiteral("https://example.com:8443/")); - QVERIFY(!redacted.contains(QStringLiteral("pilot"))); - QVERIFY(!redacted.contains(QStringLiteral("secret"))); - QVERIFY(!redacted.contains(QStringLiteral("abc123"))); - QVERIFY(!redacted.contains(QStringLiteral("video"))); - QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("udp://0.0.0.0:5600")), - QStringLiteral("udp://0.0.0.0:5600")); - QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("https://example.com/?token=abc123")), - QStringLiteral("https://example.com/")); - QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("https://[2001:db8::1]:8443/video?token=abc123")), - QStringLiteral("https://[2001:db8::1]:8443/")); - - const QString encodedUserInfo = QGCNetworkHelper::redactedUrlForLogging( - QStringLiteral("https://pilot%40ops:secret%2Fvalue@example.com/video?token=abc123")); - QCOMPARE(encodedUserInfo, QStringLiteral("https://example.com/")); - QVERIFY(!encodedUserInfo.contains(QStringLiteral("pilot"))); - QVERIFY(!encodedUserInfo.contains(QStringLiteral("secret"))); - QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("token-only-value")), - QStringLiteral("")); -} - // ============================================================================ // Request Configuration Tests // ============================================================================ diff --git a/test/Utilities/Network/QGCNetworkHelperTest.h b/test/Utilities/Network/QGCNetworkHelperTest.h index c61eb1e1a8f2..4b14265c6178 100644 --- a/test/Utilities/Network/QGCNetworkHelperTest.h +++ b/test/Utilities/Network/QGCNetworkHelperTest.h @@ -38,7 +38,6 @@ private slots: void _testBuildUrlFromMap(); void _testBuildUrlFromList(); void _testUrlWithoutQuery(); - void _testRedactedUrlForLogging(); // Request configuration tests void _testDefaultUserAgent(); diff --git a/test/Utilities/Network/QGCNetworkRedactionTest.cc b/test/Utilities/Network/QGCNetworkRedactionTest.cc new file mode 100644 index 000000000000..5c63b0ee483b --- /dev/null +++ b/test/Utilities/Network/QGCNetworkRedactionTest.cc @@ -0,0 +1,30 @@ +#include "QGCNetworkRedactionTest.h" + +#include "QGCNetworkHelper.h" + +void QGCNetworkRedactionTest::_testRedactedUrlForLogging() +{ + const QString sensitiveUrl = + QStringLiteral("https://pilot:secret@example.com:8443/video%20feed?token=abc123#session"); + const QString redacted = QGCNetworkHelper::redactedUrlForLogging(sensitiveUrl); + + QCOMPARE(redacted, QStringLiteral("https://example.com:8443/")); + QVERIFY(!redacted.contains(QStringLiteral("pilot"))); + QVERIFY(!redacted.contains(QStringLiteral("secret"))); + QVERIFY(!redacted.contains(QStringLiteral("abc123"))); + QVERIFY(!redacted.contains(QStringLiteral("video"))); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("udp://0.0.0.0:5600")), + QStringLiteral("udp://0.0.0.0:5600")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("https://example.com/?token=abc123")), + QStringLiteral("https://example.com/")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("https://[2001:db8::1]:8443/video?token=abc123")), + QStringLiteral("https://[2001:db8::1]:8443/")); + + const QString encodedUserInfo = QGCNetworkHelper::redactedUrlForLogging( + QStringLiteral("https://pilot%40ops:secret%2Fvalue@example.com/video?token=abc123")); + QCOMPARE(encodedUserInfo, QStringLiteral("https://example.com/")); + QVERIFY(!encodedUserInfo.contains(QStringLiteral("pilot"))); + QVERIFY(!encodedUserInfo.contains(QStringLiteral("secret"))); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("token-only-value")), + QStringLiteral("")); +} diff --git a/test/Utilities/Network/QGCNetworkRedactionTest.h b/test/Utilities/Network/QGCNetworkRedactionTest.h new file mode 100644 index 000000000000..05a206d83514 --- /dev/null +++ b/test/Utilities/Network/QGCNetworkRedactionTest.h @@ -0,0 +1,11 @@ +#pragma once + +#include "UnitTest.h" + +class QGCNetworkRedactionTest : public UnitTest +{ + Q_OBJECT + +private slots: + void _testRedactedUrlForLogging(); +}; From 6ffe7b08315ff3cb6ca9a8742015746b9172bf74 Mon Sep 17 00:00:00 2001 From: alireza787b Date: Tue, 28 Jul 2026 09:41:56 +0000 Subject: [PATCH 04/11] style(Utilities): keep redaction diff focused --- test/Utilities/Network/QGCNetworkHelperTest.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/Utilities/Network/QGCNetworkHelperTest.cc b/test/Utilities/Network/QGCNetworkHelperTest.cc index 481d627928d5..589b8fb8cc24 100644 --- a/test/Utilities/Network/QGCNetworkHelperTest.cc +++ b/test/Utilities/Network/QGCNetworkHelperTest.cc @@ -321,8 +321,7 @@ void QGCNetworkHelperTest::_testBuildUrlFromMap() void QGCNetworkHelperTest::_testBuildUrlFromList() { QList> params = { - {"key1", "value1"}, - {"key1", "value2"}, // Duplicate key allowed with list + {"key1", "value1"}, {"key1", "value2"}, // Duplicate key allowed with list }; QUrl url = QGCNetworkHelper::buildUrl("http://example.com/api", params); QVERIFY(url.isValid()); From 4a2d907c602141d890bee0794023562be407751e Mon Sep 17 00:00:00 2001 From: alireza787b Date: Tue, 28 Jul 2026 10:11:32 +0000 Subject: [PATCH 05/11] test(Utilities): register URL redaction test --- test/Utilities/Network/QGCNetworkRedactionTest.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Utilities/Network/QGCNetworkRedactionTest.cc b/test/Utilities/Network/QGCNetworkRedactionTest.cc index 5c63b0ee483b..79886bfd48ef 100644 --- a/test/Utilities/Network/QGCNetworkRedactionTest.cc +++ b/test/Utilities/Network/QGCNetworkRedactionTest.cc @@ -28,3 +28,5 @@ void QGCNetworkRedactionTest::_testRedactedUrlForLogging() QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("token-only-value")), QStringLiteral("")); } + +UT_REGISTER_TEST(QGCNetworkRedactionTest, TestLabel::Unit, TestLabel::Utilities) From aff1d9bf226f9b104ec5dfc8c8fbc010401f06e8 Mon Sep 17 00:00:00 2001 From: alireza787b Date: Fri, 31 Jul 2026 01:15:43 +0000 Subject: [PATCH 06/11] fix(VideoManager): preserve useful stream diagnostics --- src/Camera/VehicleCameraControl.cc | 6 +- src/Settings/VideoSettings.cc | 25 ++++++--- src/Utilities/Network/QGCNetworkHelper.cc | 33 +++++++---- src/Utilities/Network/QGCNetworkHelper.h | 4 +- .../GStreamer/GStreamerHelpers.cc | 2 +- .../GStreamer/GStreamerHelpers.h | 4 ++ .../GStreamer/GstSourceFactory.cc | 9 ++- .../GStreamer/GstVideoReceiver.cc | 27 ++++----- .../VideoReceiver/GStreamer/README.md | 2 + test/Utilities/Network/CMakeLists.txt | 3 - .../Utilities/Network/QGCNetworkHelperTest.cc | 56 +++++++++++++++++++ test/Utilities/Network/QGCNetworkHelperTest.h | 5 ++ .../Network/QGCNetworkRedactionTest.cc | 32 ----------- .../Network/QGCNetworkRedactionTest.h | 11 ---- test/VideoManager/GStreamer/GStreamerTest.cc | 28 ++++++++++ test/VideoManager/GStreamer/GStreamerTest.h | 1 + 16 files changed, 159 insertions(+), 89 deletions(-) delete mode 100644 test/Utilities/Network/QGCNetworkRedactionTest.cc delete mode 100644 test/Utilities/Network/QGCNetworkRedactionTest.h diff --git a/src/Camera/VehicleCameraControl.cc b/src/Camera/VehicleCameraControl.cc index ede5743095ab..907a9efc94dd 100644 --- a/src/Camera/VehicleCameraControl.cc +++ b/src/Camera/VehicleCameraControl.cc @@ -1817,7 +1817,8 @@ void VehicleCameraControl::setCurrentStream(int stream) if (stream != _currentStream && stream >= 0 && stream < _streamLabels.count()) { QGCVideoStreamInfo* pInfo = currentStreamInstance(); if(pInfo) { - qCDebug(VehicleCameraControlLog) << "Stopping stream:" << pInfo->uri(); + qCDebug(VehicleCameraControlLog) + << "Stopping stream:" << QGCNetworkHelper::redactedUrlForLogging(pInfo->uri()); //-- Stop current stream _vehicle->sendMavCommand( _compID, // Target component @@ -1829,7 +1830,8 @@ void VehicleCameraControl::setCurrentStream(int stream) pInfo = currentStreamInstance(); if(pInfo) { //-- Start new stream - qCDebug(VehicleCameraControlLog) << "Starting stream:" << pInfo->uri(); + qCDebug(VehicleCameraControlLog) + << "Starting stream:" << QGCNetworkHelper::redactedUrlForLogging(pInfo->uri()); _vehicle->sendMavCommand( _compID, // Target component MAV_CMD_VIDEO_START_STREAMING, // Command id diff --git a/src/Settings/VideoSettings.cc b/src/Settings/VideoSettings.cc index c4f799e613af..ffe9767c42c3 100644 --- a/src/Settings/VideoSettings.cc +++ b/src/Settings/VideoSettings.cc @@ -2,6 +2,7 @@ #include "VideoManager.h" #include "QGCLoggingCategory.h" +#include "QGCNetworkHelper.h" #include #include @@ -243,23 +244,31 @@ bool VideoSettings::streamConfigured(void) } //-- If UDP, check for URL if(vSource == videoSourceUDPH264 || vSource == videoSourceUDPH265) { - qCDebug(VideoSettingsLog) << "Testing configuration for UDP Stream"; - return !udpUrl()->rawValue().toString().isEmpty(); + const QString url = udpUrl()->rawValue().toString(); + qCDebug(VideoSettingsLog) << "Testing configuration for UDP Stream:" + << QGCNetworkHelper::redactedUrlForLogging(url); + return !url.isEmpty(); } //-- If RTSP, check for URL if(vSource == videoSourceRTSP) { - qCDebug(VideoSettingsLog) << "Testing configuration for RTSP Stream"; - return !rtspUrl()->rawValue().toString().isEmpty(); + const QString url = rtspUrl()->rawValue().toString(); + qCDebug(VideoSettingsLog) << "Testing configuration for RTSP Stream:" + << QGCNetworkHelper::redactedUrlForLogging(url); + return !url.isEmpty(); } //-- If TCP, check for URL if(vSource == videoSourceTCP) { - qCDebug(VideoSettingsLog) << "Testing configuration for TCP Stream"; - return !tcpUrl()->rawValue().toString().isEmpty(); + const QString url = tcpUrl()->rawValue().toString(); + qCDebug(VideoSettingsLog) << "Testing configuration for TCP Stream:" + << QGCNetworkHelper::redactedUrlForLogging(url); + return !url.isEmpty(); } //-- If MPEG-TS, check for URL if(vSource == videoSourceMPEGTS) { - qCDebug(VideoSettingsLog) << "Testing configuration for MPEG-TS Stream"; - return !udpUrl()->rawValue().toString().isEmpty(); + const QString url = udpUrl()->rawValue().toString(); + qCDebug(VideoSettingsLog) << "Testing configuration for MPEG-TS Stream:" + << QGCNetworkHelper::redactedUrlForLogging(url); + return !url.isEmpty(); } //-- If Herelink Air unit, good to go if(vSource == videoSourceHerelinkAirUnit) { diff --git a/src/Utilities/Network/QGCNetworkHelper.cc b/src/Utilities/Network/QGCNetworkHelper.cc index 17046bd960cc..0478068fdbb0 100644 --- a/src/Utilities/Network/QGCNetworkHelper.cc +++ b/src/Utilities/Network/QGCNetworkHelper.cc @@ -344,22 +344,31 @@ QUrl urlWithoutQuery(const QUrl& url) QString redactedUrlForLogging(const QUrl& url) { - if (!url.isValid() || url.scheme().isEmpty()) { + if (url.isEmpty()) { + return QStringLiteral(""); + } + if (!url.isValid()) { return QStringLiteral(""); } - QUrl redactedUrl(url); - const bool hadPath = !redactedUrl.path().isEmpty() && (redactedUrl.path() != QLatin1String("/")); - redactedUrl.setUserInfo(QString()); - redactedUrl.setPath(hadPath ? QString() : redactedUrl.path()); - redactedUrl.setQuery(QString()); - redactedUrl.setFragment(QString()); - - QString displayUrl = redactedUrl.toDisplayString(QUrl::FullyEncoded); - if (hadPath) { - displayUrl += QStringLiteral("/"); + QUrl redactedUrl = url.adjusted(QUrl::RemoveUserInfo); + if (redactedUrl.hasQuery()) { + const auto queryItems = QUrlQuery(redactedUrl).queryItems(QUrl::FullyDecoded); + QUrlQuery redactedQuery; + for (const auto& queryItem : queryItems) { + redactedQuery.addQueryItem(queryItem.first, QStringLiteral("REDACTED")); + } + if (queryItems.isEmpty()) { + redactedUrl.setQuery(QStringLiteral("REDACTED")); + } else { + redactedUrl.setQuery(redactedQuery); + } + } + if (redactedUrl.hasFragment()) { + redactedUrl.setFragment(QStringLiteral("REDACTED")); } - return displayUrl; + + return redactedUrl.toDisplayString(QUrl::FullyEncoded); } QString redactedUrlForLogging(const QString& url) diff --git a/src/Utilities/Network/QGCNetworkHelper.h b/src/Utilities/Network/QGCNetworkHelper.h index 1d3faf7aeaf2..e83571e20c74 100644 --- a/src/Utilities/Network/QGCNetworkHelper.h +++ b/src/Utilities/Network/QGCNetworkHelper.h @@ -139,8 +139,8 @@ QUrl buildUrl(const QString& baseUrl, const QList>& para /// Get URL without query string and fragment QUrl urlWithoutQuery(const QUrl& url); -/// Return a URL suitable for diagnostics without user info, path, query, or fragment secrets. -/// Invalid and relative input is not echoed. +/// Return a URL suitable for diagnostics. Stream identity is preserved while user info, +/// query values, and fragment content are redacted. QString redactedUrlForLogging(const QUrl& url); QString redactedUrlForLogging(const QString& url); diff --git a/src/VideoManager/VideoReceiver/GStreamer/GStreamerHelpers.cc b/src/VideoManager/VideoReceiver/GStreamer/GStreamerHelpers.cc index 6176704bfd69..c20378ece58a 100644 --- a/src/VideoManager/VideoReceiver/GStreamer/GStreamerHelpers.cc +++ b/src/VideoManager/VideoReceiver/GStreamer/GStreamerHelpers.cc @@ -82,7 +82,7 @@ QString writePipelineDot(GstElement* pipeline, const char* tag) QFile::remove(existing.takeFirst().absoluteFilePath()); } - gchar* data = gst_debug_bin_to_dot_data(GST_BIN(pipeline), GST_DEBUG_GRAPH_SHOW_ALL); + gchar* data = gst_debug_bin_to_dot_data(GST_BIN(pipeline), kDiagnosticDotGraphDetails); if (!data) return {}; const QString fileName = QStringLiteral("%1-%2.dot") diff --git a/src/VideoManager/VideoReceiver/GStreamer/GStreamerHelpers.h b/src/VideoManager/VideoReceiver/GStreamer/GStreamerHelpers.h index 568884a98985..2bc6d87a93ca 100644 --- a/src/VideoManager/VideoReceiver/GStreamer/GStreamerHelpers.h +++ b/src/VideoManager/VideoReceiver/GStreamer/GStreamerHelpers.h @@ -9,6 +9,10 @@ #include "GStreamer.h" // VideoDecoderOptions namespace GStreamer { +/// Diagnostic graphs omit element properties because source properties can contain credentials. +inline constexpr GstDebugGraphDetails kDiagnosticDotGraphDetails = static_cast( + GST_DEBUG_GRAPH_SHOW_MEDIA_TYPE | GST_DEBUG_GRAPH_SHOW_CAPS_DETAILS | GST_DEBUG_GRAPH_SHOW_STATES); + bool isValidRtspUri(const gchar* uri_str); /// Dump @p pipeline's graph as a rotating .dot under CacheLocation/qgc-pipeline-dot/ for field reports. diff --git a/src/VideoManager/VideoReceiver/GStreamer/GstSourceFactory.cc b/src/VideoManager/VideoReceiver/GStreamer/GstSourceFactory.cc index 04163fa862e8..0ed15d6b3128 100644 --- a/src/VideoManager/VideoReceiver/GStreamer/GstSourceFactory.cc +++ b/src/VideoManager/VideoReceiver/GStreamer/GstSourceFactory.cc @@ -296,8 +296,7 @@ void linkPad(GstElement* element, GstPad* pad, gpointer data) GstElement* buildRtspSource(const QString& uri, const QUrl& sourceUrl, const Config& config, guint latencyMs) { if (!GStreamer::isValidRtspUri(uri.toUtf8().constData())) { - qCCritical(GstSourceFactoryLog) << "Invalid RTSP URI:" - << QGCNetworkHelper::redactedUrlForLogging(sourceUrl); + qCWarning(GstSourceFactoryLog) << "Invalid RTSP URI:" << QGCNetworkHelper::redactedUrlForLogging(sourceUrl); return nullptr; } @@ -338,13 +337,13 @@ GstElement* buildTcpSource(const QUrl& sourceUrl) { const int port = sourceUrl.port(); if (!validPort(port)) { - qCCritical(GstSourceFactoryLog) << "Invalid TCP port" << port << "in" + qCWarning(GstSourceFactoryLog) << "Invalid TCP port" << port << "in" << QGCNetworkHelper::redactedUrlForLogging(sourceUrl); return nullptr; } const QString host = sourceUrl.host(); if (host.isEmpty()) { - qCCritical(GstSourceFactoryLog) << "Missing host in TCP URI" + qCWarning(GstSourceFactoryLog) << "Missing host in TCP URI" << QGCNetworkHelper::redactedUrlForLogging(sourceUrl); return nullptr; } @@ -363,7 +362,7 @@ GstElement* buildUdpSource(const QUrl& sourceUrl, bool isUdpH264, bool isUdpH265 { const int port = sourceUrl.port(); if (!validPort(port)) { - qCCritical(GstSourceFactoryLog) << "Invalid UDP port" << port << "in" + qCWarning(GstSourceFactoryLog) << "Invalid UDP port" << port << "in" << QGCNetworkHelper::redactedUrlForLogging(sourceUrl); return nullptr; } diff --git a/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.cc b/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.cc index 3a2767e62935..be554a8c05f4 100644 --- a/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.cc +++ b/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.cc @@ -274,7 +274,7 @@ void GstVideoReceiver::start(uint32_t timeout) gst_clear_object(&bus); } - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-initial"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-initial"); running = (gst_element_set_state(_pipeline, GST_STATE_PLAYING) != GST_STATE_CHANGE_FAILURE); } while(0); @@ -298,7 +298,7 @@ void GstVideoReceiver::start(uint32_t timeout) emit onStartComplete(STATUS_FAIL); } else { - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-started"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-started"); qCDebug(GstVideoReceiverLog) << "Started" << _redactedUri(); // _watchdogTimer lives on `this` (GUI thread); the emit runs synchronously on the @@ -416,7 +416,7 @@ void GstVideoReceiver::stop() _shutdownDecodingBranch(); } - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-stopped"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-stopped"); // Lock before nulling so an in-flight _onBusMessage on the streaming thread cannot read // a half-destroyed _pipeline. _acquirePipelineRef takes its own ref under the same lock. @@ -738,7 +738,7 @@ void GstVideoReceiver::_watchdog() qint64 elapsed = now - lastSourceFrameTime; if (elapsed > _timeout) { qCDebug(GstVideoReceiverLog) << "Stream timeout, no frames for" << elapsed << _redactedUri(); - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-watchdog-timeout"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-watchdog-timeout"); emit timeout(); _scheduleReconnect("source watchdog"); return; @@ -754,7 +754,8 @@ void GstVideoReceiver::_watchdog() elapsed = now - lastVideoFrameTime; if (elapsed > (_timeout * 2)) { qCDebug(GstVideoReceiverLog) << "Video decoder timeout, no frames for" << elapsed << _redactedUri(); - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-watchdog-timeout"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, + "pipeline-watchdog-timeout"); emit timeout(); _scheduleReconnect("decoder watchdog"); } @@ -820,7 +821,7 @@ void GstVideoReceiver::dumpPipelineGraph(const QString &tag) return; } const QByteArray tagUtf8 = tag.toUtf8(); - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(pipelineRef), GST_DEBUG_GRAPH_SHOW_ALL, tagUtf8.constData()); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(pipelineRef), GStreamer::kDiagnosticDotGraphDetails, tagUtf8.constData()); const QString dotPath = GStreamer::writePipelineDot(pipelineRef, tagUtf8.constData()); if (!dotPath.isEmpty()) { qCInfo(GstVideoReceiverLog) << "Pipeline graph saved to" << dotPath; @@ -1018,7 +1019,7 @@ void GstVideoReceiver::_onNewSourcePad(GstPad *pad) return; } - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-with-new-source-pad"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-with-new-source-pad"); _ensureVideoSinkInPipeline(); @@ -1094,7 +1095,7 @@ void GstVideoReceiver::_onNewDecoderPad(GstPad *pad) { qCDebug(GstVideoReceiverLog) << "_onNewDecoderPad" << _redactedUri(); - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-with-new-decoder-pad"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-with-new-decoder-pad"); // We should now know what codec decodebin3 selected. _logDecodebin3SelectedCodec(_decoder); @@ -1117,7 +1118,7 @@ bool GstVideoReceiver::_addDecoder(GstElement *src) (void) gst_bin_add(GST_BIN(_pipeline), _decoder); (void) gst_element_sync_state_with_parent(_decoder); - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-with-decoder"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-with-decoder"); if (!gst_element_link(src, _decoder)) { qCCritical(GstVideoReceiverLog) << "Unable to link decoder"; @@ -1192,7 +1193,7 @@ bool GstVideoReceiver::_addVideoSink(GstPad *pad) (void) gst_element_sync_state_with_parent(_videoSink); - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-with-videosink"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-with-videosink"); // Determine video size. Errors here are non-fatal. QSize videoSize; @@ -1377,7 +1378,7 @@ void GstVideoReceiver::_shutdownDecodingBranch() emit decodingChanged(_decoding); } - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-decoding-stopped"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-decoding-stopped"); } void GstVideoReceiver::_shutdownRecordingBranch() @@ -1409,7 +1410,7 @@ void GstVideoReceiver::_shutdownRecordingBranch() emit onStopRecordingComplete(STATUS_OK); } - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-recording-stopped"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-recording-stopped"); } bool GstVideoReceiver::_needDispatch() @@ -1468,7 +1469,7 @@ gboolean GstVideoReceiver::_onBusMessage(GstBus * /* bus */, GstMessage *msg, gp if (GstElement *pipelineRef = pThis->_acquirePipelineRef()) { // Native dump path (no-op without GST_DEBUG_DUMP_DOT_DIR) plus an unconditional // CacheLocation fallback so field-bug-report bundles include pipeline topology. - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(pipelineRef), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-error"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(pipelineRef), GStreamer::kDiagnosticDotGraphDetails, "pipeline-error"); const QString dotPath = GStreamer::writePipelineDot(pipelineRef, "pipeline-error"); if (!dotPath.isEmpty()) { qCInfo(GstVideoReceiverLog) << "Pipeline graph saved to" << dotPath; diff --git a/src/VideoManager/VideoReceiver/GStreamer/README.md b/src/VideoManager/VideoReceiver/GStreamer/README.md index f7f6504bf17a..2898166842de 100644 --- a/src/VideoManager/VideoReceiver/GStreamer/README.md +++ b/src/VideoManager/VideoReceiver/GStreamer/README.md @@ -137,6 +137,8 @@ dot -Tpng /tmp/qgc-pipeline-dots/0.00.00.*-pipeline-started.dot -o pipeline.png When the env var is **unset**, QGC still writes a rotating snapshot (≤10 files) to `/qgc-pipeline-dot/-.dot` on `ERROR` and on watchdog timeout, so field-bug-report bundles include the topology automatically. The `GstVideoReceiver::dumpPipelineGraph(tag)` slot (callable from QML) writes a snapshot on demand for use from a debug menu. +QGC graph dumps include topology, caps, media types, and states. Element property values are omitted because source properties can contain stream credentials. + ### Latency tracer Per-element latency from source to sink: diff --git a/test/Utilities/Network/CMakeLists.txt b/test/Utilities/Network/CMakeLists.txt index ce0c1a5d072e..af4c50eba070 100644 --- a/test/Utilities/Network/CMakeLists.txt +++ b/test/Utilities/Network/CMakeLists.txt @@ -7,11 +7,8 @@ target_sources(${CMAKE_PROJECT_NAME} PRIVATE QGCNetworkHelperTest.cc QGCNetworkHelperTest.h - QGCNetworkRedactionTest.cc - QGCNetworkRedactionTest.h ) target_include_directories(${CMAKE_PROJECT_NAME} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) add_qgc_test(QGCNetworkHelperTest LABELS Unit Utilities Network) -add_qgc_test(QGCNetworkRedactionTest LABELS Unit Utilities) diff --git a/test/Utilities/Network/QGCNetworkHelperTest.cc b/test/Utilities/Network/QGCNetworkHelperTest.cc index 589b8fb8cc24..78e830e54170 100644 --- a/test/Utilities/Network/QGCNetworkHelperTest.cc +++ b/test/Utilities/Network/QGCNetworkHelperTest.cc @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -343,6 +344,61 @@ void QGCNetworkHelperTest::_testUrlWithoutQuery() QVERIFY(result.fragment().isEmpty()); } +void QGCNetworkHelperTest::_testRedactedUrlPreservesStreamIdentity() +{ + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("rtsp://camera.example:554/axis-media/media.amp")), + QStringLiteral("rtsp://camera.example:554/axis-media/media.amp")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("udp://0.0.0.0:5600")), + QStringLiteral("udp://0.0.0.0:5600")); +} + +void QGCNetworkHelperTest::_testRedactedUrlRemovesUserInfo() +{ + const QString result = QGCNetworkHelper::redactedUrlForLogging( + QStringLiteral("https://pilot%40ops:secret%2Fvalue@example.com:8443/video%20feed")); + + QCOMPARE(result, QStringLiteral("https://example.com:8443/video%20feed")); + QVERIFY(!result.contains(QStringLiteral("pilot"))); + QVERIFY(!result.contains(QStringLiteral("secret"))); +} + +void QGCNetworkHelperTest::_testRedactedUrlRedactsQueryValues() +{ + const QString result = QGCNetworkHelper::redactedUrlForLogging( + QStringLiteral("https://example.com/video?token=abc123&mode=low-latency#session")); + const QUrl resultUrl(result); + const QUrlQuery resultQuery(resultUrl); + + QCOMPARE(resultUrl.path(), QStringLiteral("/video")); + QCOMPARE(resultQuery.queryItemValue(QStringLiteral("token")), QStringLiteral("REDACTED")); + QCOMPARE(resultQuery.queryItemValue(QStringLiteral("mode")), QStringLiteral("REDACTED")); + QCOMPARE(resultUrl.fragment(), QStringLiteral("REDACTED")); + QVERIFY(!result.contains(QStringLiteral("abc123"))); + QVERIFY(!result.contains(QStringLiteral("low-latency"))); + QVERIFY(!result.contains(QStringLiteral("session"))); +} + +void QGCNetworkHelperTest::_testRedactedUrlHandlesRelativeAndInvalidInput() +{ + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("5600")), QStringLiteral("5600")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("camera.local:5600")), + QStringLiteral("camera.local:5600")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QString()), QStringLiteral("")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("http://[invalid")), + QStringLiteral("")); +} + +void QGCNetworkHelperTest::_testRedactedUrlQUrlOverload() +{ + const QUrl sourceUrl(QStringLiteral("rtsp://pilot:secret@camera.example:8554/live?token=abc123")); + const QString result = QGCNetworkHelper::redactedUrlForLogging(sourceUrl); + + QCOMPARE(QUrl(result).path(), QStringLiteral("/live")); + QVERIFY(!result.contains(QStringLiteral("pilot"))); + QVERIFY(!result.contains(QStringLiteral("secret"))); + QVERIFY(!result.contains(QStringLiteral("abc123"))); +} + // ============================================================================ // Request Configuration Tests // ============================================================================ diff --git a/test/Utilities/Network/QGCNetworkHelperTest.h b/test/Utilities/Network/QGCNetworkHelperTest.h index 4b14265c6178..bef94e99522f 100644 --- a/test/Utilities/Network/QGCNetworkHelperTest.h +++ b/test/Utilities/Network/QGCNetworkHelperTest.h @@ -38,6 +38,11 @@ private slots: void _testBuildUrlFromMap(); void _testBuildUrlFromList(); void _testUrlWithoutQuery(); + void _testRedactedUrlPreservesStreamIdentity(); + void _testRedactedUrlRemovesUserInfo(); + void _testRedactedUrlRedactsQueryValues(); + void _testRedactedUrlHandlesRelativeAndInvalidInput(); + void _testRedactedUrlQUrlOverload(); // Request configuration tests void _testDefaultUserAgent(); diff --git a/test/Utilities/Network/QGCNetworkRedactionTest.cc b/test/Utilities/Network/QGCNetworkRedactionTest.cc deleted file mode 100644 index 79886bfd48ef..000000000000 --- a/test/Utilities/Network/QGCNetworkRedactionTest.cc +++ /dev/null @@ -1,32 +0,0 @@ -#include "QGCNetworkRedactionTest.h" - -#include "QGCNetworkHelper.h" - -void QGCNetworkRedactionTest::_testRedactedUrlForLogging() -{ - const QString sensitiveUrl = - QStringLiteral("https://pilot:secret@example.com:8443/video%20feed?token=abc123#session"); - const QString redacted = QGCNetworkHelper::redactedUrlForLogging(sensitiveUrl); - - QCOMPARE(redacted, QStringLiteral("https://example.com:8443/")); - QVERIFY(!redacted.contains(QStringLiteral("pilot"))); - QVERIFY(!redacted.contains(QStringLiteral("secret"))); - QVERIFY(!redacted.contains(QStringLiteral("abc123"))); - QVERIFY(!redacted.contains(QStringLiteral("video"))); - QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("udp://0.0.0.0:5600")), - QStringLiteral("udp://0.0.0.0:5600")); - QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("https://example.com/?token=abc123")), - QStringLiteral("https://example.com/")); - QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("https://[2001:db8::1]:8443/video?token=abc123")), - QStringLiteral("https://[2001:db8::1]:8443/")); - - const QString encodedUserInfo = QGCNetworkHelper::redactedUrlForLogging( - QStringLiteral("https://pilot%40ops:secret%2Fvalue@example.com/video?token=abc123")); - QCOMPARE(encodedUserInfo, QStringLiteral("https://example.com/")); - QVERIFY(!encodedUserInfo.contains(QStringLiteral("pilot"))); - QVERIFY(!encodedUserInfo.contains(QStringLiteral("secret"))); - QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("token-only-value")), - QStringLiteral("")); -} - -UT_REGISTER_TEST(QGCNetworkRedactionTest, TestLabel::Unit, TestLabel::Utilities) diff --git a/test/Utilities/Network/QGCNetworkRedactionTest.h b/test/Utilities/Network/QGCNetworkRedactionTest.h deleted file mode 100644 index 05a206d83514..000000000000 --- a/test/Utilities/Network/QGCNetworkRedactionTest.h +++ /dev/null @@ -1,11 +0,0 @@ -#pragma once - -#include "UnitTest.h" - -class QGCNetworkRedactionTest : public UnitTest -{ - Q_OBJECT - -private slots: - void _testRedactedUrlForLogging(); -}; diff --git a/test/VideoManager/GStreamer/GStreamerTest.cc b/test/VideoManager/GStreamer/GStreamerTest.cc index 4d4d77713e3f..1c9e3e6f7bd4 100644 --- a/test/VideoManager/GStreamer/GStreamerTest.cc +++ b/test/VideoManager/GStreamer/GStreamerTest.cc @@ -536,6 +536,33 @@ void GStreamerTest::_testWritePipelineDotReturnsEmptyOnWriteFailure() QVERIFY2(path.isEmpty(), qPrintable(QStringLiteral("Expected empty path for failed dot write, got %1").arg(path))); } +void GStreamerTest::_testPipelineDotOmitsElementProperties() +{ + GstElement* pipeline = gst_pipeline_new("safe-dot-test"); + QVERIFY(pipeline); + const auto pipelineCleanup = qScopeGuard([&] { gst_object_unref(pipeline); }); + + GstElement* source = gst_element_factory_make("filesrc", "source"); + GstElement* sink = gst_element_factory_make("fakesink", "sink"); + QVERIFY(source); + QVERIFY(sink); + + constexpr auto kSecretLocation = "/tmp/qgc-dot-secret-token"; + g_object_set(source, "location", kSecretLocation, nullptr); + gst_bin_add_many(GST_BIN(pipeline), source, sink, nullptr); + QVERIFY(gst_element_link(source, sink)); + + gchar* dotData = gst_debug_bin_to_dot_data(GST_BIN(pipeline), GStreamer::kDiagnosticDotGraphDetails); + QVERIFY(dotData); + const QByteArray dot(dotData); + g_free(dotData); + + QVERIFY(dot.contains("source")); + QVERIFY(dot.contains("sink")); + QVERIFY(!dot.contains(kSecretLocation)); + QVERIFY(!dot.contains("qgc-dot-secret-token")); +} + void GStreamerTest::_testCompleteInit() { GStreamer::redirectGLibLogging(); @@ -839,6 +866,7 @@ QGC_GST_SKIP_TEST(_testConfigureDebugLoggingIsIdempotent) QGC_GST_SKIP_TEST(_testVerifyRequiredPlugins) QGC_GST_SKIP_TEST(_testEnvironmentSetup) QGC_GST_SKIP_TEST(_testWritePipelineDotReturnsEmptyOnWriteFailure) +QGC_GST_SKIP_TEST(_testPipelineDotOmitsElementProperties) QGC_GST_SKIP_TEST(_testCompleteInit) QGC_GST_SKIP_TEST(_testCreateVideoReceiver) diff --git a/test/VideoManager/GStreamer/GStreamerTest.h b/test/VideoManager/GStreamer/GStreamerTest.h index a2a3db99ca20..25d5493c5b99 100644 --- a/test/VideoManager/GStreamer/GStreamerTest.h +++ b/test/VideoManager/GStreamer/GStreamerTest.h @@ -22,6 +22,7 @@ private slots: void _testVerifyRequiredPlugins(); void _testEnvironmentSetup(); void _testWritePipelineDotReturnsEmptyOnWriteFailure(); + void _testPipelineDotOmitsElementProperties(); void _testCompleteInit(); void _testCreateVideoReceiver(); void _testRecordingSinkAcceptsElementaryStreams_data(); From 6ab2f5a5cdc47be4d24cad69d23e8ec14889bf45 Mon Sep 17 00:00:00 2001 From: alireza787b Date: Fri, 31 Jul 2026 01:38:46 +0000 Subject: [PATCH 07/11] test(Utilities): run redaction coverage in CI --- test/Utilities/Network/CMakeLists.txt | 3 + .../Utilities/Network/QGCNetworkHelperTest.cc | 55 ---------------- test/Utilities/Network/QGCNetworkHelperTest.h | 5 -- .../Network/QGCNetworkRedactionTest.cc | 63 +++++++++++++++++++ .../Network/QGCNetworkRedactionTest.h | 15 +++++ 5 files changed, 81 insertions(+), 60 deletions(-) create mode 100644 test/Utilities/Network/QGCNetworkRedactionTest.cc create mode 100644 test/Utilities/Network/QGCNetworkRedactionTest.h diff --git a/test/Utilities/Network/CMakeLists.txt b/test/Utilities/Network/CMakeLists.txt index af4c50eba070..ce0c1a5d072e 100644 --- a/test/Utilities/Network/CMakeLists.txt +++ b/test/Utilities/Network/CMakeLists.txt @@ -7,8 +7,11 @@ target_sources(${CMAKE_PROJECT_NAME} PRIVATE QGCNetworkHelperTest.cc QGCNetworkHelperTest.h + QGCNetworkRedactionTest.cc + QGCNetworkRedactionTest.h ) target_include_directories(${CMAKE_PROJECT_NAME} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) add_qgc_test(QGCNetworkHelperTest LABELS Unit Utilities Network) +add_qgc_test(QGCNetworkRedactionTest LABELS Unit Utilities) diff --git a/test/Utilities/Network/QGCNetworkHelperTest.cc b/test/Utilities/Network/QGCNetworkHelperTest.cc index 78e830e54170..e74d934a39ec 100644 --- a/test/Utilities/Network/QGCNetworkHelperTest.cc +++ b/test/Utilities/Network/QGCNetworkHelperTest.cc @@ -344,61 +344,6 @@ void QGCNetworkHelperTest::_testUrlWithoutQuery() QVERIFY(result.fragment().isEmpty()); } -void QGCNetworkHelperTest::_testRedactedUrlPreservesStreamIdentity() -{ - QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("rtsp://camera.example:554/axis-media/media.amp")), - QStringLiteral("rtsp://camera.example:554/axis-media/media.amp")); - QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("udp://0.0.0.0:5600")), - QStringLiteral("udp://0.0.0.0:5600")); -} - -void QGCNetworkHelperTest::_testRedactedUrlRemovesUserInfo() -{ - const QString result = QGCNetworkHelper::redactedUrlForLogging( - QStringLiteral("https://pilot%40ops:secret%2Fvalue@example.com:8443/video%20feed")); - - QCOMPARE(result, QStringLiteral("https://example.com:8443/video%20feed")); - QVERIFY(!result.contains(QStringLiteral("pilot"))); - QVERIFY(!result.contains(QStringLiteral("secret"))); -} - -void QGCNetworkHelperTest::_testRedactedUrlRedactsQueryValues() -{ - const QString result = QGCNetworkHelper::redactedUrlForLogging( - QStringLiteral("https://example.com/video?token=abc123&mode=low-latency#session")); - const QUrl resultUrl(result); - const QUrlQuery resultQuery(resultUrl); - - QCOMPARE(resultUrl.path(), QStringLiteral("/video")); - QCOMPARE(resultQuery.queryItemValue(QStringLiteral("token")), QStringLiteral("REDACTED")); - QCOMPARE(resultQuery.queryItemValue(QStringLiteral("mode")), QStringLiteral("REDACTED")); - QCOMPARE(resultUrl.fragment(), QStringLiteral("REDACTED")); - QVERIFY(!result.contains(QStringLiteral("abc123"))); - QVERIFY(!result.contains(QStringLiteral("low-latency"))); - QVERIFY(!result.contains(QStringLiteral("session"))); -} - -void QGCNetworkHelperTest::_testRedactedUrlHandlesRelativeAndInvalidInput() -{ - QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("5600")), QStringLiteral("5600")); - QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("camera.local:5600")), - QStringLiteral("camera.local:5600")); - QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QString()), QStringLiteral("")); - QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("http://[invalid")), - QStringLiteral("")); -} - -void QGCNetworkHelperTest::_testRedactedUrlQUrlOverload() -{ - const QUrl sourceUrl(QStringLiteral("rtsp://pilot:secret@camera.example:8554/live?token=abc123")); - const QString result = QGCNetworkHelper::redactedUrlForLogging(sourceUrl); - - QCOMPARE(QUrl(result).path(), QStringLiteral("/live")); - QVERIFY(!result.contains(QStringLiteral("pilot"))); - QVERIFY(!result.contains(QStringLiteral("secret"))); - QVERIFY(!result.contains(QStringLiteral("abc123"))); -} - // ============================================================================ // Request Configuration Tests // ============================================================================ diff --git a/test/Utilities/Network/QGCNetworkHelperTest.h b/test/Utilities/Network/QGCNetworkHelperTest.h index bef94e99522f..4b14265c6178 100644 --- a/test/Utilities/Network/QGCNetworkHelperTest.h +++ b/test/Utilities/Network/QGCNetworkHelperTest.h @@ -38,11 +38,6 @@ private slots: void _testBuildUrlFromMap(); void _testBuildUrlFromList(); void _testUrlWithoutQuery(); - void _testRedactedUrlPreservesStreamIdentity(); - void _testRedactedUrlRemovesUserInfo(); - void _testRedactedUrlRedactsQueryValues(); - void _testRedactedUrlHandlesRelativeAndInvalidInput(); - void _testRedactedUrlQUrlOverload(); // Request configuration tests void _testDefaultUserAgent(); diff --git a/test/Utilities/Network/QGCNetworkRedactionTest.cc b/test/Utilities/Network/QGCNetworkRedactionTest.cc new file mode 100644 index 000000000000..21ad614d3419 --- /dev/null +++ b/test/Utilities/Network/QGCNetworkRedactionTest.cc @@ -0,0 +1,63 @@ +#include "QGCNetworkRedactionTest.h" + +#include "QGCNetworkHelper.h" + +#include +#include + +void QGCNetworkRedactionTest::_testPreservesStreamIdentity() +{ + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("rtsp://camera.example:554/axis-media/media.amp")), + QStringLiteral("rtsp://camera.example:554/axis-media/media.amp")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("udp://0.0.0.0:5600")), + QStringLiteral("udp://0.0.0.0:5600")); +} + +void QGCNetworkRedactionTest::_testRemovesUserInfo() +{ + const QString result = QGCNetworkHelper::redactedUrlForLogging( + QStringLiteral("https://pilot%40ops:secret%2Fvalue@example.com:8443/video%20feed")); + + QCOMPARE(result, QStringLiteral("https://example.com:8443/video%20feed")); + QVERIFY(!result.contains(QStringLiteral("pilot"))); + QVERIFY(!result.contains(QStringLiteral("secret"))); +} + +void QGCNetworkRedactionTest::_testRedactsQueryValues() +{ + const QString result = QGCNetworkHelper::redactedUrlForLogging( + QStringLiteral("https://example.com/video?token=abc123&mode=low-latency#session")); + const QUrl resultUrl(result); + const QUrlQuery resultQuery(resultUrl); + + QCOMPARE(resultUrl.path(), QStringLiteral("/video")); + QCOMPARE(resultQuery.queryItemValue(QStringLiteral("token")), QStringLiteral("REDACTED")); + QCOMPARE(resultQuery.queryItemValue(QStringLiteral("mode")), QStringLiteral("REDACTED")); + QCOMPARE(resultUrl.fragment(), QStringLiteral("REDACTED")); + QVERIFY(!result.contains(QStringLiteral("abc123"))); + QVERIFY(!result.contains(QStringLiteral("low-latency"))); + QVERIFY(!result.contains(QStringLiteral("session"))); +} + +void QGCNetworkRedactionTest::_testHandlesRelativeAndInvalidInput() +{ + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("5600")), QStringLiteral("5600")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("camera.local:5600")), + QStringLiteral("camera.local:5600")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QString()), QStringLiteral("")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("http://[invalid")), + QStringLiteral("")); +} + +void QGCNetworkRedactionTest::_testQUrlOverload() +{ + const QUrl sourceUrl(QStringLiteral("rtsp://pilot:secret@camera.example:8554/live?token=abc123")); + const QString result = QGCNetworkHelper::redactedUrlForLogging(sourceUrl); + + QCOMPARE(QUrl(result).path(), QStringLiteral("/live")); + QVERIFY(!result.contains(QStringLiteral("pilot"))); + QVERIFY(!result.contains(QStringLiteral("secret"))); + QVERIFY(!result.contains(QStringLiteral("abc123"))); +} + +UT_REGISTER_TEST(QGCNetworkRedactionTest, TestLabel::Unit, TestLabel::Utilities) diff --git a/test/Utilities/Network/QGCNetworkRedactionTest.h b/test/Utilities/Network/QGCNetworkRedactionTest.h new file mode 100644 index 000000000000..bc33846a4af8 --- /dev/null +++ b/test/Utilities/Network/QGCNetworkRedactionTest.h @@ -0,0 +1,15 @@ +#pragma once + +#include "UnitTest.h" + +class QGCNetworkRedactionTest : public UnitTest +{ + Q_OBJECT + +private slots: + void _testPreservesStreamIdentity(); + void _testRemovesUserInfo(); + void _testRedactsQueryValues(); + void _testHandlesRelativeAndInvalidInput(); + void _testQUrlOverload(); +}; From d1fd5c7f1e32583b8c5864fa4aae68ea131e2ed1 Mon Sep 17 00:00:00 2001 From: alireza787b Date: Fri, 31 Jul 2026 01:41:02 +0000 Subject: [PATCH 08/11] style(Utilities): follow configured include order --- test/Utilities/Network/QGCNetworkRedactionTest.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Utilities/Network/QGCNetworkRedactionTest.cc b/test/Utilities/Network/QGCNetworkRedactionTest.cc index 21ad614d3419..a5e532935dd6 100644 --- a/test/Utilities/Network/QGCNetworkRedactionTest.cc +++ b/test/Utilities/Network/QGCNetworkRedactionTest.cc @@ -1,10 +1,10 @@ #include "QGCNetworkRedactionTest.h" -#include "QGCNetworkHelper.h" - #include #include +#include "QGCNetworkHelper.h" + void QGCNetworkRedactionTest::_testPreservesStreamIdentity() { QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("rtsp://camera.example:554/axis-media/media.amp")), From 55c22941289b222e5b38c3159499ab92d46b645b Mon Sep 17 00:00:00 2001 From: alireza787b Date: Fri, 31 Jul 2026 02:10:34 +0000 Subject: [PATCH 09/11] test(VideoManager): expect invalid URI warnings --- .../GStreamer/SourceFactory/GStreamerSourceFactoryTest.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/VideoManager/GStreamer/SourceFactory/GStreamerSourceFactoryTest.cc b/test/VideoManager/GStreamer/SourceFactory/GStreamerSourceFactoryTest.cc index c7228756a472..3cf58df45613 100644 --- a/test/VideoManager/GStreamer/SourceFactory/GStreamerSourceFactoryTest.cc +++ b/test/VideoManager/GStreamer/SourceFactory/GStreamerSourceFactoryTest.cc @@ -167,9 +167,9 @@ void GStreamerTest::_testSourceFactoryRtspJitterBufferPolicy() void GStreamerTest::_testSourceFactoryRejectsBadUri() { ignoreLogMessage("Video.GStreamer.GstSourceFactory", QtCriticalMsg, - QRegularExpression(QStringLiteral("URI is not specified|Invalid UDP port"))); + QRegularExpression(QStringLiteral("URI is not specified"))); ignoreLogMessage("Video.GStreamer.GstSourceFactory", QtWarningMsg, - QRegularExpression(QStringLiteral("Unsupported URI scheme"))); + QRegularExpression(QStringLiteral("Unsupported URI scheme|Invalid UDP port"))); GStreamer::SourceFactory::Config config; QVERIFY(!GStreamer::SourceFactory::create(QString(), config)); @@ -204,7 +204,7 @@ void GStreamerTest::_testSourceFactoryTcpMpegTs() void GStreamerTest::_testSourceFactoryRejectsBadTcpUri() { - ignoreLogMessage("Video.GStreamer.GstSourceFactory", QtCriticalMsg, + ignoreLogMessage("Video.GStreamer.GstSourceFactory", QtWarningMsg, QRegularExpression(QStringLiteral("Invalid TCP port|Missing host in TCP URI"))); GStreamer::SourceFactory::Config config; From 0fdf313b8e3b094f427529495e2bf326f66f6f37 Mon Sep 17 00:00:00 2001 From: alireza787b Date: Mon, 10 Aug 2026 04:58:19 +0000 Subject: [PATCH 10/11] fix(VideoManager): preserve host-port diagnostics --- src/Utilities/Network/QGCNetworkHelper.cc | 23 +++++++++++- .../GStreamer/GStreamerHelpers.h | 2 +- .../GStreamer/GstVideoReceiver.cc | 37 +++++++++---------- .../Utilities/Network/QGCNetworkHelperTest.cc | 1 - .../Network/QGCNetworkRedactionTest.cc | 10 ++++- 5 files changed, 49 insertions(+), 24 deletions(-) diff --git a/src/Utilities/Network/QGCNetworkHelper.cc b/src/Utilities/Network/QGCNetworkHelper.cc index 0478068fdbb0..84804189e547 100644 --- a/src/Utilities/Network/QGCNetworkHelper.cc +++ b/src/Utilities/Network/QGCNetworkHelper.cc @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -342,12 +343,23 @@ QUrl urlWithoutQuery(const QUrl& url) return url.adjusted(QUrl::RemoveQuery | QUrl::RemoveFragment); } +namespace { +bool isHostPortForLogging(const QString& value) +{ + static const QRegularExpression pattern(QStringLiteral(R"(^[^/@?#\s]+:\d{1,5}$)")); + return pattern.match(value).hasMatch(); +} +} // namespace + QString redactedUrlForLogging(const QUrl& url) { if (url.isEmpty()) { return QStringLiteral(""); } if (!url.isValid()) { + if (url.scheme().isEmpty() && isHostPortForLogging(url.path())) { + return url.path(); + } return QStringLiteral(""); } @@ -359,7 +371,7 @@ QString redactedUrlForLogging(const QUrl& url) redactedQuery.addQueryItem(queryItem.first, QStringLiteral("REDACTED")); } if (queryItems.isEmpty()) { - redactedUrl.setQuery(QStringLiteral("REDACTED")); + redactedUrl.setQuery(QString()); } else { redactedUrl.setQuery(redactedQuery); } @@ -373,7 +385,14 @@ QString redactedUrlForLogging(const QUrl& url) QString redactedUrlForLogging(const QString& url) { - return redactedUrlForLogging(QUrl(url)); + if (isHostPortForLogging(url)) { + return url; + } + const QUrl parsedUrl(url); + if (!url.isEmpty() && !parsedUrl.isValid()) { + return QStringLiteral("").arg(url.size()); + } + return redactedUrlForLogging(parsedUrl); } // ============================================================================ diff --git a/src/VideoManager/VideoReceiver/GStreamer/GStreamerHelpers.h b/src/VideoManager/VideoReceiver/GStreamer/GStreamerHelpers.h index 2bc6d87a93ca..cdd28168c3b7 100644 --- a/src/VideoManager/VideoReceiver/GStreamer/GStreamerHelpers.h +++ b/src/VideoManager/VideoReceiver/GStreamer/GStreamerHelpers.h @@ -9,7 +9,7 @@ #include "GStreamer.h" // VideoDecoderOptions namespace GStreamer { -/// Diagnostic graphs omit element properties because source properties can contain credentials. +/// Automatic field-report graphs omit element properties because source properties can contain credentials. inline constexpr GstDebugGraphDetails kDiagnosticDotGraphDetails = static_cast( GST_DEBUG_GRAPH_SHOW_MEDIA_TYPE | GST_DEBUG_GRAPH_SHOW_CAPS_DETAILS | GST_DEBUG_GRAPH_SHOW_STATES); diff --git a/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.cc b/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.cc index be554a8c05f4..de3aa443490d 100644 --- a/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.cc +++ b/src/VideoManager/VideoReceiver/GStreamer/GstVideoReceiver.cc @@ -274,7 +274,7 @@ void GstVideoReceiver::start(uint32_t timeout) gst_clear_object(&bus); } - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-initial"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-initial"); running = (gst_element_set_state(_pipeline, GST_STATE_PLAYING) != GST_STATE_CHANGE_FAILURE); } while(0); @@ -298,7 +298,7 @@ void GstVideoReceiver::start(uint32_t timeout) emit onStartComplete(STATUS_FAIL); } else { - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-started"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-started"); qCDebug(GstVideoReceiverLog) << "Started" << _redactedUri(); // _watchdogTimer lives on `this` (GUI thread); the emit runs synchronously on the @@ -416,7 +416,7 @@ void GstVideoReceiver::stop() _shutdownDecodingBranch(); } - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-stopped"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-stopped"); // Lock before nulling so an in-flight _onBusMessage on the streaming thread cannot read // a half-destroyed _pipeline. _acquirePipelineRef takes its own ref under the same lock. @@ -613,7 +613,7 @@ void GstVideoReceiver::startRecording(const QString &videoFile, FILE_FORMAT form _fileSink = _makeFileSink(videoFile, format, inputCaps); gst_clear_caps(&inputCaps); if (!_fileSink) { - qCCritical(GstVideoReceiverLog) << "_makeFileSink() failed" << _uri; + qCCritical(GstVideoReceiverLog) << "_makeFileSink() failed" << _redactedUri(); failRecordingStart(); return; } @@ -621,20 +621,20 @@ void GstVideoReceiver::startRecording(const QString &videoFile, FILE_FORMAT form _removingRecorder = false; if (!gst_bin_add(GST_BIN(_pipeline), _fileSink)) { - qCCritical(GstVideoReceiverLog) << "gst_bin_add(file sink) failed" << _uri; + qCCritical(GstVideoReceiverLog) << "gst_bin_add(file sink) failed" << _redactedUri(); failRecordingStart(); return; } (void) gst_object_ref(_fileSink); // Keep a reference in addition to the pipeline's ownership. if (!gst_element_link(_recorderValve, _fileSink)) { - qCCritical(GstVideoReceiverLog) << "Failed to link valve and file sink" << _uri; + qCCritical(GstVideoReceiverLog) << "Failed to link valve and file sink" << _redactedUri(); failRecordingStart(); return; } if (!gst_element_sync_state_with_parent(_fileSink)) { - qCCritical(GstVideoReceiverLog) << "gst_element_sync_state_with_parent(file sink) failed" << _uri; + qCCritical(GstVideoReceiverLog) << "gst_element_sync_state_with_parent(file sink) failed" << _redactedUri(); failRecordingStart(); return; } @@ -646,7 +646,7 @@ void GstVideoReceiver::startRecording(const QString &videoFile, FILE_FORMAT form // This will ensure the first frame is a keyframe at t=0, and decoding can begin immediately on playback _keyframeWatchId = gst_pad_add_probe(probepad, GST_PAD_PROBE_TYPE_BUFFER, _keyframeWatch, this, nullptr); if (_keyframeWatchId == 0) { - qCCritical(GstVideoReceiverLog) << "gst_pad_add_probe(_keyframeWatch) failed" << _uri; + qCCritical(GstVideoReceiverLog) << "gst_pad_add_probe(_keyframeWatch) failed" << _redactedUri(); failRecordingStart(); return; } @@ -738,7 +738,7 @@ void GstVideoReceiver::_watchdog() qint64 elapsed = now - lastSourceFrameTime; if (elapsed > _timeout) { qCDebug(GstVideoReceiverLog) << "Stream timeout, no frames for" << elapsed << _redactedUri(); - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-watchdog-timeout"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-watchdog-timeout"); emit timeout(); _scheduleReconnect("source watchdog"); return; @@ -754,8 +754,7 @@ void GstVideoReceiver::_watchdog() elapsed = now - lastVideoFrameTime; if (elapsed > (_timeout * 2)) { qCDebug(GstVideoReceiverLog) << "Video decoder timeout, no frames for" << elapsed << _redactedUri(); - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, - "pipeline-watchdog-timeout"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-watchdog-timeout"); emit timeout(); _scheduleReconnect("decoder watchdog"); } @@ -821,7 +820,7 @@ void GstVideoReceiver::dumpPipelineGraph(const QString &tag) return; } const QByteArray tagUtf8 = tag.toUtf8(); - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(pipelineRef), GStreamer::kDiagnosticDotGraphDetails, tagUtf8.constData()); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(pipelineRef), GST_DEBUG_GRAPH_SHOW_ALL, tagUtf8.constData()); const QString dotPath = GStreamer::writePipelineDot(pipelineRef, tagUtf8.constData()); if (!dotPath.isEmpty()) { qCInfo(GstVideoReceiverLog) << "Pipeline graph saved to" << dotPath; @@ -1019,7 +1018,7 @@ void GstVideoReceiver::_onNewSourcePad(GstPad *pad) return; } - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-with-new-source-pad"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-with-new-source-pad"); _ensureVideoSinkInPipeline(); @@ -1095,7 +1094,7 @@ void GstVideoReceiver::_onNewDecoderPad(GstPad *pad) { qCDebug(GstVideoReceiverLog) << "_onNewDecoderPad" << _redactedUri(); - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-with-new-decoder-pad"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-with-new-decoder-pad"); // We should now know what codec decodebin3 selected. _logDecodebin3SelectedCodec(_decoder); @@ -1118,7 +1117,7 @@ bool GstVideoReceiver::_addDecoder(GstElement *src) (void) gst_bin_add(GST_BIN(_pipeline), _decoder); (void) gst_element_sync_state_with_parent(_decoder); - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-with-decoder"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-with-decoder"); if (!gst_element_link(src, _decoder)) { qCCritical(GstVideoReceiverLog) << "Unable to link decoder"; @@ -1193,7 +1192,7 @@ bool GstVideoReceiver::_addVideoSink(GstPad *pad) (void) gst_element_sync_state_with_parent(_videoSink); - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-with-videosink"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-with-videosink"); // Determine video size. Errors here are non-fatal. QSize videoSize; @@ -1378,7 +1377,7 @@ void GstVideoReceiver::_shutdownDecodingBranch() emit decodingChanged(_decoding); } - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-decoding-stopped"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-decoding-stopped"); } void GstVideoReceiver::_shutdownRecordingBranch() @@ -1410,7 +1409,7 @@ void GstVideoReceiver::_shutdownRecordingBranch() emit onStopRecordingComplete(STATUS_OK); } - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GStreamer::kDiagnosticDotGraphDetails, "pipeline-recording-stopped"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(_pipeline), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-recording-stopped"); } bool GstVideoReceiver::_needDispatch() @@ -1469,7 +1468,7 @@ gboolean GstVideoReceiver::_onBusMessage(GstBus * /* bus */, GstMessage *msg, gp if (GstElement *pipelineRef = pThis->_acquirePipelineRef()) { // Native dump path (no-op without GST_DEBUG_DUMP_DOT_DIR) plus an unconditional // CacheLocation fallback so field-bug-report bundles include pipeline topology. - GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(pipelineRef), GStreamer::kDiagnosticDotGraphDetails, "pipeline-error"); + GST_DEBUG_BIN_TO_DOT_FILE(GST_BIN(pipelineRef), GST_DEBUG_GRAPH_SHOW_ALL, "pipeline-error"); const QString dotPath = GStreamer::writePipelineDot(pipelineRef, "pipeline-error"); if (!dotPath.isEmpty()) { qCInfo(GstVideoReceiverLog) << "Pipeline graph saved to" << dotPath; diff --git a/test/Utilities/Network/QGCNetworkHelperTest.cc b/test/Utilities/Network/QGCNetworkHelperTest.cc index e74d934a39ec..589b8fb8cc24 100644 --- a/test/Utilities/Network/QGCNetworkHelperTest.cc +++ b/test/Utilities/Network/QGCNetworkHelperTest.cc @@ -2,7 +2,6 @@ #include #include -#include #include #include #include diff --git a/test/Utilities/Network/QGCNetworkRedactionTest.cc b/test/Utilities/Network/QGCNetworkRedactionTest.cc index a5e532935dd6..2956fa0d28ef 100644 --- a/test/Utilities/Network/QGCNetworkRedactionTest.cc +++ b/test/Utilities/Network/QGCNetworkRedactionTest.cc @@ -37,6 +37,8 @@ void QGCNetworkRedactionTest::_testRedactsQueryValues() QVERIFY(!result.contains(QStringLiteral("abc123"))); QVERIFY(!result.contains(QStringLiteral("low-latency"))); QVERIFY(!result.contains(QStringLiteral("session"))); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("https://example.com/video?")), + QStringLiteral("https://example.com/video")); } void QGCNetworkRedactionTest::_testHandlesRelativeAndInvalidInput() @@ -44,20 +46,26 @@ void QGCNetworkRedactionTest::_testHandlesRelativeAndInvalidInput() QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("5600")), QStringLiteral("5600")); QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("camera.local:5600")), QStringLiteral("camera.local:5600")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("0.0.0.0:5600")), QStringLiteral("0.0.0.0:5600")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("192.168.1.10:5600")), + QStringLiteral("192.168.1.10:5600")); QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QString()), QStringLiteral("")); QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("http://[invalid")), - QStringLiteral("")); + QStringLiteral("")); } void QGCNetworkRedactionTest::_testQUrlOverload() { const QUrl sourceUrl(QStringLiteral("rtsp://pilot:secret@camera.example:8554/live?token=abc123")); const QString result = QGCNetworkHelper::redactedUrlForLogging(sourceUrl); + const QUrl hostPortUrl(QStringLiteral("192.168.1.10:5600")); QCOMPARE(QUrl(result).path(), QStringLiteral("/live")); QVERIFY(!result.contains(QStringLiteral("pilot"))); QVERIFY(!result.contains(QStringLiteral("secret"))); QVERIFY(!result.contains(QStringLiteral("abc123"))); + QVERIFY(!hostPortUrl.isValid()); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(hostPortUrl), QStringLiteral("192.168.1.10:5600")); } UT_REGISTER_TEST(QGCNetworkRedactionTest, TestLabel::Unit, TestLabel::Utilities) From c8ffeaf59337c8828f7a36c921ff8ac1d9ba66e5 Mon Sep 17 00:00:00 2001 From: alireza787b Date: Thu, 20 Aug 2026 09:08:56 +0000 Subject: [PATCH 11/11] fix(Utilities): validate host-port diagnostics --- src/Utilities/Network/QGCNetworkHelper.cc | 95 +++++++++++++++++-- .../VideoReceiver/GStreamer/README.md | 2 +- .../Network/QGCNetworkRedactionTest.cc | 14 +++ 3 files changed, 102 insertions(+), 9 deletions(-) diff --git a/src/Utilities/Network/QGCNetworkHelper.cc b/src/Utilities/Network/QGCNetworkHelper.cc index 84804189e547..59cc10573d87 100644 --- a/src/Utilities/Network/QGCNetworkHelper.cc +++ b/src/Utilities/Network/QGCNetworkHelper.cc @@ -5,8 +5,8 @@ #include #include #include -#include #include +#include #include #include #include @@ -344,10 +344,79 @@ QUrl urlWithoutQuery(const QUrl& url) } namespace { -bool isHostPortForLogging(const QString& value) +enum class HostPortClassification { - static const QRegularExpression pattern(QStringLiteral(R"(^[^/@?#\s]+:\d{1,5}$)")); - return pattern.match(value).hasMatch(); + NotHostPort, + Valid, + Invalid, +}; + +bool isAsciiAlphaNumeric(char character) +{ + return ((character >= 'a') && (character <= 'z')) || ((character >= 'A') && (character <= 'Z')) || + ((character >= '0') && (character <= '9')); +} + +bool isValidHostname(QString hostname) +{ + if (hostname.endsWith(QLatin1Char('.'))) { + hostname.chop(1); + } + + const QByteArray aceHostname = QUrl::toAce(hostname); + if (aceHostname.isEmpty() || (aceHostname.size() > 253)) { + return false; + } + + const QList labels = aceHostname.split('.'); + for (const QByteArray& label : labels) { + if (label.isEmpty() || (label.size() > 63) || !isAsciiAlphaNumeric(label.front()) || + !isAsciiAlphaNumeric(label.back())) { + return false; + } + for (const char character : label) { + if (!isAsciiAlphaNumeric(character) && (character != '-')) { + return false; + } + } + } + return true; +} + +HostPortClassification classifyHostPort(const QString& value) +{ + if (value.contains(QStringLiteral("://"))) { + return HostPortClassification::NotHostPort; + } + + const qsizetype separator = value.lastIndexOf(QLatin1Char(':')); + if ((separator <= 0) || (separator == (value.size() - 1))) { + return HostPortClassification::NotHostPort; + } + + const QString portText = value.sliced(separator + 1); + for (const QChar character : portText) { + if (!character.isDigit()) { + return HostPortClassification::NotHostPort; + } + } + + bool portOk = false; + const int port = portText.toInt(&portOk); + if (!portOk || (port < 1) || (port > 65535)) { + return HostPortClassification::Invalid; + } + + const QUrl authority(QStringLiteral("qgc://") + value, QUrl::StrictMode); + if (!authority.isValid() || authority.host().isEmpty() || !authority.userInfo().isEmpty() || + !authority.path().isEmpty() || authority.hasQuery() || authority.hasFragment() || + (authority.port(-1) != port)) { + return HostPortClassification::Invalid; + } + + QHostAddress address; + const bool validHost = address.setAddress(authority.host()) || isValidHostname(authority.host()); + return validHost ? HostPortClassification::Valid : HostPortClassification::Invalid; } } // namespace @@ -356,10 +425,16 @@ QString redactedUrlForLogging(const QUrl& url) if (url.isEmpty()) { return QStringLiteral(""); } + + const QString sourceText = url.isValid() ? url.toString(QUrl::FullyEncoded) : url.path(); + const HostPortClassification hostPort = classifyHostPort(sourceText); + if (hostPort == HostPortClassification::Valid) { + return sourceText; + } + if (hostPort == HostPortClassification::Invalid) { + return QStringLiteral("").arg(sourceText.size()); + } if (!url.isValid()) { - if (url.scheme().isEmpty() && isHostPortForLogging(url.path())) { - return url.path(); - } return QStringLiteral(""); } @@ -385,9 +460,13 @@ QString redactedUrlForLogging(const QUrl& url) QString redactedUrlForLogging(const QString& url) { - if (isHostPortForLogging(url)) { + const HostPortClassification hostPort = classifyHostPort(url); + if (hostPort == HostPortClassification::Valid) { return url; } + if (hostPort == HostPortClassification::Invalid) { + return QStringLiteral("").arg(url.size()); + } const QUrl parsedUrl(url); if (!url.isEmpty() && !parsedUrl.isValid()) { return QStringLiteral("").arg(url.size()); diff --git a/src/VideoManager/VideoReceiver/GStreamer/README.md b/src/VideoManager/VideoReceiver/GStreamer/README.md index 2898166842de..556238547cff 100644 --- a/src/VideoManager/VideoReceiver/GStreamer/README.md +++ b/src/VideoManager/VideoReceiver/GStreamer/README.md @@ -137,7 +137,7 @@ dot -Tpng /tmp/qgc-pipeline-dots/0.00.00.*-pipeline-started.dot -o pipeline.png When the env var is **unset**, QGC still writes a rotating snapshot (≤10 files) to `/qgc-pipeline-dot/-.dot` on `ERROR` and on watchdog timeout, so field-bug-report bundles include the topology automatically. The `GstVideoReceiver::dumpPipelineGraph(tag)` slot (callable from QML) writes a snapshot on demand for use from a debug menu. -QGC graph dumps include topology, caps, media types, and states. Element property values are omitted because source properties can contain stream credentials. +Automatic CacheLocation snapshots include topology, caps, media types, and states while omitting element properties that can contain stream credentials. Native dumps explicitly enabled with `GST_DEBUG_DUMP_DOT_DIR` retain GStreamer's full `SHOW_ALL` detail for local debugging and can include credential-bearing source properties; handle those files as sensitive data. ### Latency tracer diff --git a/test/Utilities/Network/QGCNetworkRedactionTest.cc b/test/Utilities/Network/QGCNetworkRedactionTest.cc index 2956fa0d28ef..49a624abea6a 100644 --- a/test/Utilities/Network/QGCNetworkRedactionTest.cc +++ b/test/Utilities/Network/QGCNetworkRedactionTest.cc @@ -49,6 +49,17 @@ void QGCNetworkRedactionTest::_testHandlesRelativeAndInvalidInput() QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("0.0.0.0:5600")), QStringLiteral("0.0.0.0:5600")); QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("192.168.1.10:5600")), QStringLiteral("192.168.1.10:5600")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("[2001:db8::1]:5600")), + QStringLiteral("[2001:db8::1]:5600")); + + const QString malformedCredential = QStringLiteral("token=supersecret:5600"); + const QString malformedResult = QGCNetworkHelper::redactedUrlForLogging(malformedCredential); + QCOMPARE(malformedResult, QStringLiteral("").arg(malformedCredential.size())); + QVERIFY(!malformedResult.contains(QStringLiteral("supersecret"))); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("camera.local:0")), + QStringLiteral("")); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("camera.local:65536")), + QStringLiteral("")); QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QString()), QStringLiteral("")); QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(QStringLiteral("http://[invalid")), QStringLiteral("")); @@ -59,6 +70,7 @@ void QGCNetworkRedactionTest::_testQUrlOverload() const QUrl sourceUrl(QStringLiteral("rtsp://pilot:secret@camera.example:8554/live?token=abc123")); const QString result = QGCNetworkHelper::redactedUrlForLogging(sourceUrl); const QUrl hostPortUrl(QStringLiteral("192.168.1.10:5600")); + const QUrl ipv6HostPortUrl(QStringLiteral("[2001:db8::1]:5600")); QCOMPARE(QUrl(result).path(), QStringLiteral("/live")); QVERIFY(!result.contains(QStringLiteral("pilot"))); @@ -66,6 +78,8 @@ void QGCNetworkRedactionTest::_testQUrlOverload() QVERIFY(!result.contains(QStringLiteral("abc123"))); QVERIFY(!hostPortUrl.isValid()); QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(hostPortUrl), QStringLiteral("192.168.1.10:5600")); + QVERIFY(!ipv6HostPortUrl.isValid()); + QCOMPARE(QGCNetworkHelper::redactedUrlForLogging(ipv6HostPortUrl), QStringLiteral("[2001:db8::1]:5600")); } UT_REGISTER_TEST(QGCNetworkRedactionTest, TestLabel::Unit, TestLabel::Utilities)