Skip to content

feat(protocol): validate GET and POST request options - #48

Open
Jason-Morcos wants to merge 2 commits into
QuiteYellow:mainfrom
Moballo-LLC:codex/coap-request-options
Open

feat(protocol): validate GET and POST request options#48
Jason-Morcos wants to merge 2 commits into
QuiteYellow:mainfrom
Moballo-LLC:codex/coap-request-options

Conversation

@Jason-Morcos

@Jason-Morcos Jason-Morcos commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add repeated URI-query support to post() while preserving the existing get() query contract
  • add keyword-only extra_options to GET and POST for ordered OCF extension options
  • validate path/query text, option counts, numbers, ordering, value types, and sizes before sending
  • keep extension options on every token-stable Block2 GET request
  • reserve path, query, Observe, content-format, Accept, Block1, Block2, Size1, and Size2 for the transport

Why

Higher-level consumers need query-qualified resources and a narrow way to carry reviewed OCF extension options without constructing request datagrams themselves. The Home Assistant integration I use with local Samsung appliances needs this for OCF content-version and routing options, including across Block2 continuation requests.

This is the first PY-09 request-completeness slice. DELETE is intentionally separate so it can reuse this validation contract, and Block1 remains transport-owned in the later stack. Response interpretation, Observe relations, ownership transactions, and device policy are unchanged.

Stack / merge order

  1. fix(protocol): pace CoAP request sends #51 owns request and OBSERVE-registration pacing.
  2. feat(protocol): validate GET and POST request options #48 (this PR) adds validated query and extension options.
  3. feat(protocol): add CoAP DELETE requests #49 adds DELETE.
  4. feat(protocol): add bounded Block1 POST uploads #50 adds bounded Block1 POST uploads.

Please review this after #51. The request-option commit is 16e4608.

Validation

  • 304 SmartThings-Local tests against this exact head
  • 1,625 LocalThings tests against this exact source tree
  • focused Block2, reader-death, public API, and request-option coverage
  • bytecode compilation, diff checks, and share-safety checks
  • Python 3.11 dependency-floor and package smoke checks at both endpoints of the stack

@QuiteYellow

Copy link
Copy Markdown
Owner

I have not reviewed this yet, and the three of them have sat longer than they should have. Here is where it stands.

#51 and #47 are merged. #56 now holds the order for everything left, because #36, #48, #49, #50 and #54 all collide in dtls_session.py, and two of them build the same MID registry from opposite ends. Your three come after #36 there, on the reasoning that additive features re-express onto a rewritten substrate more easily than a rewrite absorbs three new features. If you disagree, #56 is the place to say so.

Two practical notes. This one and #49 and #50 still carry tests/test_request_pacing.py from when they were stacked on #51. That merged this morning, so a rebase onto main should shrink all three. And your declared order, #48 then #49 then #50, is what #56 proposes, so nothing to resequence.

The part I will look at hardest when I do review it is keeping extension options on every token-stable Block2 continuation request. Token-stable Block2 is where this codebase has historically got things wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants