Skip to content

Refuse a pending dataset write while one is in place #275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
agners merged 1 commit into from
Sep 3, 2026
+81 −20
Merged

Refuse a pending dataset write while one is in place #275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions python_otbr_api/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,17 +336,36 @@ async def create_pending_dataset(self, dataset: PendingDataSet) -> None:

The passed in PendingDataSet does not need to be fully populated, any fields
not set will be automatically set by the open thread border router.
Raises if the http status is 400 or higher or if the response is invalid.

A write while a pending dataset is in flight is refused outright, for the
same reasons set_pending_dataset_tlvs refuses it: superseding one races
the delay timer on every device that already holds the old dataset, so a
late replacement can split the mesh, and it would also silently undo
whatever the in-flight dataset was doing, such as a channel change.

The check runs locally first, and again on the border router for one
that honors If-None-Match on this endpoint
(https://github.com/openthread/ot-br-posix/pull/3552), where it is
atomic with the write; older border routers ignore the header.

Raises PendingDatasetConflictError when either check refuses the write,
and OTBRError if the http status is 400 or higher for any other reason
or the response is invalid.
"""
if await self.get_pending_dataset_tlvs() is not None:
raise PendingDatasetConflictError("a pending dataset is already in place")
await self._maybe_detect_key_format()
response = await self._session.put(
f"{self._url}/node/dataset/pending",
json=self._encode(dataset.as_json()),
headers={"If-None-Match": "*"},
timeout=aiohttp.ClientTimeout(total=self._timeout),
)

if response.status == HTTPStatus.CONFLICT:
raise ThreadNetworkActiveError
if response.status == HTTPStatus.PRECONDITION_FAILED:
raise PendingDatasetConflictError("a pending dataset is already in place")
if response.status not in (HTTPStatus.CREATED, HTTPStatus.OK):
raise OTBRError(f"unexpected http status {response.status}")

Expand Down Expand Up @@ -424,15 +443,14 @@ async def set_channel(
"""Change the channel

The channel is changed by creating a new pending dataset based on the active
dataset. If a pending dataset is already in place, the change is refused:
stamping from the active dataset alone would make the mesh silently ignore
it while the router accepts the write, and superseding the pending dataset
would race its delay timer on devices that miss the update.
dataset. If a pending dataset is already in place, create_pending_dataset
refuses the write and raises PendingDatasetConflictError: stamping from the
active dataset alone would make the mesh silently ignore it while the router
accepts the write, and superseding the pending dataset would race its delay
timer on devices that miss the update.
"""
if not 11 <= channel <= 26:
raise OTBRError(f"invalid channel {channel}")
if await self.get_pending_dataset_tlvs() is not None:
raise OTBRError("a pending dataset is already in place")
if not (dataset := await self.get_active_dataset()):
raise OTBRError("router has no active dataset")

Expand Down
69 changes: 56 additions & 13 deletions tests/test_init_legacy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,13 +310,15 @@ async def test_create_pending_dataset(aioclient_mock: AiohttpClientMocker):
BASE_URL, aioclient_mock.create_session(), key_format=KeyFormat.PASCAL_CASE
)

aioclient_mock.get(f"{BASE_URL}/node/dataset/pending", status=HTTPStatus.NO_CONTENT)
aioclient_mock.put(f"{BASE_URL}/node/dataset/pending", status=HTTPStatus.CREATED)

await otbr.create_pending_dataset(python_otbr_api.PendingDataSet())
assert aioclient_mock.call_count == 1
assert aioclient_mock.call_count == 2
assert aioclient_mock.mock_calls[-1][0] == "PUT"
assert aioclient_mock.mock_calls[-1][1].path == "/node/dataset/pending"
assert aioclient_mock.mock_calls[-1][2] == {}
assert aioclient_mock.mock_calls[-1][3]["If-None-Match"] == "*"

await otbr.create_pending_dataset(
python_otbr_api.PendingDataSet(
Expand All @@ -325,7 +327,7 @@ async def test_create_pending_dataset(aioclient_mock: AiohttpClientMocker):
python_otbr_api.Timestamp(),
)
)
assert aioclient_mock.call_count == 2
assert aioclient_mock.call_count == 4
assert aioclient_mock.mock_calls[-1][0] == "PUT"
assert aioclient_mock.mock_calls[-1][1].path == "/node/dataset/pending"
assert aioclient_mock.mock_calls[-1][2] == {
Expand All @@ -342,7 +344,7 @@ async def test_create_pending_dataset(aioclient_mock: AiohttpClientMocker):
23456,
)
)
assert aioclient_mock.call_count == 3
assert aioclient_mock.call_count == 6
assert aioclient_mock.mock_calls[-1][0] == "PUT"
assert aioclient_mock.mock_calls[-1][1].path == "/node/dataset/pending"
assert aioclient_mock.mock_calls[-1][2] == {
Expand Down Expand Up @@ -393,9 +395,9 @@ async def test_set_channel(aioclient_mock: AiohttpClientMocker) -> None:
await otbr.set_channel(new_channel, 1234)
assert aioclient_mock.call_count == 3
assert aioclient_mock.mock_calls[0][0] == "GET"
assert aioclient_mock.mock_calls[0][1].path == "/node/dataset/pending"
assert aioclient_mock.mock_calls[0][1].path == "/node/dataset/active"
assert aioclient_mock.mock_calls[1][0] == "GET"
assert aioclient_mock.mock_calls[1][1].path == "/node/dataset/active"
assert aioclient_mock.mock_calls[1][1].path == "/node/dataset/pending"
assert aioclient_mock.mock_calls[2][0] == "PUT"
assert aioclient_mock.mock_calls[2][1].path == "/node/dataset/pending"
assert aioclient_mock.mock_calls[2][2] == expected_pending_dataset
Expand Down Expand Up @@ -425,9 +427,9 @@ async def test_set_channel_default_delay(aioclient_mock: AiohttpClientMocker) ->
await otbr.set_channel(new_channel)
assert aioclient_mock.call_count == 3
assert aioclient_mock.mock_calls[0][0] == "GET"
assert aioclient_mock.mock_calls[0][1].path == "/node/dataset/pending"
assert aioclient_mock.mock_calls[0][1].path == "/node/dataset/active"
assert aioclient_mock.mock_calls[1][0] == "GET"
assert aioclient_mock.mock_calls[1][1].path == "/node/dataset/active"
assert aioclient_mock.mock_calls[1][1].path == "/node/dataset/pending"
assert aioclient_mock.mock_calls[2][0] == "PUT"
assert aioclient_mock.mock_calls[2][1].path == "/node/dataset/pending"
assert aioclient_mock.mock_calls[2][2] == expected_pending_dataset
Expand Down Expand Up @@ -460,9 +462,9 @@ async def test_set_channel_no_timestamp(aioclient_mock: AiohttpClientMocker) ->
await otbr.set_channel(new_channel)
assert aioclient_mock.call_count == 3
assert aioclient_mock.mock_calls[0][0] == "GET"
assert aioclient_mock.mock_calls[0][1].path == "/node/dataset/pending"
assert aioclient_mock.mock_calls[0][1].path == "/node/dataset/active"
assert aioclient_mock.mock_calls[1][0] == "GET"
assert aioclient_mock.mock_calls[1][1].path == "/node/dataset/active"
assert aioclient_mock.mock_calls[1][1].path == "/node/dataset/pending"
assert aioclient_mock.mock_calls[2][0] == "PUT"
assert aioclient_mock.mock_calls[2][1].path == "/node/dataset/pending"
assert aioclient_mock.mock_calls[2][2] == expected_pending_dataset
Expand All @@ -484,7 +486,6 @@ async def test_set_channel_no_dataset(aioclient_mock: AiohttpClientMocker) -> No
BASE_URL, aioclient_mock.create_session(), key_format=KeyFormat.PASCAL_CASE
)

aioclient_mock.get(f"{BASE_URL}/node/dataset/pending", status=HTTPStatus.NO_CONTENT)
aioclient_mock.get(f"{BASE_URL}/node/dataset/active", status=HTTPStatus.NO_CONTENT)

with pytest.raises(python_otbr_api.OTBRError):
Expand All @@ -498,7 +499,8 @@ async def test_set_channel_rejected_while_pending(

Stamping from the active dataset alone would make the mesh silently ignore
the write, and superseding the pending dataset would race its delay timer
on devices that miss the update; refusing is the only honest answer.
on devices that miss the update; refusing is the only honest answer. The
refusal comes from create_pending_dataset, so nothing is written.
"""
otbr = python_otbr_api.OTBR(
BASE_URL, aioclient_mock.create_session(), key_format=KeyFormat.PASCAL_CASE
Expand All @@ -509,11 +511,13 @@ async def test_set_channel_rejected_while_pending(
"0708FD17C9D59809B27A05107546326F20BCCFD946609FBAF7F39AD5030F4F70656E5468726561"
"642D32366363010226CC0410FA7EC34EBE58DD1FD74F13F65D021C5B0C0402A0F7F8"
)
aioclient_mock.get(f"{BASE_URL}/node/dataset/active", json=DATASET_JSON)
aioclient_mock.get(f"{BASE_URL}/node/dataset/pending", text=mock_response)

with pytest.raises(python_otbr_api.OTBRError):
with pytest.raises(python_otbr_api.PendingDatasetConflictError):
await otbr.set_channel(16)
assert aioclient_mock.call_count == 1
assert not [call for call in aioclient_mock.mock_calls if call[0] == "PUT"]
assert aioclient_mock.call_count == 2


async def test_get_extended_address(aioclient_mock: AiohttpClientMocker) -> None:
Expand Down Expand Up @@ -681,6 +685,7 @@ async def test_create_pending_dataset_thread_active(
BASE_URL, aioclient_mock.create_session(), key_format=KeyFormat.PASCAL_CASE
)

aioclient_mock.get(f"{BASE_URL}/node/dataset/pending", status=HTTPStatus.NO_CONTENT)
aioclient_mock.put(f"{BASE_URL}/node/dataset/pending", status=HTTPStatus.CONFLICT)

with pytest.raises(python_otbr_api.ThreadNetworkActiveError):
Expand All @@ -693,6 +698,7 @@ async def test_create_pending_dataset_202(aioclient_mock: AiohttpClientMocker):
BASE_URL, aioclient_mock.create_session(), key_format=KeyFormat.PASCAL_CASE
)

aioclient_mock.get(f"{BASE_URL}/node/dataset/pending", status=HTTPStatus.NO_CONTENT)
aioclient_mock.put(f"{BASE_URL}/node/dataset/pending", status=HTTPStatus.ACCEPTED)

with pytest.raises(python_otbr_api.OTBRError):
Expand Down Expand Up @@ -877,3 +883,40 @@ async def test_set_pending_dataset_tlvs_202(

with pytest.raises(python_otbr_api.OTBRError):
await otbr.set_pending_dataset_tlvs(b"")


async def test_create_pending_dataset_refused_while_pending(
aioclient_mock: AiohttpClientMocker,
) -> None:
"""Test a write is refused while a pending dataset is in place."""
otbr = python_otbr_api.OTBR(
BASE_URL, aioclient_mock.create_session(), key_format=KeyFormat.PASCAL_CASE
)

in_flight = "0E080000000000010000340400006699000300000C"
aioclient_mock.get(f"{BASE_URL}/node/dataset/pending", text=in_flight)

with pytest.raises(python_otbr_api.PendingDatasetConflictError):
await otbr.create_pending_dataset(python_otbr_api.PendingDataSet())
assert aioclient_mock.call_count == 1


async def test_create_pending_dataset_refused_by_router(
aioclient_mock: AiohttpClientMocker,
) -> None:
"""Test the border router refusing the precondition is surfaced.

A pending dataset created between the local check and the write is only
caught by the router, which evaluates If-None-Match atomically with it.
"""
otbr = python_otbr_api.OTBR(
BASE_URL, aioclient_mock.create_session(), key_format=KeyFormat.PASCAL_CASE
)

aioclient_mock.get(f"{BASE_URL}/node/dataset/pending", status=HTTPStatus.NO_CONTENT)
aioclient_mock.put(
f"{BASE_URL}/node/dataset/pending", status=HTTPStatus.PRECONDITION_FAILED
)

with pytest.raises(python_otbr_api.PendingDatasetConflictError):
await otbr.create_pending_dataset(python_otbr_api.PendingDataSet())