From 586c6b306004bd8ac089e3dc8b4c164fd9540d80 Mon Sep 17 00:00:00 2001
From: Andrew Bettison <andrew@servalproject.org>
Date: Mon, 10 Aug 2015 18:08:42 +0930
Subject: [PATCH 01/11] Add HTTP GET /restful/keyring/add

---
 keyring_restful.c    | 38 +++++++++++++++++++++++++++++++++----
 testdefs.sh          |  8 ++++++++
 tests/keyring        |  7 -------
 tests/keyringrestful | 45 +++++++++++++++++++++++++++++++-------------
 4 files changed, 74 insertions(+), 24 deletions(-)

diff --git a/keyring_restful.c b/keyring_restful.c
index f7d73d02..fed212ef 100644
--- a/keyring_restful.c
+++ b/keyring_restful.c
@@ -28,6 +28,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 #define alloca_keyring_token(bid, offset) keyring_    token_to_str(alloca(keyring_TOKEN_STRLEN + 1), (bid), (offset))
 
 static HTTP_HANDLER restful_keyring_identitylist_json;
+static HTTP_HANDLER restful_keyring_add;
 
 int restful_keyring_(httpd_request *r, const char *remainder)
 {
@@ -40,13 +41,16 @@ int restful_keyring_(httpd_request *r, const char *remainder)
   const char *verb = HTTP_VERB_GET;
   http_size_t content_length = CONTENT_LENGTH_UNKNOWN;
   HTTP_HANDLER *handler = NULL;
-  
   if (strcmp(remainder, "identities.json") == 0) {
     handler = restful_keyring_identitylist_json;
     verb = HTTP_VERB_GET;
     remainder = "";
   }
-
+  else if (strcmp(remainder, "add") == 0) {
+    handler = restful_keyring_add;
+    verb = HTTP_VERB_GET;
+    remainder = "";
+  }
   if (handler == NULL)
     return 404;
   if (	 content_length != CONTENT_LENGTH_UNKNOWN
@@ -60,16 +64,20 @@ int restful_keyring_(httpd_request *r, const char *remainder)
   return handler(r, remainder);
 }
 
+static int http_request_keyring_response(struct httpd_request *r, uint16_t result, const char *message)
+{
+  http_request_simple_response(&r->http, result, message);
+  return result;
+}
+
 static HTTP_CONTENT_GENERATOR restful_keyring_identitylist_json_content;
 
 static int restful_keyring_identitylist_json(httpd_request *r, const char *remainder)
 {
   if (*remainder)
     return 404;
-
   r->u.sidlist.phase = LIST_HEADER;
   keyring_iterator_start(keyring, &r->u.sidlist.it);
-
   http_request_response_generated(&r->http, 200, CONTENT_TYPE_JSON, restful_keyring_identitylist_json_content);
   return 1;
 }
@@ -147,3 +155,25 @@ static int restful_keyring_identitylist_json_content_chunk(struct http_request *
   return 0;
 }
 
+static int restful_keyring_add(httpd_request *r, const char *remainder)
+{
+  if (*remainder)
+    return 404;
+  const keyring_identity *id = keyring_create_identity(keyring, "");
+  if (id == NULL)
+    return http_request_keyring_response(r, 501, "Could not create identity");
+  const sid_t *sidp = NULL;
+  const char *did = "";
+  const char *name = "";
+  keyring_identity_extract(id, &sidp, &did, &name);
+  if (!sidp)
+    return http_request_keyring_response(r, 501, "New identity has no SID");
+  if (keyring_commit(keyring) == -1)
+    return http_request_keyring_response(r, 501, "Could not store new identity");
+  strbuf s = strbuf_alloca(200);
+  strbuf_puts(s, "{\n \"sid\":");
+  strbuf_json_hex(s, sidp->binary, sizeof sidp->binary);
+  strbuf_puts(s, "\n}");
+  http_request_response_static(&r->http, 200, CONTENT_TYPE_JSON, strbuf_str(s), strbuf_len(s));
+  return 1;
+}
diff --git a/testdefs.sh b/testdefs.sh
index 01782c82..942ae12a 100644
--- a/testdefs.sh
+++ b/testdefs.sh
@@ -759,6 +759,14 @@ create_identities() {
    done
 }
 
+# Assertion function:
+# - asserts that the list contains N identities that have the correct format
+assert_keyring_list() {
+   unpack_stdout_list __X
+   assert --stdout --stderr [ $__XNROWS -eq $1 ]
+   assertStdoutGrep --stderr --matches=$1 "^$rexp_sid:\($rexp_did\)\?:.*\$"
+}
+
 # Utility function, to be overridden as needed:
 #  - set up the configuration immediately prior to starting a servald server process
 #  - called by start_servald_instances
diff --git a/tests/keyring b/tests/keyring
index d043e965..8cbbf089 100755
--- a/tests/keyring
+++ b/tests/keyring
@@ -46,13 +46,6 @@ setup_instances() {
    done
 }
 
-assert_keyring_list() {
-    unpack_stdout_list X
-    assert --stdout --stderr [ $XNROWS -eq $1 ]
-    assertStdoutGrep --stderr --matches=$1 "^[0-9a-fA-F]\{64\}:[0-9*#+]*:.*\$"
-    tfw_cat --stdout
-}
-
 doc_KeyringCreate="Create keyring destroys existing keys"
 test_KeyringCreate() {
     executeOk_servald keyring add ''
diff --git a/tests/keyringrestful b/tests/keyringrestful
index 5f825a53..944b1c9e 100755
--- a/tests/keyringrestful
+++ b/tests/keyringrestful
@@ -29,6 +29,7 @@ setup() {
    setup_json
    setup_servald
    set_instance +A
+   set_keyring_config
    executeOk_servald config \
       set api.restful.users.harry.password potter \
       set api.restful.users.ron.password weasley \
@@ -60,7 +61,6 @@ set_extra_config() {
 
 set_keyring_config() {
    executeOk_servald config \
-      set debug.http_server on \
       set debug.httpd on \
       set debug.rhizome_manifest on \
       set debug.rhizome_store on \
@@ -70,27 +70,46 @@ set_keyring_config() {
       set log.console.level debug
 }
 
-doc_keyringListIdentities="HTTP RESTful list SIDs as JSON"
-setup_keyringListIdentities() {
+doc_keyringList="HTTP RESTful list keyring identities as JSON"
+setup_keyringList() {
    IDENTITY_COUNT=10
    setup
 }
-
-test_keyringListIdentities() {
+test_keyringList() {
    executeOk curl \
          --silent --fail --show-error \
-         --output identitylist1.json \
+         --output list.json \
          --dump-header http.headers \
          --basic --user harry:potter \
          "http://$addr_localhost:$PORTA/restful/keyring/identities.json"
-   tfw_cat http.headers identitylist1.json
-   tfw_preserve identitylist1.json
-   
-   assert [ "$(jq '.rows | length' identitylist1.json)" = $IDENTITY_COUNT ]
+   tfw_cat http.headers list.json
+   tfw_preserve list.json
+   assert [ "$(jq '.rows | length' list.json)" = $IDENTITY_COUNT ]
+   assert [ "$(jq -r '.rows[0][0]' list.json)" = $SIDA1 ]
+   assert [ "$(jq -r '.rows[4][0]' list.json)" = $SIDA5 ]
+   assert [ "$(jq -r '.rows[9][0]' list.json)" = $SIDA10 ]
+}
 
-   assert [ "$(jq -r '.rows[0][0]' identitylist1.json)" = $SIDA1 ]
-   assert [ "$(jq -r '.rows[4][0]' identitylist1.json)" = $SIDA5 ]
-   assert [ "$(jq -r '.rows[9][0]' identitylist1.json)" = $SIDA10 ]
+doc_keyringAdd="HTTP RESTful add keyring identity with empty PIN"
+setup_keyringAdd() {
+   IDENTITY_COUNT=2
+   setup
+}
+test_keyringAdd() {
+   executeOk curl \
+         --silent --show-error --write-out '%{http_code}' \
+         --output add.json \
+         --dump-header http.headers \
+         --basic --user harry:potter \
+         "http://$addr_localhost:$PORTA/restful/keyring/add"
+   tfw_cat http.headers add.json
+   tfw_preserve add.json
+   assertStdoutIs '200'
+   SID="$(jq -r '.sid' add.json)"
+   assert matches_rexp "^${rexp_sid}$" "$SID"
+   executeOk_servald keyring list
+   assert_keyring_list 3
+   assertStdoutGrep --stderr --matches=1 "^$SID::\$"
 }
 
 runTests "$@"

From e73d50b48a77abfbcaf2d26eeb1e87596b4a1b17 Mon Sep 17 00:00:00 2001
From: Andrew Bettison <andrew@servalproject.org>
Date: Mon, 17 Aug 2015 19:43:58 +0930
Subject: [PATCH 02/11] Refactor "str.h": rename parameters 'dstlen' to
 'dstsiz'

---
 str.c | 16 ++++++++--------
 str.h | 12 ++++++------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/str.c b/str.c
index 1eebb509..1b0ea787 100644
--- a/str.c
+++ b/str.c
@@ -60,11 +60,11 @@ int fromhexstr(unsigned char *dstBinary, const char *srcHex, size_t nbinary)
   return -1;
 }
 
-size_t strn_fromhex(unsigned char *dstBinary, ssize_t dstlen, const char *srcHex, const char **afterHex)
+size_t strn_fromhex(unsigned char *dstBinary, ssize_t dstsiz, const char *srcHex, const char **afterHex)
 {
   unsigned char *dstorig = dstBinary;
-  unsigned char *dstend = dstBinary + dstlen;
-  while (dstlen == -1 || dstBinary < dstend) {
+  unsigned char *dstend = dstBinary + dstsiz;
+  while (dstsiz == -1 || dstBinary < dstend) {
     int high = hexvalue(srcHex[0]);
     if (high == -1)
       break;
@@ -988,14 +988,14 @@ size_t strn_fromprint(unsigned char *dst, size_t dstsiz, const char *src, size_t
   return dst - odst;
 }
 
-void str_digest_passphrase(unsigned char *dstBinary, size_t dstlen, const char *passphrase)
+void str_digest_passphrase(unsigned char *dstBinary, size_t dstsiz, const char *passphrase)
 {
-  return strn_digest_passphrase(dstBinary, dstlen, passphrase, strlen(passphrase));
+  return strn_digest_passphrase(dstBinary, dstsiz, passphrase, strlen(passphrase));
 }
 
-void strn_digest_passphrase(unsigned char *dstBinary, size_t dstlen, const char *passphrase, size_t passlen)
+void strn_digest_passphrase(unsigned char *dstBinary, size_t dstsiz, const char *passphrase, size_t passlen)
 {
-  assert(dstlen <= SERVAL_PASSPHRASE_DIGEST_MAX_BINARY);
+  assert(dstsiz <= SERVAL_PASSPHRASE_DIGEST_MAX_BINARY);
   SHA512_CTX context;
   static const char salt1[] = "Sago pudding";
   static const char salt2[] = "Rhubarb pie";
@@ -1003,7 +1003,7 @@ void strn_digest_passphrase(unsigned char *dstBinary, size_t dstlen, const char
   SHA512_Update(&context, (unsigned char *)salt1, sizeof salt1 - 1);
   SHA512_Update(&context, (unsigned char *)passphrase, passlen);
   SHA512_Update(&context, (unsigned char *)salt2, sizeof salt2 - 1);
-  SHA512_Final_Len(dstBinary, dstlen, &context);
+  SHA512_Final_Len(dstBinary, dstsiz, &context);
 }
 
 /* Return true if the string resembles a URI.
diff --git a/str.h b/str.h
index 43a6e01c..1d8f53c4 100644
--- a/str.h
+++ b/str.h
@@ -126,7 +126,7 @@ int fromhexstr(unsigned char *dstBinary, const char *srcHex, size_t nbinary);
  *
  * @author Andrew Bettison <andrew@servalproject.com>
  */
-size_t strn_fromhex(unsigned char *dstBinary, ssize_t dstlen, const char *src, const char **afterp);
+size_t strn_fromhex(unsigned char *dstBinary, ssize_t dstsiz, const char *src, const char **afterp);
 
 /* -------------------- Base64 encoding and decoding -------------------- */
 
@@ -286,7 +286,7 @@ __SERVAL_DNA__STR_INLINE int hexvalue(char c) {
 /* -------------------- In-line string formatting -------------------- */
 
 size_t sprintf_len(const char *fmt, ...);
-#define alloca_sprintf(dstlen, fmt,...) strbuf_str(strbuf_sprintf(strbuf_alloca((dstlen) == -1 ? sprintf_len((fmt), ##__VA_ARGS__) + 1 : (size_t)(dstlen)), (fmt), ##__VA_ARGS__))
+#define alloca_sprintf(dstsiz, fmt,...) strbuf_str(strbuf_sprintf(strbuf_alloca((dstsiz) == -1 ? sprintf_len((fmt), ##__VA_ARGS__) + 1 : (size_t)(dstsiz)), (fmt), ##__VA_ARGS__))
 
 /* -------------------- Printable string representation -------------------- */
 
@@ -296,8 +296,8 @@ size_t toprint_len(const char *srcBuf, size_t srcBytes, const char quotes[2]);
 size_t toprint_str_len(const char *srcStr, const char quotes[2]);
 size_t strn_fromprint(unsigned char *dst, size_t dstsiz, const char *src, size_t srclen, char endquote, const char **afterp);
 
-#define alloca_toprint_quoted(dstlen,buf,len,quotes)  toprint((char *)alloca((dstlen) == -1 ? toprint_len((const char *)(buf),(len), (quotes)) + 1 : (size_t)(dstlen)), (size_t)(dstlen), (const char *)(buf), (len), (quotes))
-#define alloca_toprint(dstlen,buf,len)  alloca_toprint_quoted(dstlen,buf,len,"``")
+#define alloca_toprint_quoted(dstsiz,buf,len,quotes)  toprint((char *)alloca((dstsiz) == -1 ? toprint_len((const char *)(buf),(len), (quotes)) + 1 : (size_t)(dstsiz)), (size_t)(dstsiz), (const char *)(buf), (len), (quotes))
+#define alloca_toprint(dstsiz,buf,len)  alloca_toprint_quoted(dstsiz,buf,len,"``")
 
 #define alloca_str_toprint_quoted(str, quotes)  toprint_str((char *)alloca(toprint_str_len((str), (quotes)) + 1), -1, (str), (quotes))
 #define alloca_str_toprint(str)  alloca_str_toprint_quoted(str, "``")
@@ -311,8 +311,8 @@ size_t strn_fromprint(unsigned char *dst, size_t dstsiz, const char *src, size_t
  *
  * @author Andrew Bettison <andrew@servalproject.com>
  */
-void str_digest_passphrase(unsigned char *dstBinary, size_t dstlen, const char *passphrase);
-void strn_digest_passphrase(unsigned char *dstBinary, size_t dstlen, const char *passphrase, size_t passlen);
+void str_digest_passphrase(unsigned char *dstBinary, size_t dstsiz, const char *passphrase);
+void strn_digest_passphrase(unsigned char *dstBinary, size_t dstsiz, const char *passphrase, size_t passlen);
 
 /* -------------------- Useful string primitives -------------------- */
 

From 0a40d9849c365ff99b412f204d1a2291d45fc6ea Mon Sep 17 00:00:00 2001
From: Andrew Bettison <andrew@servalproject.org>
Date: Mon, 17 Aug 2015 19:46:50 +0930
Subject: [PATCH 03/11] Add uri and www-form-uri encode/decode functions

---
 str.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 str.h |  50 ++++++++++++++++++++++++++-
 2 files changed, 155 insertions(+), 1 deletion(-)

diff --git a/str.c b/str.c
index 1b0ea787..ef5aa2e7 100644
--- a/str.c
+++ b/str.c
@@ -81,6 +81,112 @@ size_t strn_fromhex(unsigned char *dstBinary, ssize_t dstsiz, const char *srcHex
   return dstBinary - dstorig;
 }
 
+static size_t _uri_encodev(int www_form, char *const dstUrienc, ssize_t dstsiz, struct iovec ** iovp, int *iovcntp)
+{
+  char * dst = dstUrienc;
+  char * const dstend = dstUrienc + dstsiz;
+  while (*iovcntp && (dstsiz == -1 || dst < dstend)) {
+    if ((*iovp)->iov_len == 0) {
+      --*iovcntp;
+      ++*iovp;
+    } else {
+      unsigned char c = *(unsigned char *)(*iovp)->iov_base;
+      if (www_form && c == ' ') {
+	if (dstUrienc)
+	  *dst = '+';
+	++dst;
+      } else if (is_uri_char_unreserved(c)) {
+	if (dstUrienc)
+	  *dst = c;
+	++dst;
+      } else if (dst + 3 <= dstend) {
+	if (dstUrienc) {
+	  dst[0] = '%';
+	  dst[1] = hexdigit_upper[c & 0xf];
+	  dst[2] = hexdigit_upper[c >> 4];
+	}
+	dst += 3;
+      } else {
+	break;
+      }
+      ++(*iovp)->iov_base;
+      --(*iovp)->iov_len;
+    }
+  }
+  return dst - dstUrienc;
+}
+
+static size_t _uri_encode(int www_form, char *const dstUrienc, ssize_t dstsiz, const char *src, size_t srclen, const char **afterp)
+{
+  struct iovec _iov;
+  _iov.iov_base = (void *) src;
+  _iov.iov_len = srclen;
+  struct iovec *iov = &_iov;
+  int ioc = 1;
+  size_t encoded = _uri_encodev(www_form, dstUrienc, dstsiz, &iov, &ioc);
+  if (afterp)
+    *afterp = _iov.iov_base;
+  return encoded;
+}
+
+size_t uri_encode(char *const dstUrienc, ssize_t dstsiz, const char *src, size_t srclen, const char **afterp)
+{
+  return _uri_encode(0, dstUrienc, dstsiz, src, srclen, afterp);
+}
+
+size_t www_form_uri_encode(char *const dstUrienc, ssize_t dstsiz, const char *src, size_t srclen, const char **afterp)
+{
+  return _uri_encode(1, dstUrienc, dstsiz, src, srclen, afterp);
+}
+
+size_t uri_encodev(char *const dstUrienc, ssize_t dstsiz, struct iovec ** iovp, int *iovcntp)
+{
+  return _uri_encodev(0, dstUrienc, dstsiz, iovp, iovcntp);
+}
+
+size_t www_form_uri_encodev(char *const dstUrienc, ssize_t dstsiz, struct iovec ** iovp, int *iovcntp)
+{
+  return _uri_encodev(1, dstUrienc, dstsiz, iovp, iovcntp);
+}
+
+static size_t _uri_decode(int www_form, char *const dstOrig, ssize_t dstsiz, const char *srcUrienc, size_t srclen, const char **afterp)
+{
+  char *dst = dstOrig;
+  char *const dstend = dst + dstsiz;
+  while (srclen && (dstsiz == -1 || dst < dstend)) {
+    if (www_form && *srcUrienc == '+') {
+      if (dstOrig)
+	*dst = ' ';
+      ++srcUrienc;
+      --srclen;
+    } else if (srclen >= 3 && srcUrienc[0] == '%' && isxdigit(srcUrienc[1]) && isxdigit(srcUrienc[2])) {
+      if (dstOrig)
+	*dst = (hexvalue(srcUrienc[1]) << 4) + hexvalue(srcUrienc[2]);
+      srcUrienc += 3;
+      srclen -= 3;
+    } else {
+      if (dstOrig)
+	*dst = *srcUrienc;
+      ++srcUrienc;
+      --srclen;
+    }
+    ++dst;
+  }
+  if (afterp)
+    *afterp = srcUrienc;
+  return dst - dstOrig;
+}
+
+size_t uri_decode(char *const dst, ssize_t dstsiz, const char *srcUrienc, size_t srclen, const char **afterp)
+{
+  return _uri_decode(0, dst, dstsiz, srcUrienc, srclen, afterp);
+}
+
+size_t www_form_uri_decode(char *const dst, ssize_t dstsiz, const char *srcUrienc, size_t srclen, const char **afterp)
+{
+  return _uri_decode(1, dst, dstsiz, srcUrienc, srclen, afterp);
+}
+
 const char base64_symbols[65] = {
   'A','B','C','D','E','F','G','H','I','J','K','L','M','N','O','P',
   'Q','R','S','T','U','V','W','X','Y','Z','a','b','c','d','e','f',
diff --git a/str.h b/str.h
index 1d8f53c4..eb58b499 100644
--- a/str.h
+++ b/str.h
@@ -495,7 +495,55 @@ int uint64_scaled_to_str(char *str, size_t len, uint64_t value);
  */
 int str_to_uint64_interval_ms(const char *str, int64_t *result, const char **afterp);
 
-/* -------------------- URI strings -------------------- */
+/* -------------------- URI encoding and decoding -------------------- */
+
+/* Encode up to 'srclen' bytes of byte data (or up to first nul if 'srclen' == -1) at 'src' into at
+ * most 'dstsiz' bytes of URI-encoded (or www-form-urlencoded) representation at 'dstUrienc'.  If
+ * 'dstsiz' is -1 or 'dstUrienc' is NULL, does not write any encoded bytes, but still counts them.
+ * If 'afterp' is not NULL, then sets *afterp to point to the source byte immediately following the
+ * last character encoded.  A "%xx" sequence will never be partially encoded; if all the "%xx" does
+ * not fit within the destination buffer, then none of it is produced.
+ *
+ *
+ * Returns the total number of encoded bytes written at 'dstUrienc'.
+ *
+ * Can be used to count encoded bytes without actually encoding, eg:
+ *
+ *    uri_encode(NULL, -1, buf, buflen, NULL);
+ *
+ * The uri_encodev() and www_form_uri_encodev() functions are a multi-buffer gather variants,
+ * analagous to readv(2) and writev(2).  Modifies the supplied *iovp, *iovcntp parameters and the
+ * iovec structures at (*iovp)[...] to represent the remaining source bytes not encoded.
+ *
+ * @author Andrew Bettison <andrew@servalproject.com>
+ */
+size_t uri_encode(char *const dstUrienc, ssize_t dstsiz, const char *src, size_t srclen, const char **afterp);
+size_t www_form_uri_encode(char *const dstUrienc, ssize_t dstsiz, const char *src, size_t srclen, const char **afterp);
+
+size_t uri_encodev(char *const dstUrienc, ssize_t dstsiz, struct iovec **iovp, int *iovcntp); // modifies *iovp, (*iovp)[...] and *iovcntp
+size_t www_form_uri_encodev(char *const dstUrienc, ssize_t dstsiz, struct iovec **iovp, int *iovcntp); // modifies *iovp, (*iovp)[...] and *iovcntp
+
+/* Decode up to 'srclen' bytes of URI-encoded (or www-form-urlencoded) data at 'srcUrienc' into at
+ * most 'dstsiz' bytes at 'dst'.  If 'dstsiz' is -1 or 'dst' is NULL, then does not write any
+ * decoded bytes, but still counts them.  If 'afterp' is not NULL, then sets *afterp to point to the
+ * source byte immediately following the last byte decoded.
+ *
+ * Returns the total number of decoded bytes written at 'dst'.
+ *
+ * Can be used to decode in-place, eg:
+ *
+ *    uri_decode((char *)buf, n, (const unsigned char *)buf, n, NULL);
+ *
+ * Can be used to count decoded bytes without actually decoding, eg:
+ *
+ *    uri_decode(NULL, -1, buf, buflen, NULL);
+ *
+ * @author Andrew Bettison <andrew@servalproject.com>
+ */
+size_t uri_decode(char *const dst, ssize_t dstsiz, const char *srcUrienc, size_t srclen, const char **afterp);
+size_t www_form_uri_decode(char *const dst, ssize_t dstsiz, const char *srcUrienc, size_t srclen, const char **afterp);
+
+/* -------------------- URI parsing -------------------- */
 
 /* Return true if the string resembles a nul-terminated URI.
  * Based on RFC-3986 generic syntax, assuming nothing about the hierarchical part.

From ce7a6ba988da8f78e7c832d23067c30414c89f04 Mon Sep 17 00:00:00 2001
From: Andrew Bettison <andrew@servalproject.org>
Date: Mon, 17 Aug 2015 19:47:25 +0930
Subject: [PATCH 04/11] HTTP server: parse query parameters

---
 http_server.c | 135 ++++++++++++++++++++++++++++++++++++++++----------
 http_server.h |  12 +++++
 2 files changed, 121 insertions(+), 26 deletions(-)

diff --git a/http_server.c b/http_server.c
index 12e84c80..fef23a6d 100644
--- a/http_server.c
+++ b/http_server.c
@@ -114,8 +114,9 @@ void http_request_init(struct http_request *r, int sockfd)
   r->alarm.poll.events = POLLIN;
   r->phase = RECEIVE;
   r->reserved = r->buffer;
-  // Put aside a few bytes for reserving strings, so that the path can be reserved ok.
-  r->received = r->end = r->parsed = r->cursor = r->buffer + 32;
+  // Put aside a few bytes for reserving strings, so that the path and query parameters can be
+  // reserved ok.
+  r->received = r->end = r->parsed = r->cursor = r->buffer + sizeof(void*) * (1 + NELS(r->query_parameters));
   r->parser = http_request_parse_verb;
   watch(&r->alarm);
   http_request_set_idle_timeout(r);
@@ -215,11 +216,9 @@ static void *read_pointer(const unsigned char *mem)
   return v;
 }
 
-/* Allocate space from the start of the request buffer to hold the given substring plus a
+/* Allocate space from the start of the request buffer to hold a given number of bytes plus a
  * terminating NUL.  Enough bytes must have already been marked as parsed in order to make room,
- * otherwise the reservation fails and returns 0.  If successful, copies the substring plus a
- * terminating NUL into the reserved space, places a pointer to the reserved area into '*resp', and
- * returns 1.
+ * otherwise the reservation fails and returns 0.  If successful, returns 1.
  *
  * Keeps a copy to the pointer 'resp', so that when the reserved area is released, all pointers into
  * it can be set to NULL automatically.  This provides some safety: if the pointer is accidentally
@@ -229,14 +228,11 @@ static void *read_pointer(const unsigned char *mem)
  *
  * @author Andrew Bettison <andrew@servalproject.com>
  */
-static int _reserve(struct http_request *r, const char **resp, struct substring str)
+static int _reserve(struct http_request *r, const char **resp, const char *src, size_t len, void (*mover)(char *, const char *, size_t))
 {
   // Reserved string pointer must lie within this http_request struct.
   assert((char*)resp >= (char*)r);
   assert((char*)resp < (char*)(r + 1));
-  size_t len = str.end - str.start;
-  // Substring must contain no NUL chars.
-  assert(strnchr(str.start, len, '\0') == NULL);
   char *reslim = r->buffer + sizeof r->buffer - 1024; // always leave this much unreserved space
   assert(r->reserved <= reslim);
   size_t siz = sizeof(char**) + len + 1;
@@ -245,17 +241,15 @@ static int _reserve(struct http_request *r, const char **resp, struct substring
     return 0;
   }
   if (r->reserved + siz > r->parsed) {
-    WARNF("Error during HTTP parsing, unparsed content %s would be overwritten by reserving %s",
-	alloca_toprint(30, r->parsed, r->end - r->parsed),
-	alloca_substring_toprint(str)
+    WARNF("Error during HTTP parsing, unparsed content %s would be overwritten by reserving %zu bytes",
+	alloca_toprint(30, r->parsed, r->end - r->parsed), len + 1
       );
     r->response.result_code = 500;
     return 0;
   }
   const char ***respp = (const char ***) r->reserved;
   char *restr = (char *)(respp + 1);
-  if (restr != str.start)
-    memmove(restr, str.start, len);
+  mover(restr, src, len);
   restr[len] = '\0';
   r->reserved += siz;
   assert(r->reserved == &restr[len+1]);
@@ -269,6 +263,25 @@ static int _reserve(struct http_request *r, const char **resp, struct substring
   return 1;
 }
 
+static void _mover_mem(char *dst, const char *src, size_t len)
+{
+  if (dst != src)
+    memmove(dst, src, len);
+}
+
+/* Allocate space from the start of the request buffer to hold the given substring plus a
+ * terminating NUL.
+ *
+ * @author Andrew Bettison <andrew@servalproject.com>
+ */
+static int _reserve_substring(struct http_request *r, const char **resp, struct substring str)
+{
+  size_t len = str.end - str.start;
+  // Substring must contain no NUL chars.
+  assert(strnchr(str.start, len, '\0') == NULL);
+  return _reserve(r, resp, str.start, len, _mover_mem);
+}
+
 /* The same as _reserve(), but takes a NUL-terminated string as a source argument instead of a
  * substring.
  *
@@ -276,8 +289,26 @@ static int _reserve(struct http_request *r, const char **resp, struct substring
  */
 static int _reserve_str(struct http_request *r, const char **resp, const char *str)
 {
-  struct substring sub = { .start = str, .end = str + strlen(str) };
-  return _reserve(r, resp, sub);
+  return _reserve(r, resp, str, strlen(str), _mover_mem);
+}
+
+/* The same as _reserve(), but decodes the source bytes using www-form-urlencoding.
+ *
+ * @author Andrew Bettison <andrew@servalproject.com>
+ */
+static void _mover_www_form_uri_decode(char *, const char *, size_t);
+static int _reserve_www_form_uriencoded(struct http_request *r, const char **resp, struct substring str)
+{
+  assert(str.end > str.start);
+  const char *after = NULL;
+  size_t len = www_form_uri_decode(NULL, -1, (char *)str.start, str.end - str.start, &after);
+  assert(len <= (size_t)(str.end - str.start)); // decoded must not be longer than encoded
+  assert(after == str.end);
+  return _reserve(r, resp, str.start, len, _mover_www_form_uri_decode);
+}
+static void _mover_www_form_uri_decode(char *dst, const char *src, size_t len)
+{
+  www_form_uri_decode(dst, len, src, -1, NULL);
 }
 
 /* Release all the strings reserved by _reserve(), returning the space to the request buffer, and
@@ -457,17 +488,17 @@ static inline int _skip_space(struct http_request *r)
   return r->cursor > start;
 }
 
-static size_t _skip_word_printable(struct http_request *r, struct substring *str)
+static size_t _skip_word_printable(struct http_request *r, struct substring *str, char until)
 {
-  if (_run_out(r) || isspace(*r->cursor) || !isprint(*r->cursor))
+  if (_run_out(r) || isspace(*r->cursor) || !isprint(*r->cursor) || *r->cursor == until)
     return 0;
   const char *start = r->cursor;
-  for (++r->cursor; !_run_out(r) && !isspace(*r->cursor) && isprint(*r->cursor); ++r->cursor)
+  for (++r->cursor; !_run_out(r) && !isspace(*r->cursor) && isprint(*r->cursor) && *r->cursor != until; ++r->cursor)
     ;
   if (_run_out(r))
     return 0;
   assert(r->cursor > start);
-  assert(isspace(*r->cursor));
+  assert(isspace(*r->cursor) || *r->cursor == until);
   if (str) {
     str->start = start;
     str->end = r->cursor;
@@ -782,19 +813,71 @@ static int http_request_parse_path(struct http_request *r)
   // Parse path: word immediately following verb, delimited by spaces.
   assert(r->path == NULL);
   struct substring path;
-  if (!(_skip_word_printable(r, &path) && _skip_literal(r, " "))) {
+  struct {
+    struct substring name;
+    struct substring value;
+  } params[NELS(r->query_parameters)];
+  unsigned count = 0;
+  if (_skip_word_printable(r, &path, '?')) {
+    struct substring param;
+    while (   count < NELS(params)
+	   && (_skip_literal(r, "?") || _skip_literal(r, "&"))
+	   && _skip_word_printable(r, &param, '&')
+    ) {
+      const char *eq = strnchr(param.start, param.end - param.start, '=');
+      params[count].name.start = param.start;
+      if (eq) {
+	params[count].name.end = eq;
+	params[count].value.start = eq + 1;
+	params[count].value.end = param.end;
+      } else {
+	params[count].name.end = param.end;
+	params[count].value.start = NULL;
+	params[count].value.end = NULL;
+      }
+      IDEBUGF(r->debug, "Query parameter: %s%s%s",
+	  alloca_substring_toprint(params[count].name),
+	  params[count].value.start ? "=" : "",
+	  params[count].value.start ? alloca_substring_toprint(params[count].value) : ""
+	);
+      ++count;
+    }
+  }
+  if (!_skip_literal(r, " ")) {
     if (_run_out(r))
       return 100; // read more and try again
-    IDEBUGF(r->debug, "Malformed HTTP %s request at path: %s", r->verb, alloca_toprint(20, r->parsed, r->end - r->parsed));
+    if (count == NELS(params))
+      IDEBUGF(r->debug, "Unsupported HTTP %s request, too many query parameters: %s", r->verb, alloca_toprint(20, r->parsed, r->end - r->parsed));
+    else
+      IDEBUGF(r->debug, "Malformed HTTP %s request at path: %s", r->verb, alloca_toprint(20, r->parsed, r->end - r->parsed));
     return 400;
   }
   _commit(r);
-  if (!_reserve(r, &r->path, path))
+  if (!_reserve_www_form_uriencoded(r, &r->path, path))
     return 0; // error
+  unsigned i;
+  for (i = 0; i != count; ++i) {
+    if (!_reserve_www_form_uriencoded(r, &r->query_parameters[i].name, params[i].name))
+      return 0; // error
+    if (params[i].value.start && !_reserve_www_form_uriencoded(r, &r->query_parameters[i].value, params[i].value))
+      return 0; // error
+  }
   r->parser = http_request_parse_http_version;
   return 0;
 }
 
+const char HTTP_REQUEST_PARAM_NOVALUE[] = "";
+
+const char *http_request_get_query_param(struct http_request *r, const char *name)
+{
+  unsigned i;
+  for (i = 0; i != NELS(r->query_parameters) && r->query_parameters[i].name; ++i) {
+    if (strcmp(r->query_parameters[i].name, name) == 0)
+      return r->query_parameters[i].value ? r->query_parameters[i].value : HTTP_REQUEST_PARAM_NOVALUE;
+  }
+  return NULL;
+}
+
 /* If parsing completes, then sets r->parser to the next parsing function and returns 0.  If parsing
  * cannot complete due to running out of data, returns 100 without changing r->parser, so this
  * function will be called again once more data has been read.  Returns a 4nn or 5nn HTTP result
@@ -986,12 +1069,12 @@ static int http_request_parse_header(struct http_request *r)
     }
     _skip_optional_space(r);
     struct substring origin;
-    if (_skip_word_printable(r, &origin) 
+    if (_skip_word_printable(r, &origin, ' ') 
 	&& _skip_optional_space(r)
 	&& r->cursor == eol) {
       r->cursor = nextline;
       _commit(r);
-      _reserve(r, &r->request_header.origin, origin);
+      _reserve_substring(r, &r->request_header.origin, origin);
       return 0;
     }
     goto malformed;
diff --git a/http_server.h b/http_server.h
index be0f22cb..618e1036 100644
--- a/http_server.h
+++ b/http_server.h
@@ -196,6 +196,11 @@ struct http_request {
   // The parsed HTTP request is accumulated into the following fields.
   const char *verb; // points to nul terminated static string, "GET", "PUT", etc.
   const char *path; // points into buffer; nul terminated
+  struct query_parameter {
+    const char *name; // points into buffer; nul terminated
+    const char *value; // points into buffer; nul terminated
+  }
+    query_parameters[10]; // can make this as big as needed, but not dynamic
   uint8_t version_major; // m from from HTTP/m.n
   uint8_t version_minor; // n from HTTP/m.n
   struct http_request_headers request_header;
@@ -237,4 +242,11 @@ struct http_request {
   char buffer[8 * 1024];
 };
 
+/* Return the nul-terminated string value of a given query parameter: NULL if
+ * no such parameter was supplied; HTTP_REQUEST_PARAM_NOVALUE if the parameter
+ * was supplied without an '=value' part.
+ */
+const char *http_request_get_query_param(struct http_request *r, const char *name);
+extern const char HTTP_REQUEST_PARAM_NOVALUE[];
+
 #endif // __SERVAL_DNA__HTTP_SERVER_H

From 73ced0f93a870019343a0796a2ec7eb330ea59ff Mon Sep 17 00:00:00 2001
From: Andrew Bettison <andrew@servalproject.org>
Date: Mon, 24 Aug 2015 17:42:26 +0930
Subject: [PATCH 05/11] pin= query param for HTTP GET /restful/keyring/add

---
 keyring_restful.c    |  3 ++-
 tests/keyringrestful | 23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/keyring_restful.c b/keyring_restful.c
index fed212ef..c4ae1047 100644
--- a/keyring_restful.c
+++ b/keyring_restful.c
@@ -159,7 +159,8 @@ static int restful_keyring_add(httpd_request *r, const char *remainder)
 {
   if (*remainder)
     return 404;
-  const keyring_identity *id = keyring_create_identity(keyring, "");
+  const char *pin = http_request_get_query_param(&r->http, "pin");
+  const keyring_identity *id = keyring_create_identity(keyring, pin ? pin : "");
   if (id == NULL)
     return http_request_keyring_response(r, 501, "Could not create identity");
   const sid_t *sidp = NULL;
diff --git a/tests/keyringrestful b/tests/keyringrestful
index 944b1c9e..81160732 100755
--- a/tests/keyringrestful
+++ b/tests/keyringrestful
@@ -112,4 +112,27 @@ test_keyringAdd() {
    assertStdoutGrep --stderr --matches=1 "^$SID::\$"
 }
 
+doc_keyringAddPin="HTTP RESTful add keyring identity with non-empty PIN"
+setup_keyringAddPin() {
+   IDENTITY_COUNT=2
+   setup
+}
+test_keyringAddPin() {
+   executeOk curl \
+         --silent --fail --show-error \
+         --output add.json \
+         --dump-header http.headers \
+         --basic --user harry:potter \
+         "http://$addr_localhost:$PORTA/restful/keyring/add?pin=1234"
+   tfw_cat http.headers add.json
+   tfw_preserve add.json
+   SID="$(jq -r '.sid' add.json)"
+   executeOk_servald keyring list
+   assert_keyring_list 2
+   assertStdoutGrep --stderr --matches=0 "^$SID::\$"
+   executeOk_servald keyring list --entry-pin=1234
+   assert_keyring_list 3
+   assertStdoutGrep --stderr --matches=1 "^$SID::\$"
+}
+
 runTests "$@"

From 166a03c7c602a2c38c6be5ad9636faca15d30030 Mon Sep 17 00:00:00 2001
From: Andrew Bettison <andrew@servalproject.org>
Date: Mon, 24 Aug 2015 22:32:22 +0930
Subject: [PATCH 06/11] Remove useless safety checks

---
 keyring.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/keyring.c b/keyring.c
index 924c45ba..5f733114 100644
--- a/keyring.c
+++ b/keyring.c
@@ -1198,7 +1198,6 @@ int keyring_enter_pin(keyring_file *k, const char *pin)
 {
   IN();
   DEBUGF(keyring, "k=%p, pin=%s", k, alloca_str_toprint(pin));
-  if (!k) RETURN(-1);
   if (!pin) pin="";
 
   // Check if PIN is already entered.
@@ -1300,7 +1299,6 @@ keyring_identity *keyring_create_identity(keyring_file *k, const char *pin)
 {
   DEBUGF(keyring, "k=%p", k);
   /* Check obvious abort conditions early */
-  if (!k) { WHY("keyring is NULL"); return NULL; }
   if (!k->bam) { WHY("keyring lacks BAM (not to be confused with KAPOW)"); return NULL; }
 
   if (!pin) pin="";
@@ -1348,8 +1346,6 @@ keyring_identity *keyring_create_identity(keyring_file *k, const char *pin)
 int keyring_commit(keyring_file *k)
 {
   DEBUGF(keyring, "k=%p", k);
-  if (!k)
-    return WHY("keyring was NULL");
   unsigned errorCount = 0;
   /* Write all BAMs */
   keyring_bam *b;
@@ -1408,10 +1404,6 @@ int keyring_commit(keyring_file *k)
 
 int keyring_set_did(keyring_identity *id, const char *did, const char *name)
 {
-  if (!id) return WHY("id is null");
-  if (!did) return WHY("did is null");
-  if (!name) name="Mr. Smith";
-
   /* Find where to put it */
   keypair *kp = id->keypairs;
   while(kp){

From b7ba297e317cd67359b80a6d7addeadbb3d2fb65 Mon Sep 17 00:00:00 2001
From: Andrew Bettison <andrew@servalproject.org>
Date: Mon, 24 Aug 2015 22:33:19 +0930
Subject: [PATCH 07/11] Add TODO comments to randomise keyring slot allocation

---
 keyring.c            | 1 +
 tests/keyringrestful | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/keyring.c b/keyring.c
index 5f733114..1d61a0dd 100644
--- a/keyring.c
+++ b/keyring.c
@@ -1265,6 +1265,7 @@ static void set_slot(keyring_file *k, unsigned slot, int bitvalue)
 
 /* Find free slot in keyring.  Slot 0 in any slab is the BAM and possible keyring salt, so only
  * search for space in slots 1 and above.  TODO: Extend to handle more than one slab!
+ * TODO: random search to avoid predictability of used slots!
  */
 static unsigned find_free_slot(const keyring_file *k)
 {
diff --git a/tests/keyringrestful b/tests/keyringrestful
index 81160732..189e721d 100755
--- a/tests/keyringrestful
+++ b/tests/keyringrestful
@@ -85,6 +85,10 @@ test_keyringList() {
    tfw_cat http.headers list.json
    tfw_preserve list.json
    assert [ "$(jq '.rows | length' list.json)" = $IDENTITY_COUNT ]
+   # TODO: these tests only work because the listed order of identities is the
+   # order of creation, which makes locked identities easy to attack.  When the
+   # random search TODO in keyring.c:find_free_slot() is done, then these tests
+   # should fail.
    assert [ "$(jq -r '.rows[0][0]' list.json)" = $SIDA1 ]
    assert [ "$(jq -r '.rows[4][0]' list.json)" = $SIDA5 ]
    assert [ "$(jq -r '.rows[9][0]' list.json)" = $SIDA10 ]

From 564f8973af9f2cb09bbeb4db72c0a6713ed7295b Mon Sep 17 00:00:00 2001
From: Andrew Bettison <andrew@servalproject.org>
Date: Mon, 24 Aug 2015 22:34:10 +0930
Subject: [PATCH 08/11] Improve test defs: create_identities() with PINs

---
 testdefs.sh | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/testdefs.sh b/testdefs.sh
index 942ae12a..28ca2703 100644
--- a/testdefs.sh
+++ b/testdefs.sh
@@ -702,6 +702,8 @@ create_single_identity() {
 #  - pass [args...] to the keyring add
 #  - if variables DID{I}{1..N} and/or NAME{I}{1..N} are already set, then use
 #    them to set the DIDs and names of each identity
+#  - if variables PIN{I}{1..N} are already set, then use them as the --entry-pin
+#    option to set the DIDs and names of each identity
 #  - assert that all SIDs are unique
 #  - assert that all SIDs appear in keyring list
 #  - set variables SID{I}{1..N} to SIDs of identities, eg, SIDA1, SIDA2...
@@ -723,11 +725,19 @@ create_identities() {
    shift
    local i j
    for ((i = 1; i <= N; ++i)); do
-      executeOk_servald keyring add "${servald_options[@]}"
-      assert [ -e "$SERVALINSTANCE_PATH/serval.keyring" ]
+      local pinvar=PIN$instance_name$i
+      local pin="${!pinvar}"
+      servald_options+=(${pin:+--entry-pin="$pin"})
+   done
+   for ((i = 1; i <= N; ++i)); do
+      local pinvar=PIN$instance_name$i
       local sidvar=SID$instance_name$i
       local didvar=DID$instance_name$i
       local namevar=NAME$instance_name$i
+      local pin="${!pinvar}"
+      [ -n "$pin" ] && tfw_log "$pinvar=$(shellarg "$pin")"
+      executeOk_servald keyring add "${servald_options[@]}" "$pin"
+      assert [ -e "$SERVALINSTANCE_PATH/serval.keyring" ]
       extract_stdout_keyvalue $sidvar sid "$rexp_sid"
       tfw_log "$sidvar=${!sidvar}"
       # If the DID and/or NAME is already specified in the variables, then use

From 380a72113c2763a86fff90b420af1627b0f16eb5 Mon Sep 17 00:00:00 2001
From: Andrew Bettison <andrew@servalproject.org>
Date: Mon, 24 Aug 2015 22:37:23 +0930
Subject: [PATCH 09/11] pin= query param for HTTP GET
 /restful/keyring/identities.json

---
 keyring_restful.c    |  3 +++
 tests/keyringrestful | 39 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/keyring_restful.c b/keyring_restful.c
index c4ae1047..81af3f36 100644
--- a/keyring_restful.c
+++ b/keyring_restful.c
@@ -76,6 +76,9 @@ static int restful_keyring_identitylist_json(httpd_request *r, const char *remai
 {
   if (*remainder)
     return 404;
+  const char *pin = http_request_get_query_param(&r->http, "pin");
+  if (pin)
+    keyring_enter_pin(keyring, pin);
   r->u.sidlist.phase = LIST_HEADER;
   keyring_iterator_start(keyring, &r->u.sidlist.it);
   http_request_response_generated(&r->http, 200, CONTENT_TYPE_JSON, restful_keyring_identitylist_json_content);
diff --git a/tests/keyringrestful b/tests/keyringrestful
index 189e721d..c20b0731 100755
--- a/tests/keyringrestful
+++ b/tests/keyringrestful
@@ -94,7 +94,44 @@ test_keyringList() {
    assert [ "$(jq -r '.rows[9][0]' list.json)" = $SIDA10 ]
 }
 
-doc_keyringAdd="HTTP RESTful add keyring identity with empty PIN"
+doc_keyringListPin="HTTP RESTful list keyring identities as JSON, with PIN"
+setup_keyringListPin() {
+   IDENTITY_COUNT=3
+   PINA1='wif waf'
+   setup
+}
+test_keyringListPin() {
+   # First, list without supplying the PIN
+   executeOk curl \
+         --silent --fail --show-error \
+         --output list1.json \
+         --dump-header http.headers \
+         --basic --user harry:potter \
+         "http://$addr_localhost:$PORTA/restful/keyring/identities.json"
+   tfw_cat http.headers list1.json
+   tfw_preserve list1.json
+   transform_list_json list1.json ids1.json
+   assert [ "$(jq 'length' ids1.json)" = $((IDENTITY_COUNT-1)) ]
+   assertJq ids1.json 'contains([{"sid": "'$SIDA1'"}]) | not'
+   assertJq ids1.json 'contains([{"sid": "'$SIDA2'"}])'
+   assertJq ids1.json 'contains([{"sid": "'$SIDA3'"}])'
+   # Then, list supplying the PIN
+   executeOk curl \
+         --silent --fail --show-error \
+         --output list2.json \
+         --dump-header http.headers \
+         --basic --user harry:potter \
+         "http://$addr_localhost:$PORTA/restful/keyring/identities.json?pin=wif+waf"
+   tfw_cat http.headers list2.json
+   tfw_preserve list2.json
+   transform_list_json list2.json ids2.json
+   assert [ "$(jq 'length' ids2.json)" = $IDENTITY_COUNT ]
+   assertJq ids2.json 'contains([{"sid": "'$SIDA1'"}])'
+   assertJq ids2.json 'contains([{"sid": "'$SIDA2'"}])'
+   assertJq ids2.json 'contains([{"sid": "'$SIDA3'"}])'
+}
+
+doc_keyringAdd="HTTP RESTful add keyring identity"
 setup_keyringAdd() {
    IDENTITY_COUNT=2
    setup

From 8834a815757655bf05e3c6bb6b3a70b682bd5e92 Mon Sep 17 00:00:00 2001
From: Andrew Bettison <andrew@servalproject.org>
Date: Mon, 24 Aug 2015 22:39:58 +0930
Subject: [PATCH 10/11] Refactor HTTP GET /restful/keyring/add

---
 keyring_restful.c    | 43 +++++++++++++++++++++++++++++++------------
 tests/keyringrestful | 17 +++++++++++++++--
 2 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/keyring_restful.c b/keyring_restful.c
index 81af3f36..1b998e18 100644
--- a/keyring_restful.c
+++ b/keyring_restful.c
@@ -70,6 +70,36 @@ static int http_request_keyring_response(struct httpd_request *r, uint16_t resul
   return result;
 }
 
+static int http_request_keyring_response_identity(struct httpd_request *r, uint16_t result, const char *message, const keyring_identity *id)
+{
+  const sid_t *sidp = NULL;
+  const char *did = NULL;
+  const char *name = NULL;
+  keyring_identity_extract(id, &sidp, &did, &name);
+  if (!sidp)
+    return http_request_keyring_response(r, 501, "Identity has no SID");
+  unsigned i = 0;
+  if (sidp) {
+    r->http.response.result_extra[i].label = "sid";
+    r->http.response.result_extra[i].value.type = JSON_STRING_NULTERM;
+    r->http.response.result_extra[i].value.u.string.content = alloca_tohex_sid_t(*sidp);
+    ++i;
+  }
+  if (did) {
+    r->http.response.result_extra[i].label = "did";
+    r->http.response.result_extra[i].value.type = JSON_STRING_NULTERM;
+    r->http.response.result_extra[i].value.u.string.content = did;
+    ++i;
+  }
+  if (name) {
+    r->http.response.result_extra[i].label = "name";
+    r->http.response.result_extra[i].value.type = JSON_STRING_NULTERM;
+    r->http.response.result_extra[i].value.u.string.content = name;
+    ++i;
+  }
+  return http_request_keyring_response(r, result, message);
+}
+
 static HTTP_CONTENT_GENERATOR restful_keyring_identitylist_json_content;
 
 static int restful_keyring_identitylist_json(httpd_request *r, const char *remainder)
@@ -166,18 +196,7 @@ static int restful_keyring_add(httpd_request *r, const char *remainder)
   const keyring_identity *id = keyring_create_identity(keyring, pin ? pin : "");
   if (id == NULL)
     return http_request_keyring_response(r, 501, "Could not create identity");
-  const sid_t *sidp = NULL;
-  const char *did = "";
-  const char *name = "";
-  keyring_identity_extract(id, &sidp, &did, &name);
-  if (!sidp)
-    return http_request_keyring_response(r, 501, "New identity has no SID");
   if (keyring_commit(keyring) == -1)
     return http_request_keyring_response(r, 501, "Could not store new identity");
-  strbuf s = strbuf_alloca(200);
-  strbuf_puts(s, "{\n \"sid\":");
-  strbuf_json_hex(s, sidp->binary, sizeof sidp->binary);
-  strbuf_puts(s, "\n}");
-  http_request_response_static(&r->http, 200, CONTENT_TYPE_JSON, strbuf_str(s), strbuf_len(s));
-  return 1;
+  return http_request_keyring_response_identity(r, 200, CONTENT_TYPE_JSON, id);
 }
diff --git a/tests/keyringrestful b/tests/keyringrestful
index c20b0731..f8c24b41 100755
--- a/tests/keyringrestful
+++ b/tests/keyringrestful
@@ -153,20 +153,21 @@ test_keyringAdd() {
    assertStdoutGrep --stderr --matches=1 "^$SID::\$"
 }
 
-doc_keyringAddPin="HTTP RESTful add keyring identity with non-empty PIN"
+doc_keyringAddPin="HTTP RESTful add keyring identity with PIN"
 setup_keyringAddPin() {
    IDENTITY_COUNT=2
    setup
 }
 test_keyringAddPin() {
    executeOk curl \
-         --silent --fail --show-error \
+         --silent --show-error --write-out '%{http_code}' \
          --output add.json \
          --dump-header http.headers \
          --basic --user harry:potter \
          "http://$addr_localhost:$PORTA/restful/keyring/add?pin=1234"
    tfw_cat http.headers add.json
    tfw_preserve add.json
+   assertStdoutIs '200'
    SID="$(jq -r '.sid' add.json)"
    executeOk_servald keyring list
    assert_keyring_list 2
@@ -174,6 +175,18 @@ test_keyringAddPin() {
    executeOk_servald keyring list --entry-pin=1234
    assert_keyring_list 3
    assertStdoutGrep --stderr --matches=1 "^$SID::\$"
+   # Now the server has internalised the PIN, so the new identity appears in the
+   # list
+   executeOk curl \
+         --silent --fail --show-error \
+         --output list.json \
+         --dump-header http.headers \
+         --basic --user harry:potter \
+         "http://$addr_localhost:$PORTA/restful/keyring/identities.json"
+   tfw_cat http.headers list.json
+   tfw_preserve list.json
+   transform_list_json list.json ids.json
+   assertJq ids.json 'contains([{"sid": "'$SIDA1'"}])'
 }
 
 runTests "$@"

From 8ba612ffa1eefe0ba60cc1b3633ef7f5a603191b Mon Sep 17 00:00:00 2001
From: Andrew Bettison <andrew@servalproject.org>
Date: Mon, 24 Aug 2015 22:40:16 +0930
Subject: [PATCH 11/11] HTTP GET
 /restful/keyring/set[?pin=PIN][&did=DID][&name=Name]

---
 keyring_restful.c    | 29 +++++++++++++++
 tests/keyringrestful | 85 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 114 insertions(+)

diff --git a/keyring_restful.c b/keyring_restful.c
index 1b998e18..ba6976f3 100644
--- a/keyring_restful.c
+++ b/keyring_restful.c
@@ -29,6 +29,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 static HTTP_HANDLER restful_keyring_identitylist_json;
 static HTTP_HANDLER restful_keyring_add;
+static HTTP_HANDLER restful_keyring_set;
 
 int restful_keyring_(httpd_request *r, const char *remainder)
 {
@@ -41,6 +42,7 @@ int restful_keyring_(httpd_request *r, const char *remainder)
   const char *verb = HTTP_VERB_GET;
   http_size_t content_length = CONTENT_LENGTH_UNKNOWN;
   HTTP_HANDLER *handler = NULL;
+  const char *end;
   if (strcmp(remainder, "identities.json") == 0) {
     handler = restful_keyring_identitylist_json;
     verb = HTTP_VERB_GET;
@@ -51,6 +53,13 @@ int restful_keyring_(httpd_request *r, const char *remainder)
     verb = HTTP_VERB_GET;
     remainder = "";
   }
+  else if (parse_sid_t(&r->sid1, remainder, -1, &end) != -1) {
+    remainder = end;
+    if (strcmp(remainder, "/set") == 0) {
+      handler = restful_keyring_set;
+      remainder = "";
+    }
+  }
   if (handler == NULL)
     return 404;
   if (	 content_length != CONTENT_LENGTH_UNKNOWN
@@ -200,3 +209,23 @@ static int restful_keyring_add(httpd_request *r, const char *remainder)
     return http_request_keyring_response(r, 501, "Could not store new identity");
   return http_request_keyring_response_identity(r, 200, CONTENT_TYPE_JSON, id);
 }
+
+static int restful_keyring_set(httpd_request *r, const char *remainder)
+{
+  if (*remainder)
+    return 404;
+  const char *pin = http_request_get_query_param(&r->http, "pin");
+  const char *did = http_request_get_query_param(&r->http, "did");
+  const char *name = http_request_get_query_param(&r->http, "name");
+  if (pin)
+    keyring_enter_pin(keyring, pin);
+  keyring_iterator it;
+  keyring_iterator_start(keyring, &it);
+  if (!keyring_find_sid(&it, &r->sid1))
+    return http_request_keyring_response(r, 404, NULL);
+  if (keyring_set_did(it.identity, did ? did : "", name ? name : "") == -1)
+    return http_request_keyring_response(r, 501, "Could not set identity DID/Name");
+  if (keyring_commit(keyring) == -1)
+    return http_request_keyring_response(r, 501, "Could not store new identity");
+  return http_request_keyring_response_identity(r, 200, CONTENT_TYPE_JSON, it.identity);
+}
diff --git a/tests/keyringrestful b/tests/keyringrestful
index f8c24b41..e30d346b 100755
--- a/tests/keyringrestful
+++ b/tests/keyringrestful
@@ -189,4 +189,89 @@ test_keyringAddPin() {
    assertJq ids.json 'contains([{"sid": "'$SIDA1'"}])'
 }
 
+doc_keyringSetDidName="HTTP RESTful set DID and name"
+setup_keyringSetDidName() {
+   IDENTITY_COUNT=2
+   setup
+}
+test_keyringSetDidName() {
+   executeOk curl \
+         --silent --show-error --write-out '%{http_code}' \
+         --output set.json \
+         --dump-header http.headers \
+         --basic --user harry:potter \
+         "http://$addr_localhost:$PORTA/restful/keyring/$SIDA1/set?did=987654321&name=Joe%20Bloggs"
+   tfw_cat http.headers set.json
+   tfw_preserve set.json
+   assertStdoutIs '200'
+   assertJq set.json 'contains({"sid": "'$SIDA1'"})'
+   assertJq set.json 'contains({"did": "987654321"})'
+   assertJq set.json 'contains({"name": "Joe Bloggs"})'
+   executeOk_servald keyring list
+   assert_keyring_list 2
+   assertStdoutGrep --stderr --matches=1 "^$SIDA1:987654321:Joe Bloggs\$"
+}
+
+doc_keyringSetDidNamePin="HTTP RESTful set DID and name with PIN"
+setup_keyringSetDidNamePin() {
+   IDENTITY_COUNT=2
+   PINA1=xyzabc
+   setup
+}
+test_keyringSetDidNamePin() {
+   # First try with no PIN, and make sure it fails
+   executeOk curl \
+         --silent --show-error --write-out '%{http_code}' \
+         --output set1.json \
+         --dump-header http.headers \
+         --basic --user harry:potter \
+         "http://$addr_localhost:$PORTA/restful/keyring/$SIDA1/set?did=111222333&name=Nobody"
+   tfw_cat http.headers set1.json
+   tfw_preserve set1.json
+   assertStdoutIs '404'
+   # Enter incorrect PIN, and make sure it fails
+   executeOk curl \
+         --silent --show-error --write-out '%{http_code}' \
+         --output set2.json \
+         --dump-header http.headers \
+         --basic --user harry:potter \
+         "http://$addr_localhost:$PORTA/restful/keyring/$SIDA1/set?did=444555666&name=Anybody"
+   tfw_cat http.headers set2.json
+   tfw_preserve set2.json
+   assertStdoutIs '404'
+   # Then try with correct PIN, and make sure it succeeds
+   executeOk curl \
+         --silent --show-error --write-out '%{http_code}' \
+         --output set3.json \
+         --dump-header http.headers \
+         --basic --user harry:potter \
+         "http://$addr_localhost:$PORTA/restful/keyring/$SIDA1/set?did=987654321&name=Joe%20Bloggs&pin=xyzabc"
+   tfw_cat http.headers set3.json
+   tfw_preserve set3.json
+   assertStdoutIs '200'
+   assertJq set3.json 'contains({"sid": "'$SIDA1'"})'
+   assertJq set3.json 'contains({"did": "987654321"})'
+   assertJq set3.json 'contains({"name": "Joe Bloggs"})'
+   executeOk_servald keyring list --entry-pin=xyzabc
+   assert_keyring_list 2
+   assertStdoutGrep --stderr --matches=1 "^$SIDA1:987654321:Joe Bloggs\$"
+   # Finally, try again with no PIN, and make sure it succeeds (server has
+   # internalised the PIN supplied in the last request)
+   executeOk curl \
+         --silent --show-error --write-out '%{http_code}' \
+         --output set4.json \
+         --dump-header http.headers \
+         --basic --user harry:potter \
+         "http://$addr_localhost:$PORTA/restful/keyring/$SIDA1/set?did=321321321&name=Fred+Nurks"
+   tfw_cat http.headers set4.json
+   tfw_preserve set4.json
+   assertStdoutIs '200'
+   assertJq set4.json 'contains({"sid": "'$SIDA1'"})'
+   assertJq set4.json 'contains({"did": "321321321"})'
+   assertJq set4.json 'contains({"name": "Fred Nurks"})'
+   executeOk_servald keyring list --entry-pin=xyzabc
+   assert_keyring_list 2
+   assertStdoutGrep --stderr --matches=1 "^$SIDA1:321321321:Fred Nurks\$"
+}
+
 runTests "$@"