nanocoap: add Uri-Query handling capabilities

[haukepetersen]

Uri-query strings are needed for e.g. lwm2m or CoAP resource directories (RD). This PR adds the capability to add COAP_OPT_URI_QUERY options to CoAP requests.

This PR adds an additional buffer to hold the raw query string in the coap_pkt_t structure. The alternative would be to write the query string into the url buffer, but this would make the parsing of options quite a bit harder, so I went with this (for now?!).

[kb2ma]

seperator -> separator

[kb2ma]

@haukepetersen, thanks for creating this PR. Overall, this work looks good to me. See the inline comment on a typo.

I don't see the decoding for a Uri-Query option in coap_parse() in this PR. Were you planning to create a follow-on? After the parse, how would the receiver of a message determine if a particular query key is included in the request? Add a function to text search coap_pkt_t.qs?

I agree that I'm not sure the complexity required to reuse the single 'url' attribute in coap_pkt_t to include the query is worth it for now. Below are my thoughts as I worked through it though.

Use of a single url buffer would give the user the option to combine the path and query into a single string when generating a request. For example, no changes would be needed for sock's client.c or for gcoap's gcoap_cli.c. On the other hand, I expect the more common use case would be addition of query parameters incrementally based on some conditions, as you have done with gcoap_add_qstring().

I'm not familiar with the details of RFC 3986, but sec. 3.4 says: "The query component is indicated by the first question mark ("?") character". Seems like not much extra code for coap_put_option_uri() when writing Uri-Path options.

At the same time, nanocoap's coap_handle_req() and gcoap's _handle_req() would need to use strncmp() and add logic to compare lengths between the paths in the request and a resource. I expect it would make sense to add an attribute to coap_pkt_t like 'url_query_pos' for the start of the query portion of the url, if any.

[haukepetersen]

fixed typo.

I have actually not spend much thought on the decoding and presentation of the Uri-Query when receiving packets - I basically go on a 'add what I need for the next step' flow currently :-)

But my current thought is to handle it in the following way: when receiving a packet that contains a query string, we copy the query string into coap_pkt_t->qs buffer. To access this than later in the user code, I would imagine something like a generic function, that gets a filter function and an output buffer as parameters, something like this:

coap_qs_filter(coap_pkt_t *pkt, coap_qs_filter_t filter_function, void *output_buffer);

This way, users can pass custom filter functions, that fit to their needs. For example a function that fills a custom struct holding values for ep, lt, d usw and ignoring all other query elements that it does not know. But as said, this is only a very early idea.

As general optimization we should however definitely think about getting rid of the additional buffers for url and qs all together and rather have two pointers in the coap_pkt_t struct that point to either places in the received packets buffer or to user defined external buffers...

[kb2ma]

The coap_qa_filter() approach makes sense to me for query parameters. gcoap itself doesn't have much interest in them, and I don't think any app gets much value out of the full query presented as a string. Some sort of customized or iterable approach seems more on target.

In general, the URL parameter is more usable as a complete string. However, even here RIOT has an open PR to group handling of resource names based on a common prefix. So, /app/foo, /app/bar, and /app/baz could be handled by a single /app/* resource.

I agree about the longer term goal to eliminate storage of url and qs.

[haukepetersen]

So aside from the uri-query handling on the receiving side, so you guys agree on the changes of this PR for now so we can proceed?

[kb2ma]

I will test this PR as part of reviewing RIOT #7402. As I mentioned there, I would prefer not to see the addition of the qs attribute, but can live with it as a short term solution.

[kb2ma]

RIOT #7402 tests fine on native. So, ACK here in the interest of moving forward.

[kaspar030]

I would prefer not to see the addition of the qs attribute, but can live with it as a short term solution.

+1, ACK.