gcoap/fileserver: add file and directory creation and deletion #18133

fabian18 · 2022-05-24T10:30:26Z

Contribution description

With this PR I added write support for the gcoap fileserver, using POST, PUT and DELETE methods.

~~POST: used to create files and directories~~
PUT : creates files or directories or updates files in a random access fashion
DELETE: delete files and directories

CoAP options If-Match and If-None-Match are added and handled in PUT requests.

Somehow directory deletion does not work for me. I end up in littlefs2/lfs.c:1457.

// check if we fit
lfs_size_t dsize = lfs_tag_dsize(tag);
if (commit->off + dsize > commit->end) {
    return LFS_ERR_NOSPC;
}

~~I don´t know why. Maybe it works for you.~~
Works with #18141

Testing procedure

Flash examples/gcoap_fileserver to a board with Ethernet support, connect with your PC and use a CoAP client to upload some files to the board´s VFS.

RIOT shell

> vfs ls /nvm0
2022-05-24 11:53:27,139 # vfs ls /nvm0
2022-05-24 11:53:27,144 # ./
2022-05-24 11:53:27,144 # ../
2022-05-24 11:53:27,163 # anotherdir/
2022-05-24 11:53:27,189 # cacert.der    425 B
2022-05-24 11:53:27,221 # cakey.der     121 B
2022-05-24 11:53:27,260 # capubkey.der  91 B
2022-05-24 11:53:27,263 # total 3 files
vfs ls /nvm0
2022-05-24 11:53:47,834 # vfs ls /nvm0
2022-05-24 11:53:47,839 # ./
2022-05-24 11:53:47,840 # ../
2022-05-24 11:53:47,854 # RIOT.txt      1402 B
2022-05-24 11:53:47,877 # anotherdir/
2022-05-24 11:53:47,908 # cacert.der    425 B
2022-05-24 11:53:47,944 # cakey.der     121 B
2022-05-24 11:53:47,986 # capubkey.der  91 B
2022-05-24 11:53:47,987 # total 4 files
> vfs ls /nvm0
2022-05-24 11:55:30,836 # vfs ls /nvm0
2022-05-24 11:55:30,843 # ./
2022-05-24 11:55:30,843 # ../
2022-05-24 11:55:30,864 # anotherdir/
2022-05-24 11:55:30,892 # cacert.der    425 B
2022-05-24 11:55:30,926 # cakey.der     121 B
2022-05-24 11:55:30,962 # capubkey.der  91 B
2022-05-24 11:55:31,007 # tmp/
2022-05-24 11:55:31,008 # total 3 files

I test with libcoap example client. and CFLAGS += -DCONFIG_GNRC_IPV6_NIB_ARSM=1

Host shell

$ coap-client -m put -O 27,0x00 -a fe80::3478:9157:9fd5:81c3%eth0 coap://[fe80::4c49:6fff:fe54:0]/vfs/RIOT.txt -f ~/RIOT.txt 
$ coap-client -m get -a fe80::3478:9157:9fd5:81c3%eth0 coap://[fe80::4c49:6fff:fe54:0]/vfs/RIOT.txt
                          ZZZZZZ
                        ZZZZZZZZZZZZ
                      ZZZZZZZZZZZZZZZZ
                     ZZZZZZZ     ZZZZZZ
                    ZZZZZZ        ZZZZZ
                    ZZZZZ          ZZZZ
                    ZZZZ           ZZZZZ
                    ZZZZ           ZZZZ
                    ZZZZ          ZZZZZ
                    ZZZZ        ZZZZZZ
                    ZZZZ     ZZZZZZZZ       777        7777       7777777777
              ZZ    ZZZZ   ZZZZZZZZ         777      77777777    77777777777
          ZZZZZZZ   ZZZZ  ZZZZZZZ           777     7777  7777       777
        ZZZZZZZZZ   ZZZZ    Z               777     777    777       777
       ZZZZZZ       ZZZZ                    777     777    777       777
      ZZZZZ         ZZZZ                    777     777    777       777
     ZZZZZ          ZZZZZ    ZZZZ           777     777    777       777
     ZZZZ           ZZZZZ    ZZZZZ          777     777    777       777
     ZZZZ           ZZZZZ     ZZZZZ         777     777    777       777
     ZZZZ           ZZZZ       ZZZZZ        777     777    777       777
     ZZZZZ         ZZZZZ        ZZZZZ       777     777    777       777
      ZZZZZZ     ZZZZZZ          ZZZZZ      777     7777777777       777
       ZZZZZZZZZZZZZZZ            ZZZZ      777      77777777        777
         ZZZZZZZZZZZ               Z
            ZZZZZ 

$ coap-client -m delete -a fe80::3478:9157:9fd5:81c3%eth0 coap://[fe80::4c49:6fff:fe54:0]/vfs/RIOT.txt
$ coap-client -m put -a fe80::3478:9157:9fd5:81c3%eth0 coap://[fe80::4c49:6fff:fe54:0]/vfs/tmp/
$ coap-client -m delete -a fe80::3478:9157:9fd5:81c3%eth0 coap://[fe80::4c49:6fff:fe54:0]/vfs/tmp/

Issues/PRs references

sys/net/application_layer/gcoap/fileserver.c

chrysn · 2022-05-24T12:29:32Z

From a CoAP point of view, I think that PUT should be used for file creation -- this has not only better properties (w/rt idempotency) but is also aligned with what WebDAV does. (POST would have a place if you want to create a file whose name you don't care about, but I don't think that that is a common use case).

fabian18 · 2022-05-24T13:21:31Z

PUT should be used for file creation

Indeed PUT can also be used for resource creation according to RFC7252. The handlers for POST and PUT anyways show some code duplication. I should be able to reduce it to just PUT.

chrysn · 2022-05-24T13:37:28Z

should be able to reduce it to just PUT.

I think that makes things simper.

It also aligns it with the CoAP file service draft. I feel obliged to point out that that is very drafty and has not even been properly discussed. If you want to consider that text for here, I'd rather hope for any disagreements between the text and the implementation would lead to a discussion on what good behavior is. (For example, this changes files non-atomically; maybe the text should rather not make promises unless one or the other behavior is advertised).

sys/net/application_layer/gcoap/fileserver.c

benpicco · 2022-05-26T20:40:22Z

sys/net/application_layer/gcoap/fileserver.c

+    return coap_opt_finish(pdu, COAP_OPT_FINISH_NONE);
+}
+
+static ssize_t _delete_directory(coap_pkt_t *pdu, uint8_t *buf, size_t len,


Is there the possibility to signal recursive deletion?
Or is the CoAP fileserver API fundamentally low-level?

Personally I am not interested in recursive deletion. Only empty directories can be deleted with vfs_rmdir,
If we want to implement recursive deletion later, we could define that a client must append a URI query DELETE: coap://server:port/path/to/resource?recursive=1. But this is just a first idea.

I suggest that recursive is the default. The UNIX behavior can be easily emulated with ETags (just pick a special ETag for empty directories on the server side, and insist that the client send an If-Match if a directory would be removed recursively), whereas the other direction is cumbersome to emulate by the client.

Do I understand this right?
If the client sends a DELETE on a directory, without an If-Match, the server should recursively delete the directory and all of its sub content.
If the client sends a DELETE on a directory, with an If-Match that contains the special Etag for empty directories, the server only deletes the directory if it is empty.

sys/net/application_layer/gcoap/fileserver.c

benpicco

If @chrysn has no objections to the server logic/API, this should be fine.

sys/net/application_layer/gcoap/fileserver.c

fabian18 · 2022-07-19T22:35:17Z

If @chrysn has no objections to the server logic/API, this should be fine.

I still need to implement the recursive DELETE. But I was not sure if I understood it right.
The coap fileserver draft could benefit from an example.

benpicco · 2022-07-19T23:29:03Z

Recursive delete might be generally useful, so best implement it in vfs_util

benpicco

Looks pretty good!
No need to wait for an approval to rebase btw to fix the merge conflicts 😉

Just one more thing: Can you make the write support a compile-time conditional (gcoap_fileserver_read_only? gcoap_fileserver_write?) so read-only applications can save a few bytes and don't have to worry about possible security implications.

sys/vfs_util/vfs_util.c

fabian18 · 2022-08-01T10:59:59Z

Can you make the write support a compile-time conditional

I am going to do that. I would add pseudomodules gcoap_fileserver_put that enables code in the PUT handler and gcoap_fileserver_delete that enables code in the DELETE handler.

fabian18 · 2022-08-01T11:56:03Z

Let me know if I can squash

benpicco · 2022-08-02T14:58:08Z

Please squash!

benpicco · 2022-08-02T15:00:00Z

sys/net/application_layer/gcoap/fileserver.c


 #define ENABLE_DEBUG 0
 #include "debug.h"

 /** Maximum length of an expressible path, including the trailing 0 character. */
 #define COAPFILESERVER_PATH_MAX (64)

+/** Randomly generated Etag, used by a client that a directory should only be
+    deleted, if it is empty */
+#define COAPFILESERVER_DIR_DELETE_ETAG (0x6ce88b56u)


If a client is supposed to use this, shouldn't this be made available in a header?

Also (sorry for jumping in so late, if this was discussed before), why is an ETag the signifier for this? Usually, with a REST API, this is done with some specific parameters (either as a query parameter or e.g. as a CBOR/JSON object).

Ah, I saw that this was proposed by @chrysn in #18133 (comment). But still, I am a bit confused about this

But still, I am a bit confused about this

The content of an If-Match option is an Etag.
Simply formulated: If the client sends that special Etag, it expects the directory to be empty. And if so, the request is processed, which means it is DELETEed. My first idea was indeed a query parameter, but I think the If-Match option works as well. Does this help or what exactly is unclear?

benpicco

Works like a charm!
For testing with coap-client I had to ignore the URI_HOST option as it would send it and it's a critical option - but we aren't doing vhosting, so any hostname will do.

Since it's just a two line change I've taken the liberty to just push that to your branch.

fabian18 · 2022-08-05T07:42:46Z

Awesome, Thank You!

fabian18 requested review from miri64, haukepetersen, PeterKietzmann and cgundogan as code owners May 24, 2022 10:30

github-actions bot added Area: CoAP Area: Constrained Application Protocol implementations Area: examples Area: Example Applications Area: network Area: Networking Area: sys Area: System Area: tools Area: Supplementary tools labels May 24, 2022

miri64 reviewed May 24, 2022

View reviewed changes

sys/net/application_layer/gcoap/fileserver.c Outdated Show resolved Hide resolved

miri64 reviewed May 24, 2022

View reviewed changes

sys/net/application_layer/gcoap/fileserver.c Outdated Show resolved Hide resolved

benpicco requested a review from chrysn May 24, 2022 11:07

benpicco reviewed May 26, 2022

View reviewed changes

fabian18 force-pushed the gcoap_fileserver_file_and_directory_creation branch from cce8a0c to cf39dcf Compare June 15, 2022 15:53

benpicco reviewed Jul 17, 2022

View reviewed changes

sys/net/application_layer/gcoap/fileserver.c Outdated Show resolved Hide resolved

sys/net/application_layer/gcoap/fileserver.c Show resolved Hide resolved

sys/net/application_layer/gcoap/fileserver.c Outdated Show resolved Hide resolved

benpicco reviewed Jul 31, 2022

View reviewed changes

sys/vfs_util/vfs_util.c Outdated Show resolved Hide resolved

sys/vfs_util/vfs_util.c Outdated Show resolved Hide resolved

sys/vfs_util/vfs_util.c Outdated Show resolved Hide resolved

fabian18 force-pushed the gcoap_fileserver_file_and_directory_creation branch from e26aa9c to 7a98128 Compare August 1, 2022 10:53

github-actions bot added the Area: build system Area: Build system label Aug 1, 2022

benpicco reviewed Aug 2, 2022

View reviewed changes

fabian18 added 3 commits August 3, 2022 22:09

gcoap/fileserver: implement PUT/POST/DELETE functionality

688acff

gcoap/fileserver: update documentation

2327bef

sys/vfs_util: add recursive unlink

2e58797

fabian18 added 3 commits August 3, 2022 22:09

sys/shell/commands: add recursive unlink to sc_vfs

70a8cf0

gcoap/fileserver: recursive directory deletion as default

f357d99

gcoap/fileserver: make PUT and DELETE pseudomodules

9333970

fabian18 force-pushed the gcoap_fileserver_file_and_directory_creation branch from 6636cdf to 9333970 Compare August 3, 2022 20:14

fabian18 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 3, 2022

gcoap/fileserver: ignore URI HOST option

199e31e

benpicco approved these changes Aug 4, 2022

View reviewed changes

benpicco merged commit c125e3d into RIOT-OS:master Aug 4, 2022

fabian18 mentioned this pull request Aug 30, 2022

gcoap_fileserver: bugfix recursive delete #18536

Merged

maribu added this to the Release 2022.10 milestone Oct 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gcoap/fileserver: add file and directory creation and deletion #18133

gcoap/fileserver: add file and directory creation and deletion #18133

fabian18 commented May 24, 2022 •

edited

Loading

chrysn commented May 24, 2022

fabian18 commented May 24, 2022 •

edited

Loading

chrysn commented May 24, 2022

benpicco May 26, 2022

fabian18 May 28, 2022

chrysn May 28, 2022

fabian18 May 29, 2022

benpicco left a comment •

edited

Loading

fabian18 commented Jul 19, 2022

benpicco commented Jul 19, 2022

benpicco left a comment

fabian18 commented Aug 1, 2022

fabian18 commented Aug 1, 2022

benpicco commented Aug 2, 2022

benpicco Aug 2, 2022

miri64 Aug 2, 2022

miri64 Aug 2, 2022

fabian18 Aug 3, 2022

benpicco left a comment

fabian18 commented Aug 5, 2022

gcoap/fileserver: add file and directory creation and deletion #18133

gcoap/fileserver: add file and directory creation and deletion #18133

Conversation

fabian18 commented May 24, 2022 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

chrysn commented May 24, 2022

fabian18 commented May 24, 2022 • edited Loading

chrysn commented May 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benpicco left a comment • edited Loading

Choose a reason for hiding this comment

fabian18 commented Jul 19, 2022

benpicco commented Jul 19, 2022

benpicco left a comment

Choose a reason for hiding this comment

fabian18 commented Aug 1, 2022

fabian18 commented Aug 1, 2022

benpicco commented Aug 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benpicco left a comment

Choose a reason for hiding this comment

fabian18 commented Aug 5, 2022

fabian18 commented May 24, 2022 •

edited

Loading

fabian18 commented May 24, 2022 •

edited

Loading

benpicco left a comment •

edited

Loading