From 34ceb4bf34d1dd682ba01c86397e4417f63f6c7f Mon Sep 17 00:00:00 2001
From: niamtokik <github@steepath.eu>
Date: Sun, 26 Dec 2021 11:52:25 +0000
Subject: [PATCH] Fix typo in documentation and fix issues on kvs_fs

It started with a simple documentation review, and ended with some
fixes on kvs_fs:

  - use filename:join instead of concatenation (crafted key can
    write data anywhere on the system)

  - add kvs_fs:delete/2 supports
---
 man/kvs.htm           |  2 +-
 man/kvs_fs.htm        | 41 +++++++++++++++++++++++++++++
 src/stores/kvs_fs.erl | 61 ++++++++++++++++++++++++++++++++++---------
 3 files changed, 91 insertions(+), 13 deletions(-)

diff --git a/man/kvs.htm b/man/kvs.htm
index 7d5be6a..c5498fd 100644
--- a/man/kvs.htm
+++ b/man/kvs.htm
@@ -59,7 +59,7 @@ <h4 id="index">index(atom(),any(),any()) -> list(tuple()).</h4>
     <section>
         <h3>SEQ</h3>
         <p>Sequence table id_seq stores the counter per thing.
-           The couners are global and atomic in each supported database.
+           The counters are global and atomic in each supported database.
            Sequences are used to generate unique names for records per distributed table.
            If names in the table are not unique, e.g.
            then count function may return a different value than the current sequence.</p>
diff --git a/man/kvs_fs.htm b/man/kvs_fs.htm
index 6e3b6cc..7a522c6 100644
--- a/man/kvs_fs.htm
+++ b/man/kvs_fs.htm
@@ -27,6 +27,47 @@ <h3>INTRO</h3>
    <p>FS is a <b>filesystem</b> backend implementation for KVS.
       Put the {dba,kvs_fs} property for the kvs application in your sys.config.</p>
    <br />
+
+   <h3>EXAMPLES</h3>
+
+   <blockquote>All examples can be executed in Erlang REPL. Only
+     requirement is to have kvs set in dependencies and kvs application
+     started.</blockquote>
+   
+   <p>Create a new kvs_fs table called test. When executed, this
+     function will create a new directory called test under data
+     directory in the current workspace.</p>
+   
+   <figure><code>
+       kvs_fs:create_table(test, []).
+       % will return:
+       % ok
+   </code></figure>
+
+   <p>Put a new key/value in table test. When executed, a new file is
+     created under test directory. Its name is based on the sha1 of
+     the key encoded in base64.</p>
+
+   <figure><code>
+       Table = test.
+       Key = key.
+       Value = <<"my_value">>.
+       kvs_fs:put({test, Key, Value}).
+       % will return:
+       % ok
+   </code></figure>
+
+   <p>Get a key from value test.</p>
+
+   <figure><code>
+       kvs_fs:get(test, key).
+       % will return:
+       % {ok, {test, key, <<"my_value">>}}
+   </code></figure>
+
+
+   <p>Delete a key</p>
+   
    </section>
     <section>
 <p>This module may refer to:
diff --git a/src/stores/kvs_fs.erl b/src/stores/kvs_fs.erl
index 784c22b..a00fd81 100644
--- a/src/stores/kvs_fs.erl
+++ b/src/stores/kvs_fs.erl
@@ -6,12 +6,19 @@
 -export(?BACKEND).
 
 start()    -> ok.
+
 stop()     -> ok.
+
 destroy()  -> ok.
+
 version()  -> {version,"KVS FS"}.
+
 leave()    -> ok.
+
 dir()      -> [ {table,F} || F <- filelib:wildcard("data/*"), filelib:is_dir(F) ].
-join(_Node) -> filelib:ensure_dir("data/"), initialize(). % should be rsync or smth
+
+join(_Node) -> filelib:ensure_dir(dir_name()), initialize(). % should be rsync or smth
+
 initialize() ->
     mnesia:create_schema([node()]),
     [ kvs:initialize(kvs_fs,Module) || Module <- kvs:modules() ],
@@ -19,29 +26,59 @@ initialize() ->
 
 index(_Tab,_Key,_Value) -> [].
 get(TableName, Key) ->
-    HashKey = encode(base64:encode(crypto:hash(sha, term_to_binary(Key)))),
-    Dir = lists:concat(["data/",TableName,"/"]),
-    case file:read_file(lists:concat([Dir,HashKey])) of
+    HashKey = hashkey(Key),
+    {ok, Dir} = dir(TableName),
+    File = filename:join([Dir,HashKey]),
+    case file:read_file(File) of
          {ok,Binary} -> {ok,binary_to_term(Binary,[safe])};
          {error,Reason} -> {error,Reason} end.
+
 put(Records) when is_list(Records) -> lists:map(fun(Record) -> put(Record) end, Records);
 put(Record) ->
     TableName = element(1,Record),
-    HashKey = encode(base64:encode(crypto:hash(sha, term_to_binary(element(2,Record))))),
+    HashKey = hashkey(element(2,Record)),
     BinaryValue = term_to_binary(Record),
-    Dir = lists:concat(["data/",TableName,"/"]),
-    filelib:ensure_dir(Dir),
-    File = lists:concat([Dir,HashKey]),
+    {ok, Dir} = dir(TableName),
+    File = filename:join([Dir,HashKey]),
+    filelib:ensure_dir(File),
     file:write_file(File,BinaryValue,[write,raw,binary,sync]).
 
-delete(_Tab, _Key) -> case kvs:get(_Tab,_Key) of {ok,_} -> ok; {error,X} -> {error,X} end.
-count(RecordName) -> length(filelib:fold_files(lists:concat(["data/",RecordName]), "",true, fun(A,Acc)-> [A|Acc] end, [])).
+dir_name() -> "data".
+
+dir(TableName) ->
+    {ok, Cwd} = file:get_cwd(),
+    TablePath = filename:join([dir_name(),TableName]),
+    case filelib:safe_relative_path(TablePath, Cwd) of
+        unsafe -> {error, {unsafe, TablePath}};
+        Target -> {ok, Target}
+    end.
+
+hashkey(Key) -> encode(base64:encode(crypto:hash(sha, term_to_binary(Key)))).
+
+delete(TableName, Key) ->
+    case kvs_fs:get(TableName, Key) of
+        {ok,_} ->
+            {ok, Dir} = dir(TableName),
+            HashKey = hashkey(Key),
+            File = filename:join([Dir,HashKey]),
+            file:delete(File);
+        {error,X} -> {error,X}
+    end.
+
+count(RecordName) -> length(filelib:fold_files(filename:join([dir_name(), RecordName]), "",true, fun(A,Acc)-> [A|Acc] end, [])).
+
 all(R) -> lists:flatten([ begin case file:read_file(File) of
                         {ok,Binary} -> binary_to_term(Binary,[safe]);
                         {error,_Reason} -> [] end end || File <-
-      filelib:fold_files(lists:concat(["data/",R]), "",true, fun(A,Acc)-> [A|Acc] end, []) ]).
+      filelib:fold_files(filename:join([dir_name(), R]), "",true, fun(A,Acc)-> [A|Acc] end, []) ]).
+
 seq(RecordName, Incr) -> kvs_mnesia:seq(RecordName, Incr).
-create_table(Name,_Options) -> filelib:ensure_dir(lists:concat(["data/",Name,"/"])).
+
+create_table(Name,_Options) ->
+    {ok, Dir} = dir(Name),
+    file:make_dir(Dir),
+    filelib:ensure_dir(Dir).
+
 add_table_index(_Record, _Field) -> ok.
 
 % URL ENCODE