espressif/openthread
1
0
Fork 0
mirror of https://github.com/espressif/openthread.git synced 2026-06-05 21:14:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

[posix] detect and fail on unused radio URL parameters (#13087)

Browse Source 
This commit enhances the radio URL parsing logic to detect and fail
when unused parameters are provided in the URL. This prevents typos
or unsupported parameters from being silently ignored.

The following changes were made:

- Updated ot::Url::Url to track parameter usage by appending a
  trailing '&' delimiter in Init() and replacing it with '\0'
  in GetValue() when a parameter is matched. This marks the
  parameter as used and removes any limit on the number of
  trackable parameters.
- Added a Validate() method to ot::Url::Url to verify that all
  parameters in the query string were accessed.
- Refactored ot::Posix::Radio to share a single RadioUrl instance
  with SpinelManager, ensuring all components track usage on the
  same URL object.
- Integrated Validate() calls in otSysInit() and platformTrelInit()
  to perform validation after all platform components have been
  initialized.
- Updated Radio::ProcessMaxPowerTable to use a local copy of the
  parameter string to avoid premature modification of the URL buffer.
- Adjusted RadioUrl and unit tests to provide sufficient buffer
  space for the additional tracking delimiter.
- Added new unit tests in tests/unit/test_url.cpp to verify the
  usage tracking and validation logic.
This commit is contained in:
Jonathan Hui
2026-05-09 10:04:07 -07:00
committed by GitHub
parent 545a83efbf
commit 0841be04fd
10 changed files with 233 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/lib/url/url.cpp
+79 -13
View File
@@ -49,6 +49,7 @@ otError Url::Init(char *aUrl)
{
otError error = OT_ERROR_NONE;
char *url = aUrl;
char *query;
mEnd = aUrl + strlen(aUrl);
mProtocol = aUrl;
@@ -59,15 +60,31 @@ otError Url::Init(char *aUrl)
url += sizeof("://") - 1;
mPath = url;
url = strstr(url, "?");
query = strchr(url, '?');
if (url != nullptr)
if (query != nullptr)
{
mQuery = ++url;
char *end;
for (char *cur = strtok(url, "&"); cur != nullptr; cur = strtok(nullptr, "&"))
*query = '\0';
mQuery = query + 1;
end = strchr(const_cast<char *>(mQuery), '#');
if (end == nullptr)
{
cur[-1] = '\0';
end = const_cast<char *>(mEnd);
}
if (end > mQuery && end[-1] != '&')
{
memmove(end + 1, end, mEnd - end + 1);
*end = '&';
mEnd = end + 1;
}
else
{
mEnd = end;
}
}
else
@@ -97,21 +114,40 @@ const char *Url::GetValue(const char *aName, const char *aLastValue) const
while (start < mEnd)
{
const char *last = nullptr;
if (!strncmp(aName, start, len))
if (!strncmp(aName, start, len) && (start[len] == '=' || start[len] == '&' || start[len] == '\0'))
{
const char *delimiter = start + len;
while (delimiter < mEnd && *delimiter != '&' && *delimiter != '\0')
{
delimiter++;
}
if (delimiter < mEnd && *delimiter == '&')
{
*const_cast<char *>(delimiter) = '\0';
}
if (start[len] == '=')
{
ExitNow(rval = &start[len + 1]);
rval = &start[len + 1];
}
else if (start[len] == '\0')
else
{
ExitNow(rval = &start[len]);
rval = &start[len];
}
break;
}
while (start < mEnd && *start != '&' && *start != '\0')
{
start++;
}
if (start < mEnd)
{
start++;
}
last = start;
start = last + strlen(last) + 1;
}
exit:
@@ -202,5 +238,35 @@ exit:
return error;
}
otError Url::Validate(const char **aUnusedParam) const
{
otError error = OT_ERROR_NONE;
const char *cur = mQuery;
while (cur < mEnd)
{
if (*cur == '&')
{
const char *p = cur;
while (p > mQuery && p[-1] != '\0')
{
p--;
}
if (aUnusedParam != nullptr)
{
*aUnusedParam = p;
}
ExitNow(error = OT_ERROR_INVALID_ARGS);
}
cur++;
}
exit:
return error;
}
} // namespace Url
} // namespace ot
src/lib/url/url.hpp
+10
View File
@@ -192,6 +192,16 @@ public:
* @retval OT_ERROR_INVALID_ARGS The parameter value was not contain valid number (e.g., value out of range).
*/
otError ParseInt8(const char *aName, int8_t &aValue) const;
/**
* Checks if all parameters are used.
*
* @param[out] aUnusedParam A pointer to a string to store the name of the first unused parameter.
*
* @retval OT_ERROR_NONE All parameters were used.
* @retval OT_ERROR_INVALID_ARGS Some parameters were not used.
*/
otError Validate(const char **aUnusedParam = nullptr) const;
};
} // namespace Url
src/posix/platform/hdlc_interface.cpp
+23 -24
View File
@@ -685,9 +685,29 @@ exit:
#if OPENTHREAD_POSIX_CONFIG_RCP_PTY_ENABLE
int HdlcInterface::ForkPty(const Url::Url &aRadioUrl)
{
int fd = -1;
int pid = -1;
int rval = -1;
int fd = -1;
int pid = -1;
int rval = -1;
constexpr int kMaxArguments = 32;
char *argv[kMaxArguments + 1];
size_t index = 0;
argv[index++] = const_cast<char *>(aRadioUrl.GetPath());
for (const char *arg = nullptr;
index < OT_ARRAY_LENGTH(argv) && (arg = aRadioUrl.GetValue("forkpty-arg", arg)) != nullptr;
argv[index++] = const_cast<char *>(arg))
{
}
if (index < OT_ARRAY_LENGTH(argv))
{
argv[index] = nullptr;
}
else
{
DieNowWithMessage("Too many arguments!", OT_EXIT_INVALID_ARGUMENTS);
}
{
struct termios tios;
@@ -701,27 +721,6 @@ int HdlcInterface::ForkPty(const Url::Url &aRadioUrl)
if (0 == pid)
{
constexpr int kMaxArguments = 32;
char *argv[kMaxArguments + 1];
size_t index = 0;
argv[index++] = const_cast<char *>(aRadioUrl.GetPath());
for (const char *arg = nullptr;
index < OT_ARRAY_LENGTH(argv) && (arg = aRadioUrl.GetValue("forkpty-arg", arg)) != nullptr;
argv[index++] = const_cast<char *>(arg))
{
}
if (index < OT_ARRAY_LENGTH(argv))
{
argv[index] = nullptr;
}
else
{
DieNowWithMessage("Too many arguments!", OT_EXIT_INVALID_ARGUMENTS);
}
VerifyOrDie((rval = execvp(argv[0], argv)) != -1, OT_EXIT_ERROR_ERRNO);
}
else
src/posix/platform/radio.cpp
+26 -17
View File
@@ -63,8 +63,7 @@ extern "C" void platformRadioInit(const char *aUrl) { sRadio.Init(aUrl); }
const char Radio::kLogModuleName[] = "Radio";
Radio::Radio(void)
: mRadioUrl(nullptr)
, mRadioSpinel()
: mRadioSpinel()
#if OPENTHREAD_POSIX_CONFIG_RCP_CAPS_DIAG_ENABLE
, mRcpCapsDiag(mRadioSpinel)
#endif
@@ -80,8 +79,10 @@ OT_TOOL_WEAK void platformCoprocessorResetFailed(void *) {}
void Radio::Init(const char *aUrl)
{
bool resetRadio;
bool skipCompatibilityCheck;
bool resetRadio;
bool skipCompatibilityCheck;
RadioUrl &radioUrl = SpinelManager::GetSpinelManager().GetRadioUrl();
#if OPENTHREAD_CONFIG_THREAD_VERSION >= OT_THREAD_VERSION_1_2 && OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE
bool aEnableRcpTimeSync = true;
#else
@@ -89,8 +90,8 @@ void Radio::Init(const char *aUrl)
#endif
struct ot::Spinel::RadioSpinelCallbacks callbacks;
mRadioUrl.Init(aUrl);
VerifyOrDie(mRadioUrl.GetPath() != nullptr, OT_EXIT_INVALID_ARGUMENTS);
VerifyOrDie(radioUrl.GetPath() != nullptr, OT_EXIT_INVALID_ARGUMENTS);
OT_UNUSED_VARIABLE(aUrl);
memset(&callbacks, 0, sizeof(callbacks));
callbacks.mEnergyScanDone = otPlatRadioEnergyScanDone;
@@ -105,8 +106,8 @@ void Radio::Init(const char *aUrl)
mTmpStorage.Init();
#endif
resetRadio = !mRadioUrl.HasParam("no-reset");
skipCompatibilityCheck = mRadioUrl.HasParam("skip-rcp-compatibility-check");
resetRadio = !radioUrl.HasParam("no-reset");
skipCompatibilityCheck = radioUrl.HasParam("skip-rcp-compatibility-check");
#if OPENTHREAD_SPINEL_CONFIG_COPROCESSOR_RESET_FAILURE_CALLBACK_ENABLE
GetSpinelDriver().SetCoprocessorResetFailureCallback(platformCoprocessorResetFailed, this);
@@ -116,7 +117,7 @@ void Radio::Init(const char *aUrl)
mRadioSpinel.Init(skipCompatibilityCheck, resetRadio, &GetSpinelDriver(),
(skipCompatibilityCheck ? 0 : kRequiredRadioCaps), aEnableRcpTimeSync);
ProcessRadioUrl(mRadioUrl);
ProcessRadioUrl(radioUrl);
}
void Radio::Deinit(void)
@@ -201,17 +202,21 @@ void Radio::ProcessMaxPowerTable(const RadioUrl &aRadioUrl)
#if OPENTHREAD_POSIX_CONFIG_MAX_POWER_TABLE_ENABLE
otError error;
constexpr int8_t kPowerDefault = 30; // Default power 1 watt (30 dBm).
const char *str = nullptr;
char *pSave = nullptr;
uint8_t channel = ot::Radio::kChannelMin;
int8_t power = kPowerDefault;
const char *maxPowerTable;
constexpr int8_t kPowerDefault = 30; // Default power 1 watt (30 dBm).
char *str = nullptr;
char *pSave = nullptr;
uint8_t channel = ot::Radio::kChannelMin;
int8_t power = kPowerDefault;
const char *maxPowerTable = nullptr;
char *maxPowerTableCopy = nullptr;
VerifyOrExit((maxPowerTable = aRadioUrl.GetValue("max-power-table")) != nullptr);
for (str = strtok_r(const_cast<char *>(maxPowerTable), ",", &pSave);
str != nullptr && channel <= ot::Radio::kChannelMax; str = strtok_r(nullptr, ",", &pSave))
maxPowerTableCopy = strdup(maxPowerTable);
VerifyOrDie(maxPowerTableCopy != nullptr, OT_EXIT_FAILURE);
for (str = strtok_r(maxPowerTableCopy, ",", &pSave); str != nullptr && channel <= ot::Radio::kChannelMax;
str = strtok_r(nullptr, ",", &pSave))
{
power = static_cast<int8_t>(strtol(str, nullptr, 0));
error = mRadioSpinel.SetChannelMaxTransmitPower(channel, power);
@@ -240,6 +245,10 @@ void Radio::ProcessMaxPowerTable(const RadioUrl &aRadioUrl)
VerifyOrDie(str == nullptr, OT_EXIT_INVALID_ARGUMENTS);
exit:
if (maxPowerTableCopy != nullptr)
{
free(maxPowerTableCopy);
}
return;
#endif // OPENTHREAD_POSIX_CONFIG_MAX_POWER_TABLE_ENABLE
}
src/posix/platform/radio.hpp
-1
View File
@@ -140,7 +140,6 @@ private:
#endif
OT_RADIO_CAPS_ACK_TIMEOUT | OT_RADIO_CAPS_TRANSMIT_RETRIES | OT_RADIO_CAPS_CSMA_BACKOFF;
RadioUrl mRadioUrl;
#if OPENTHREAD_SPINEL_CONFIG_VENDOR_HOOK_ENABLE
Spinel::VendorRadioSpinel mRadioSpinel;
#else
src/posix/platform/radio_url.cpp
+4 -2
View File
@@ -149,8 +149,10 @@ void RadioUrl::Init(const char *aUrl)
{
if (aUrl != nullptr)
{
VerifyOrDie(strnlen(aUrl, sizeof(mUrl)) < sizeof(mUrl), OT_EXIT_INVALID_ARGUMENTS);
strncpy(mUrl, aUrl, sizeof(mUrl) - 1);
size_t len = strlen(aUrl);
VerifyOrDie(len + 1 < sizeof(mUrl), OT_EXIT_INVALID_ARGUMENTS);
strcpy(mUrl, aUrl);
SuccessOrDie(Url::Url::Init(mUrl));
}
}
src/posix/platform/spinel_manager.hpp
+7
View File
@@ -91,6 +91,13 @@ public:
return *mSpinelInterface;
}
/**
* Returns the radio URL.
*
* @returns The radio URL.
*/
RadioUrl &GetRadioUrl(void) { return mUrl; }
private:
#if OPENTHREAD_POSIX_VIRTUAL_TIME
void VirtualTimeInit(void);
src/posix/platform/system.cpp
+11
View File
@@ -59,6 +59,7 @@
#include "posix/platform/mdns_socket.hpp"
#include "posix/platform/radio_url.hpp"
#include "posix/platform/spinel_driver_getter.hpp"
#include "posix/platform/spinel_manager.hpp"
#include "posix/platform/udp.hpp"
otInstance *gInstance = nullptr;
@@ -275,6 +276,16 @@ otInstance *otSysInit(otPlatformConfig *aPlatformConfig)
platformInit(aPlatformConfig);
{
const char *unusedParam = nullptr;
if (ot::Posix::SpinelManager::GetSpinelManager().GetRadioUrl().Validate(&unusedParam) != OT_ERROR_NONE)
{
otLogCritPlat("Radio URL contains unused parameter: \"%s\"", unusedParam);
DieNow(OT_EXIT_INVALID_ARGUMENTS);
}
}
gDryRun = aPlatformConfig->mDryRun;
if (sCoprocessorType == OT_COPROCESSOR_RCP)
{
src/posix/platform/trel.cpp
+9
View File
@@ -658,6 +658,15 @@ void platformTrelInit(const char *aTrelUrl)
ot::Posix::RadioUrl url(aTrelUrl);
otSysTrelInit(url.GetPath());
{
const char *unusedParam = nullptr;
if (url.Validate(&unusedParam) != OT_ERROR_NONE)
{
otLogCritPlat("TREL radio URL contains unused parameter: \"%s\"", unusedParam);
DieNow(OT_EXIT_INVALID_ARGUMENTS);
}
}
}
}
tests/unit/test_url.cpp
+64 -11
View File
@@ -36,7 +36,7 @@ namespace Url {
void TestSimple(void)
{
char url[] = "spinel:///dev/ttyUSB0?baudrate=460800";
char url[100] = "spinel:///dev/ttyUSB0?baudrate=460800";
Url args;
VerifyOrQuit(!args.Init(url));
@@ -53,7 +53,7 @@ void TestSimple(void)
void TestSimpleNoQueryString(void)
{
char url[] = "spinel:///dev/ttyUSB0";
char url[100] = "spinel:///dev/ttyUSB0";
Url args;
VerifyOrQuit(!args.Init(url));
@@ -67,7 +67,7 @@ void TestSimpleNoQueryString(void)
void TestEmptyValue(void)
{
char url[] = "spinel:///dev/ttyUSB0?rtscts&baudrate=460800&verbose&verbose&verbose";
char url[100] = "spinel:///dev/ttyUSB0?rtscts&baudrate=460800&verbose&verbose&verbose";
Url args;
const char *arg = nullptr;
@@ -85,7 +85,7 @@ void TestEmptyValue(void)
void TestMultipleProtocols(void)
{
char url[] = "spinel+spi:///dev/ttyUSB0?baudrate=460800";
char url[100] = "spinel+spi:///dev/ttyUSB0?baudrate=460800";
Url args;
VerifyOrQuit(!args.Init(url));
@@ -97,7 +97,7 @@ void TestMultipleProtocols(void)
void TestMultipleProtocolsAndDuplicateParameters(void)
{
char url[] = "spinel+exec:///path/to/ot-rcp?arg=1&arg=arg2&arg=3";
char url[100] = "spinel+exec:///path/to/ot-rcp?arg=1&arg=arg2&arg=3";
Url args;
const char *arg = nullptr;
@@ -122,9 +122,9 @@ void TestMultipleProtocolsAndDuplicateParameters(void)
void TestIntValue(void)
{
char int8url[] = "spinel:///dev/ttyUSB0?no-reset&val1=1&val2=0x02&val3=-0X03&val4=-4&val5=+5&val6=128&val7=-129";
char int16url[] = "spinel:///dev/ttyUSB0?val1=1&val2=0x02&val3=-0X03&val4=-400&val5=+500&val6=32768&val7=-32769";
char int32url[] =
char int8url[200] = "spinel:///dev/ttyUSB0?no-reset&val1=1&val2=0x02&val3=-0X03&val4=-4&val5=+5&val6=128&val7=-129";
char int16url[200] = "spinel:///dev/ttyUSB0?val1=1&val2=0x02&val3=-0X03&val4=-400&val5=+500&val6=32768&val7=-32769";
char int32url[200] =
"spinel:///dev/ttyUSB0?val1=1&val2=0x02&val3=-0X03&val4=-40000&val5=+50000&val6=2147483648&val7=-2147483649";
Url args;
int8_t int8val;
@@ -195,9 +195,9 @@ void TestIntValue(void)
void TestUintValue(void)
{
char uint8url[] = "spinel:///dev/ttyUSB0?no-reset&val1=1&val2=0x02&val3=0X03&val4=-4&val5=+5&val6=256&val7=-1";
char uint16url[] = "spinel:///dev/ttyUSB0?val1=1&val2=0x02&val3=0X03&val4=-400&val5=+500&val6=65536&val7=-1";
char uint32url[] =
char uint8url[200] = "spinel:///dev/ttyUSB0?no-reset&val1=1&val2=0x02&val3=0X03&val4=-4&val5=+5&val6=256&val7=-1";
char uint16url[200] = "spinel:///dev/ttyUSB0?val1=1&val2=0x02&val3=0X03&val4=-400&val5=+500&val6=65536&val7=-1";
char uint32url[200] =
"spinel:///dev/ttyUSB0?val1=1&val2=0x02&val3=0X03&val4=-40000&val5=+70000&val6=4294967296&val7=-1";
Url args;
uint8_t uint8val;
@@ -264,6 +264,58 @@ void TestUintValue(void)
printf("PASS %s\r\n", __func__);
}
void TestValidate(void)
{
char url[100] = "spinel:///dev/ttyUSB0?val1=1&val2=0x02&val3=0X03";
Url args;
const char *unusedParam = nullptr;
VerifyOrQuit(!args.Init(url));
// Test idempotency
VerifyOrQuit(args.Validate(&unusedParam) == OT_ERROR_INVALID_ARGS);
VerifyOrQuit(!strncmp(unusedParam, "val1=1", 6));
VerifyOrQuit(args.Validate(&unusedParam) == OT_ERROR_INVALID_ARGS);
VerifyOrQuit(!strncmp(unusedParam, "val1=1", 6));
VerifyOrQuit(!strcmp(args.GetValue("val1"), "1"));
VerifyOrQuit(args.Validate(&unusedParam) == OT_ERROR_INVALID_ARGS);
VerifyOrQuit(!strncmp(unusedParam, "val2=0x02", 9));
VerifyOrQuit(!strcmp(args.GetValue("val2"), "0x02"));
VerifyOrQuit(args.Validate(&unusedParam) == OT_ERROR_INVALID_ARGS);
VerifyOrQuit(!strncmp(unusedParam, "val3=0X03", 9));
VerifyOrQuit(!strcmp(args.GetValue("val3"), "0X03"));
VerifyOrQuit(args.Validate(&unusedParam) == OT_ERROR_NONE);
// Test with multiple same-named parameters
{
char url2[100] = "spinel:///dev/tty?arg=1&arg=2&val=3";
Url args2;
VerifyOrQuit(!args2.Init(url2));
VerifyOrQuit(args2.Validate(&unusedParam) == OT_ERROR_INVALID_ARGS);
VerifyOrQuit(!strncmp(unusedParam, "arg=1", 5));
const char *val = args2.GetValue("arg");
VerifyOrQuit(!strcmp(val, "1"));
VerifyOrQuit(args2.Validate(&unusedParam) == OT_ERROR_INVALID_ARGS);
VerifyOrQuit(!strncmp(unusedParam, "arg=2", 5));
val = args2.GetValue("arg", val);
VerifyOrQuit(!strcmp(val, "2"));
VerifyOrQuit(args2.Validate(&unusedParam) == OT_ERROR_INVALID_ARGS);
VerifyOrQuit(!strncmp(unusedParam, "val=3", 5));
val = args2.GetValue("val");
VerifyOrQuit(!strcmp(val, "3"));
VerifyOrQuit(args2.Validate(&unusedParam) == OT_ERROR_NONE);
}
printf("PASS %s\r\n", __func__);
}
} // namespace Url
} // namespace ot
@@ -276,6 +328,7 @@ int main(void)
ot::Url::TestMultipleProtocolsAndDuplicateParameters();
ot::Url::TestIntValue();
ot::Url::TestUintValue();
ot::Url::TestValidate();
return 0;
}