[rest] honor If-None-Match on the dataset PUT endpoints by LorbusChris · Pull Request #3552 · openthread/ot-br-posix

LorbusChris · 2026-08-25T07:08:49Z

A pending dataset that is not newer than the one the mesh already holds is silently ignored while this handler accepts the write, so a client that did not know a dataset was in flight reports success for a change that never happens. The client can check first, but not atomically with the write.

Honor the RFC 9110 If-None-Match header with the value * on PUT /node/dataset/active and /node/dataset/pending: the write then only happens when the addressed endpoint has no dataset in place yet (no pending dataset on the pending endpoint, no active dataset on the active one), and answers 412 Precondition Failed otherwise, checked inside the main-loop task atomically with the write. Header values other than * answer 400 rather than running the write unconditionally.

The checks run in the order RFC 9110 section 13.2.1 prescribes: first the existing request checks (the active dataset is only writable while Thread is stopped, so on a running node the answer stays 409 and the header cannot downgrade it to a 412), then the precondition (412), and only then is the request content processed (400 for a malformed body) and the dataset written. Requests without the header keep the replace semantics unchanged, so existing clients are unaffected.

cc @agners re home-assistant-libs/python-otbr-api#269 (review)

gemini-code-assist

Code Review

This pull request implements support for the If-None-Match: * header in the REST API to allow conditional writes of active and pending operational datasets, returning a 412 Precondition Failed error if a dataset is already present. The changes include updates to the OpenAPI specification, the REST web server implementation, and corresponding integration tests. The review feedback suggests documenting the response body schema for PreconditionFailedError in the OpenAPI spec and simplifying the manual header validation in the web server using the standard VerifyOrExit macro.

Add set_pending_dataset_tlvs(), the pending-endpoint twin of set_active_dataset_tlvs(), so a controller can hand a border router a new operational dataset as a raw TLV pending set. The TLV parser gains the write-side counterparts callers need to build such a dataset: Timestamp.from_values(), with range checks for the 48-bit seconds and 15-bit ticks fields, and DelayTimer.from_milliseconds(). PENDINGTIMESTAMP and DELAYTIMER now decode to typed items, matching ACTIVETIMESTAMP. Also raise TLVError instead of leaking struct.error on a malformed timestamp or delay timer TLV, and log unknown TLVs by type and length only -- the value may hold network credentials. Like the channel change, the write is refused while a pending dataset is already in place, since a not-newer one would be silently ignored by the mesh. A caller that deliberately supersedes the in-flight dataset, having stamped the new one above it, passes allow_replace=True. The check runs locally first, and again on the border router: unless allow_replace is set the request carries "If-None-Match: *", which a border router with openthread/ot-br-posix#3552 evaluates atomically with the write, closing the race the local check leaves open. Older border routers ignore the header and keep relying on the local check. A 412 answer raises the same OTBRError as the local refusal. Assisted-By: Claude Fable 5

agners

Other than the small nit, LGTM! Nice RFC 9110 13.2.1 compliant implementation!

codecov · 2026-08-25T15:30:30Z

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 35.86%. Comparing base (2b41187) to head (ee7d7e6).
⚠️ Report is 1629 commits behind head on main.

Files with missing lines	Patch %	Lines
src/rest/rest_web_server.cpp	0.00%	8 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #3552       +/-   ##
===========================================
- Coverage   55.77%   35.86%   -19.92%     
===========================================
  Files          87      145       +58     
  Lines        6890    17927    +11037     
  Branches        0     1458     +1458     
===========================================
+ Hits         3843     6429     +2586     
- Misses       3047    11138     +8091     
- Partials        0      360      +360

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

A pending dataset that is not newer than the one the mesh already holds is silently ignored while this handler accepts the write, so a client that did not know a dataset was in flight reports success for a change that never happens. The client can check first, but not atomically with the write. Honor the RFC 9110 If-None-Match header with the value `*` on PUT /node/dataset/active and /node/dataset/pending: the write then only happens when the addressed endpoint has no dataset in place yet (no pending dataset on the pending endpoint, no active dataset on the active one), and answers 412 Precondition Failed otherwise, checked inside the main-loop task atomically with the write. Header values other than `*` answer 400 rather than running the write unconditionally. The checks run in the order RFC 9110 section 13.2.1 prescribes: first the existing request checks (the active dataset is only writable while Thread is stopped, so on a running node the answer stays 409 and the header cannot downgrade it to a 412), then the precondition (412), and only then is the request content processed (400 for a malformed body) and the dataset written. Requests without the header keep the replace semantics unchanged, so existing clients are unaffected. The header is not CORS-safelisted, so add it to the allowed request headers for browser clients. Assisted-By: Claude Opus 5 Signed-off-by: Christian Glombek <c.glombek@cosa.systems>

jwhui

Thanks 👍🏼

If-None-Match gives a client create-only semantics; what it cannot express is "replace, but only the dataset I read". A controller that reads a dataset, stamps a successor and writes it back has no way to detect another writer slipping in between the read and the write. Give the dataset resources entity tags: GET answers with an ETag derived from the stored TLVs (hashed, so the tag does not echo the credentials it stands for; the pending dataset's delay timer is left out of the hash since the running countdown does not make it a different dataset), and a successful PUT reports the tag of what it stored. The JSON representation is now rendered from the same stored TLV bytes the tag covers, rather than the struct getters. PUT honors If-Match per RFC 9110 section 13.1.1: the write only happens when one of the given entity tags matches the stored dataset, or, with `*`, when any dataset is stored. Tags are compared with the strong comparison function (section 8.8.3.2), and the precondition is evaluated before If-None-Match (section 13.2.2), atomically with the write in the main-loop task. A failed precondition answers 412, a syntactically invalid header value 400, and the role check on the active dataset still answers 409 first (section 13.2.1). This turns a read-stamp-write sequence into a compare-and-swap. Neither the header nor the entity tag is CORS-safelisted, so add If-Match to the allowed request headers and expose ETag to cross-origin scripts. Bump the REST API version to 0.6.0, covering the conditional-write capability this completes: openthread#3552 added If-None-Match, this adds entity tags and If-Match, and they ship as one version. Assisted-By: Claude Opus 5 Signed-off-by: Christian Glombek <c.glombek@cosa.systems>

If-None-Match gives a client create-only semantics; what it cannot express is "replace, but only the dataset I read". A controller that reads a dataset, stamps a successor and writes it back has no way to detect another writer slipping in between the read and the write. Give the dataset resources entity tags: GET answers with an ETag derived from the stored TLVs (hashed, so the tag does not echo the credentials it stands for), and a successful PUT reports the tag of what it stored. The JSON representation is now rendered from the same stored TLV bytes the tag covers, rather than the struct getters. The tag is weak (RFC 9110 section 8.8.1). A pending dataset's delay timer counts down, so a tag covering it would change every second and could never serve as a precondition; leaving it out is what a client doing a read-stamp-write actually needs, but the countdown is visible in the body a GET returns, and a strong validator has to change whenever anything in that body does. The same tag also stands for both the JSON and the TLV representation, which the specification calls weak on its own. Weak is what it offers for this case: grouping representations "by some self-determined set of equivalency rather than unique sequences of data". PUT honors If-Match per RFC 9110 section 13.1.1: the write only happens when one of the given entity tags matches the stored dataset, or, with `*`, when any dataset is stored. The precondition is evaluated before If-None-Match (section 13.2.2), atomically with the write in the main-loop task. A failed precondition answers 412, a syntactically invalid header value 400, and the role check on the active dataset still answers 409 first (section 13.2.1). This turns a read-stamp-write sequence into a compare-and-swap. Tags are compared with the weak function (section 8.8.3.2) rather than the strong one section 13.1.1 asks for. The two requirements cannot both be met here: strong comparison never matches a weak tag, and this resource cannot honestly offer a strong one. Comparing weakly keeps the deviation on the write side, where this server decides what counts as the same dataset, instead of putting it on the read side, where a strong-looking tag would mislead caches as well as clients. GET answers no-store. A dataset carries the network key and the PSKc, and a pending one carries a delay timer that is stale as soon as it is stored, so nothing on the way should keep a copy -- the more so now that an entity tag invites revalidation. Neither the header nor the entity tag is CORS-safelisted, so add If-Match to the allowed request headers and expose ETag to cross-origin scripts. Bump the REST API version to 0.6.0, covering the conditional-write capability this completes: openthread#3552 added If-None-Match, this adds entity tags and If-Match, and they ship as one version. Assisted-By: Claude Opus 5 Signed-off-by: Christian Glombek <c.glombek@cosa.systems>

Add set_pending_dataset_tlvs(), the pending-endpoint twin of set_active_dataset_tlvs(), so a controller can hand a border router a new operational dataset as a raw TLV pending set. The TLV parser gains the write-side counterparts callers need to build such a dataset: Timestamp.from_values(), with range checks for the 48-bit seconds and 15-bit ticks fields, and DelayTimer.from_milliseconds(). PENDINGTIMESTAMP and DELAYTIMER now decode to typed items, matching ACTIVETIMESTAMP. Also raise TLVError instead of leaking struct.error on a malformed timestamp or delay timer TLV, and log unknown TLVs by type and length only -- the value may hold network credentials. Like the channel change, the write is refused while a pending dataset is already in place, since a not-newer one would be silently ignored by the mesh. A caller that deliberately supersedes the in-flight dataset, having stamped the new one above it, passes allow_replace=True. The check runs locally first, and again on the border router: unless allow_replace is set the request carries "If-None-Match: *", which a border router with openthread/ot-br-posix#3552 evaluates atomically with the write, closing the race the local check leaves open. Older border routers ignore the header and keep relying on the local check. Both refusals raise PendingDatasetConflictError rather than a plain OTBRError, so a caller can tell them apart from a transport or protocol failure without matching on the message: nothing was written, and the state that made the write wrong is there to be read back. Assisted-By: Claude Fable 5

If-None-Match gives a client create-only semantics; what it cannot express is "replace, but only the dataset I read". A controller that reads a dataset, stamps a successor and writes it back has no way to detect another writer slipping in between the read and the write. Give the dataset resources entity tags: GET answers with an ETag derived from the stored TLVs (hashed, so the tag does not echo the credentials it stands for), and a successful PUT reports the tag of what it stored. The JSON representation is now rendered from the same stored TLV bytes the tag covers, rather than the struct getters. The tag is weak (RFC 9110 section 8.8.1). A pending dataset's delay timer counts down, so a tag covering it would change every second and could never serve as a precondition; leaving it out is what a client doing a read-stamp-write actually needs, but the countdown is visible in the body a GET returns, and a strong validator has to change whenever anything in that body does. The same tag also stands for both the JSON and the TLV representation, which the specification calls weak on its own. Weak is what it offers for this case: grouping representations "by some self-determined set of equivalency rather than unique sequences of data". PUT honors If-Match per RFC 9110 section 13.1.1: the write only happens when one of the given entity tags matches the stored dataset, or, with `*`, when any dataset is stored. The precondition is evaluated before If-None-Match (section 13.2.2), atomically with the write in the main-loop task. A failed precondition answers 412, a syntactically invalid header value 400, and the role check on the active dataset still answers 409 first (section 13.2.1). This turns a read-stamp-write sequence into a compare-and-swap. Tags are compared with the weak function (section 8.8.3.2) rather than the strong one section 13.1.1 asks for. The two requirements cannot both be met here: strong comparison never matches a weak tag, and this resource cannot honestly offer a strong one. Comparing weakly keeps the deviation on the write side, where this server decides what counts as the same dataset, instead of putting it on the read side, where a strong-looking tag would mislead caches as well as clients. GET answers no-store. A dataset carries the network key and the PSKc, and a pending one carries a delay timer that is stale as soon as it is stored, so nothing on the way should keep a copy -- the more so now that an entity tag invites revalidation. Neither the header nor the entity tag is CORS-safelisted, so add If-Match to the allowed request headers and expose ETag to cross-origin scripts. Bump the REST API version to 0.6.0, covering the conditional-write capability this completes: openthread#3552 added If-None-Match, this adds entity tags and If-Match, and they ship as one version. Assisted-By: Claude Opus 5 Signed-off-by: Christian Glombek <c.glombek@cosa.systems>

Add set_pending_dataset_tlvs(), the pending-endpoint twin of set_active_dataset_tlvs(), so a controller can hand a border router a new operational dataset as a raw TLV pending set. The TLV parser gains the write-side counterparts callers need to build such a dataset: Timestamp.from_values(), with range checks for the 48-bit seconds and 15-bit ticks fields, and DelayTimer.from_milliseconds(). PENDINGTIMESTAMP and DELAYTIMER now decode to typed items, matching ACTIVETIMESTAMP. Also raise TLVError instead of leaking struct.error on a malformed timestamp or delay timer TLV, and log unknown TLVs by type and length only -- the value may hold network credentials. Like the channel change, the write is refused while a pending dataset is already in place. A not-newer one would be silently ignored by the mesh, and superseding is never safe to do implicitly: the replacement 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. A caller that finds the write refused should surface that, not retry. The check runs locally first, and again on the border router: unless allow_replace is set the request carries "If-None-Match: *", which a border router with openthread/ot-br-posix#3552 evaluates atomically with the write, closing the race the local check leaves open. Older border routers ignore the header and keep relying on the local check. Both refusals raise PendingDatasetConflictError rather than a plain OTBRError, so a caller can tell them apart from a transport or protocol failure without matching on the message: nothing was written, and the state that made the write wrong is there to be read back. Assisted-By: Claude Fable 5

* Add pending-dataset TLV write support Add set_pending_dataset_tlvs(), the pending-endpoint twin of set_active_dataset_tlvs(), so a controller can hand a border router a new operational dataset as a raw TLV pending set. The TLV parser gains the write-side counterparts callers need to build such a dataset: Timestamp.from_values(), with range checks for the 48-bit seconds and 15-bit ticks fields, and DelayTimer.from_milliseconds(). PENDINGTIMESTAMP and DELAYTIMER now decode to typed items, matching ACTIVETIMESTAMP. Also raise TLVError instead of leaking struct.error on a malformed timestamp or delay timer TLV, and log unknown TLVs by type and length only -- the value may hold network credentials. Like the channel change, the write is refused while a pending dataset is already in place. A not-newer one would be silently ignored by the mesh, and superseding is never safe to do implicitly: the replacement 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. A caller that finds the write refused should surface that, not retry. The check runs locally first, and again on the border router: unless allow_replace is set the request carries "If-None-Match: *", which a border router with openthread/ot-br-posix#3552 evaluates atomically with the write, closing the race the local check leaves open. Older border routers ignore the header and keep relying on the local check. Both refusals raise PendingDatasetConflictError rather than a plain OTBRError, so a caller can tell them apart from a transport or protocol failure without matching on the message: nothing was written, and the state that made the write wrong is there to be read back. Assisted-By: Claude Fable 5 * Update pyproject.toml --------- Co-authored-by: Stefan Agner <stefan@agner.ch>

gemini-code-assist Bot reviewed Aug 25, 2026

View reviewed changes

Comment thread src/rest/openapi.yaml

Comment thread src/rest/rest_web_server.cpp Outdated

LorbusChris force-pushed the rest-dataset-if-none-match branch from 214bb08 to 616f2e9 Compare August 25, 2026 07:14

This was referenced Aug 25, 2026

Add pending-dataset TLV write support home-assistant-libs/python-otbr-api#269

Merged

Add an OTBR action to migrate the whole Thread network home-assistant/core#178291

Draft

[rest] add entity tags and If-Match to the dataset endpoints #3553

Draft

agners approved these changes Aug 25, 2026

View reviewed changes

Comment thread src/rest/openapi.yaml Outdated

jwhui reviewed Aug 25, 2026

View reviewed changes

Comment thread src/rest/rest_web_server.cpp Outdated

Comment thread src/rest/openapi.yaml Outdated

Comment thread src/rest/openapi.yaml Outdated

Comment thread tests/rest/test_rest.py Outdated

LorbusChris force-pushed the rest-dataset-if-none-match branch from 616f2e9 to a7cd09b Compare August 25, 2026 15:59

jwhui reviewed Aug 26, 2026

View reviewed changes

Comment thread tests/rest/test_rest.py Outdated

LorbusChris force-pushed the rest-dataset-if-none-match branch 2 times, most recently from f032f26 to eb13631 Compare August 26, 2026 15:31

LorbusChris force-pushed the rest-dataset-if-none-match branch from eb13631 to ee7d7e6 Compare August 26, 2026 16:18

jwhui approved these changes Aug 26, 2026

View reviewed changes

jwhui added enhancement comp: web service labels Aug 26, 2026

jwhui merged commit f6ef5a3 into openthread:main Aug 26, 2026
48 of 49 checks passed

agners mentioned this pull request Sep 3, 2026

Refuse a pending dataset write while one is in place home-assistant-libs/python-otbr-api#275

Open

Navigation Menu

Uh oh!

[rest] honor If-None-Match on the dataset PUT endpoints - #3552

[rest] honor If-None-Match on the dataset PUT endpoints#3552
jwhui merged 1 commit into
openthread:mainfrom
LorbusChris:rest-dataset-if-none-match

LorbusChris commented Aug 25, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

agners left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Aug 25, 2026 •

edited

Loading

Uh oh!

Uh oh!

jwhui left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

LorbusChris commented Aug 25, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

agners left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

jwhui left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Aug 25, 2026 •

edited

Loading