Skip to content
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.

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

Add SVCB dohpath support #739

Merged
merged 12 commits into from
Jul 3, 2023
Merged

Add SVCB dohpath support #739

merged 12 commits into from
Jul 3, 2023

Conversation

TCY16
Copy link
Contributor

@TCY16 TCY16 commented Aug 15, 2022

No description provided.

Copy link
Member

@wcawijngaards wcawijngaards left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer an is printable character check on the output, so even if the input is binary, the output is printable text. Otherwise, it looks fine.

sldns/wire2str.c Outdated Show resolved Hide resolved
testdata/svcb.tdir/svcb.success-cases.zone.cmp Outdated Show resolved Hide resolved
Copy link
Member

@gthess gthess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you miss the MUST in section 5.1

If the "alpn" SvcParam indicates support for HTTP, "dohpath" MUST be present.

I would also like to see failure-cases testing, like the above and ?dns missing from the dohpath.

Also more happy testing for printing non-printable characters?

sldns/str2wire.c Outdated Show resolved Hide resolved
sldns/wire2str.c Outdated Show resolved Hide resolved
sldns/wire2str.c Outdated Show resolved Hide resolved
sldns/str2wire.c Outdated Show resolved Hide resolved
@TCY16 TCY16 requested a review from gthess November 22, 2022 09:31
@gthess gthess added this to the 1.17.1 milestone Nov 29, 2022
@gthess gthess modified the milestones: 1.17.1, 1.18.0 Jan 3, 2023
Copy link
Member

@gthess gthess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now and the other comments were addressed.

@gthess gthess merged commit 1962991 into master Jul 3, 2023
1 check passed
@gthess gthess deleted the features/dohpath-for-unbound branch July 3, 2023 09:03
jedisct1 added a commit to jedisct1/unbound that referenced this pull request Jul 15, 2023
* nlnet/master: (43 commits)
  - More clear description of the different auth-zone behaviors on the   man page.
  - Merge NLnetLabs#880 from chipitsine: services/authzone.c: remove redundant   check.
  - Merge NLnetLabs#664 from tilan7763: Add prefetch support for subnet cache   entries. - For NLnetLabs#664: Easier code flow for subnetcache prefetching. - For NLnetLabs#664: Add testcase. - For NLnetLabs#664: Rename subnet_prefetch tests to subnet_global_prefetch to   differentiate from the new subnet prefetch support.
  - For NLnetLabs#664: easier code flow for subnetcache prefetching. - For NLnetLabs#664: add testcase.
  - Skip the 00-lint test. splint is not maintained; it either does not   work or produces false positives. Static analysis is handled in the   clang test.
  - For NLnetLabs#802: Cleanup comments and add RCODE check for CD bit test case.
  - Fix dereference of NULL variable warning in mesh_do_callback.
  - Code cleanup for sldns_str2wire_svcparam_key_lookup.
  - Fix NLnetLabs#906: warning: ‘Py_SetProgramName’ is deprecated.
  - For NLnetLabs#739: minor cleanup for testcases.
  - More fixes for reference counting for python module and clean up   failure code.
  - Fix python modules with multiple scripts, by incrementing reference   counts.
  - Remove warning about unknown cast-function-type warning pragma.
  Add changelog and contrib/README mention for NLnetLabs#903 - Merge NLnetLabs#903: contrib: add yocto compatible init script.
  contrib: add yocto compatible init script
  Changelog for NLnetLabs#887 and NLnetLabs#516
  - Properly handle all return values of worker_check_request during   early EDE code. - Do not check the incoming request more than once.
  - Fix for uncertain unit test for doh buffer size events.
  Changelog note for NLnetLabs#895 - Fix NLnetLabs#895: python + sysconfig gives ANOTHER path comparing to   distutils.
  - Merge NLnetLabs#896: Fix: NLnetLabs#895: pythonmodule: add all site-packages   directories to sys.path.
  ...
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.

None yet

4 participants