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

Enable application control over .well-known/core response #1428

Merged
merged 1 commit into from
May 31, 2024

Conversation

anyc
Copy link
Contributor

@anyc anyc commented May 29, 2024

As described in #1425, this PR makes it possible for the application to control what is returned for .well-known/core requests.

@anyc anyc force-pushed the develop branch 5 times, most recently from fe0ba6f to eb1685b Compare May 29, 2024 14:03
@anyc
Copy link
Contributor Author

anyc commented May 29, 2024

I do not understand what the last documentation issue is about.

@mrdeep1
Copy link
Collaborator

mrdeep1 commented May 29, 2024

I do not understand what the last documentation issue is about.

You need to update the man page checker with

diff --git a/man/examples-code-check.c b/man/examples-code-check.c
index f242671b..9cf997d2 100644
--- a/man/examples-code-check.c
+++ b/man/examples-code-check.c
@@ -125,6 +125,7 @@ const char *number_list[] = {
   "coap_pdu_type_t ",
   "coap_mid_t ",
   "coap_pdu_code_t ",
+  "coap_print_status_t ",
   "coap_proto_t ",
   "coap_session_state_t ",
   "coap_session_type_t ",

Then run man/examples-code-check man to check that all is now working..

Copy link
Collaborator

@mrdeep1 mrdeep1 left a comment

Choose a reason for hiding this comment

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

Thanks for doing this work. Some changes are needed.

include/coap3/coap_resource.h Outdated Show resolved Hide resolved
include/coap3/coap_net.h Outdated Show resolved Hide resolved
include/coap3/coap_net.h Outdated Show resolved Hide resolved
include/coap3/coap_resource.h Outdated Show resolved Hide resolved
man/Makefile.am Outdated Show resolved Hide resolved
include/coap3/coap_resource.h Outdated Show resolved Hide resolved
include/coap3/coap_resource_internal.h Outdated Show resolved Hide resolved
src/coap_resource.c Outdated Show resolved Hide resolved
src/coap_resource.c Outdated Show resolved Hide resolved
src/coap_resource.c Outdated Show resolved Hide resolved
@mrdeep1
Copy link
Collaborator

mrdeep1 commented May 31, 2024

@anyc it would be good to get the code updated, squashed into 1 commit ready to be merged so it can be added into 4.3.5rc1 which will be happening shortly.

This is needed for any reverse proxy to pass on .well-known/core.

@anyc
Copy link
Contributor Author

anyc commented May 31, 2024

Ah, I thought it would be easier to review if I split it into smaller commits. I pushed a new commit where only the coap_handler issue remains unsolved.

@anyc
Copy link
Contributor Author

anyc commented May 31, 2024

But it seems it does not work anymore with the new locking changes:

../git/src/coap_net.c:2727: get_wkc_len: Assertion `(context) && coap_thread_pid == ((context)->lock.being_freed ? (context)->lock.freeing_pid : (context)->lock.pid)' failed.

(gdb) back
#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
#1  0x76c7a390 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78
#2  0x76c36e70 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x76c215f8 in __GI_abort () at abort.c:79
#4  0x76c30308 in __assert_fail_base (fmt=0x76d4e220 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", 
    assertion=0x76f78d10 "(context) && coap_thread_pid == ((context)->lock.being_freed ? (context)->lock.freeing_pid : (context)->lock.pid)", 
    assertion@entry=0x76ff2020 "", file=0x76f7b154 "../git/src/coap_net.c", file@entry=0x76d4e220 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", 
    line=line@entry=2727, function=0x76f7c6bc <__PRETTY_FUNCTION__.3> "get_wkc_len", function@entry=0x76f7b154 "../git/src/coap_net.c") at assert.c:92
#5  0x76c303ac in __GI___assert_fail (assertion=0x76ff2020 "", file=0x76d4e220 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", line=line@entry=2727, 
    function=0x76f7b154 "../git/src/coap_net.c") at assert.c:101
#6  0x76f4af58 in get_wkc_len (query_filter=0x0, session=0x76d4e220, context=0x76ff2020) at ../git/src/coap_net.c:2727
#7  hnd_get_wellknown (resource=0x0, session=0x76d4e220, request=0x80, query=0x0, response=0x4baec0) at ../git/src/coap_net.c:2763
#8  0x76f4fea8 in handle_request (context=0x768d9b9c, context@entry=0x443c90, session=session@entry=0x4abc38, pdu=0xffffffe8, pdu@entry=0x4b0398)
    at ../git/src/coap_net.c:3446
#9  0x76f518c4 in coap_dispatch (context=context@entry=0x443c90, session=session@entry=0x4abc38, pdu=pdu@entry=0x4b0398) at ../git/src/coap_net.c:4148
#10 0x76f528d4 in coap_handle_dgram (ctx=0x443c90, session=session@entry=0x4abc38, 
    msg=msg@entry=0x7effebf4 "B\001y;QW\273.well-known\004core~@\354\377~e3c:de0", msg_len=23) at ../git/src/coap_net.c:2471
#11 0x76f59bec in coap_dtls_receive (session=session@entry=0x4abc38, data=<optimized out>, data_len=<optimized out>) at ../git/src/coap_openssl.c:3621
#12 0x76f52a44 in coap_handle_dgram_for_proto (ctx=ctx@entry=0x443c90, session=session@entry=0x4abc38, packet=packet@entry=0x7efff1f8)
    at ../git/src/coap_net.c:1947
#13 0x76f52b5c in coap_read_endpoint (ctx=0x443c90, endpoint=endpoint@entry=0x467170, now=<optimized out>) at ../git/src/coap_net.c:2232
#14 0x76f533cc in coap_io_do_epoll_lkd (ctx=ctx@entry=0x443c90, events=events@entry=0x7efff898, nevents=nevents@entry=1) at ../git/src/coap_net.c:2359
#15 0x76f4a808 in coap_io_process_with_fds_lkd (ctx=ctx@entry=0x443c90, timeout_ms=<optimized out>, timeout_ms@entry=4294967295, enfds=enfds@entry=0, 
    ereadfds=ereadfds@entry=0x0, ewritefds=ewritefds@entry=0x0, eexceptfds=eexceptfds@entry=0x0) at ../git/src/coap_io.c:1731
#16 0x76f4aa40 in coap_io_process_lkd (ctx=ctx@entry=0x443c90, timeout_ms=timeout_ms@entry=4294967295) at ../git/src/coap_io.c:1527
#17 0x76f4aa8c in coap_io_process (ctx=0x443c90, timeout_ms=timeout_ms@entry=4294967295) at ../git/src/coap_io.c:1520

@anyc
Copy link
Contributor Author

anyc commented May 31, 2024

Maybe this helps:

(gdb) print(context)
$1 = (coap_context_t *) 0x76ff2020
(gdb) print(coap_thread_pid) 
No symbol "coap_thread_pid" in current context.
(gdb) print((context)->lock.freeing_pid)
$2 = 0
(gdb) print((context)->lock.pid)
$3 = 0

@mrdeep1
Copy link
Collaborator

mrdeep1 commented May 31, 2024

Looking at why locking is currently failing with your code.

@mrdeep1
Copy link
Collaborator

mrdeep1 commented May 31, 2024

I can see the issue. hnd_get_wellknown() is being treated as an application handler, not as an internal libcoap handler. Thinking through how best to handle this as context does need to be locked (for multi-thread safety) as context->resources is getting iterated through in coap_print_wellknown_lkd().

@mrdeep1
Copy link
Collaborator

mrdeep1 commented May 31, 2024

Try this fix
well_known.patch.zip

@anyc
Copy link
Contributor Author

anyc commented May 31, 2024

Seems to work, thank you. I also added the request object as an additional parameter to the callback signature as I need this if I am going to use async functions.

include/coap3/coap_session_internal.h Outdated Show resolved Hide resolved
man/coap_resource.txt.in Outdated Show resolved Hide resolved
@mrdeep1
Copy link
Collaborator

mrdeep1 commented May 31, 2024

Looks good - thanks for getting this together.

@mrdeep1 mrdeep1 merged commit 7c33808 into obgm:develop May 31, 2024
31 checks passed
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