[spotify] Just fixups

src/inputs/librespot-c/src/connection.c

@@ -21,6 +21,9 @@
 
 #define MERCURY_REQ_SIZE_MAX 4096
 
+// Forgot how I arrived at this upper bound
+#define HASHCASH_ITERATIONS_MAX 100000
+
 static struct timeval sp_idle_tv = { SP_AP_DISCONNECT_SECS, 0 };
 
 static uint8_t sp_aes_iv[] = { 0x72, 0xe0, 0x67, 0xfb, 0xdd, 0xcb, 0xcf, 0x77, 0xeb, 0xe8, 0xbc, 0x64, 0x3f, 0x63, 0x0d, 0x93 };
@@ -604,6 +607,7 @@ resolve_server_info_set(struct sp_server *server, const char *key, json_object *
 {
   json_object *list;
   json_object *instance;
+  size_t address_len;
   const char *s;
   char *colon;
   bool is_same;
@@ -625,7 +629,8 @@ resolve_server_info_set(struct sp_server *server, const char *key, json_object *
         RETURN_ERROR(SP_ERR_NOCONNECTION, "Unexpected data in response from access point resolver");
 
       s = json_object_get_string(instance); // This string includes the port
-      is_same = server->address && (strncmp(s, server->address, strlen(server->address)) == 0);
+      address_len = strlen(server->address);
+      is_same = (address_len > 0) && (strncmp(s, server->address, address_len) == 0);
 
       if (is_same && has_failed)
         s = NULL; // This AP has failed on us recently, so avoid
@@ -636,10 +641,10 @@ resolve_server_info_set(struct sp_server *server, const char *key, json_object *
 
   if (!is_same)
     {
-      free(server->address);
       memset(server, 0, sizeof(struct sp_server));
-
+      ret = snprintf(server->address, sizeof(server->address), "%s", s);
-      server->address = strdup(s);
+      if (ret < 0 || ret >= sizeof(server->address))
+	RETURN_ERROR(SP_ERR_INVALID, "AP resolver returned an address that is too long");
 
       colon = strchr(server->address, ':');
       if (colon)
@@ -974,7 +979,7 @@ handle_login5_challenges(Spotify__Login5__V3__Challenges *challenges, uint8_t *l
 
       memcpy(crypto_challenge->prefix, this_challenge->hashcash->prefix.data, sizeof(crypto_challenge->prefix));
       crypto_challenge->wanted_zero_bits = this_challenge->hashcash->length;
-      crypto_challenge->max_iterations = 100000; //TODO define or make variable
+      crypto_challenge->max_iterations = HASHCASH_ITERATIONS_MAX;
 
     }
 
@@ -1210,11 +1215,11 @@ msg_tcp_handle(struct sp_message *msg, struct sp_session *session)
   // We have a tcp request waiting for a response
   if (request && request->proto == SP_PROTO_TCP && request->response_handler)
     {
-      sp_cb.logmsg("Handling response to %s\n", request->name);
+//      sp_cb.logmsg("Handling response to %s\n", request->name);
       return request->response_handler(msg, session);
     }
 
-  sp_cb.logmsg("Handling incoming tcp message\n");
+//  sp_cb.logmsg("Handling incoming tcp message\n");
   // Not waiting for anything, could be a ping
   return handle_tcp_generic(msg, session);
 }
@@ -1227,7 +1232,7 @@ msg_http_handle(struct sp_message *msg, struct sp_session *session)
   // We have a http request waiting for a response
   if (request && request->proto == SP_PROTO_HTTP && request->response_handler)
     {
-      sp_cb.logmsg("Handling response to %s\n", request->name);
+//      sp_cb.logmsg("Handling response to %s\n", request->name);
       return request->response_handler(msg, session);
     }
 
@@ -2036,7 +2041,7 @@ msg_make_media_get(struct sp_message *msg, struct sp_session *session)
 
   hreq->headers[0] = asprintf_or_die("Range: bytes=%zu-%zu", bytes_from, bytes_to);
 
-  sp_cb.logmsg("Asking for %s\n", hreq->headers[0]);
+//  sp_cb.logmsg("Asking for %s\n", hreq->headers[0]);
 
   return 0;
 }
@@ -2224,7 +2229,7 @@ msg_http_send(struct http_response *hres, struct http_request *hreq, struct http
 
   hreq->user_agent = sp_sysinfo.client_name;
 
-  sp_cb.logmsg("Making http request to %s\n", hreq->url);
+//  sp_cb.logmsg("Making http request to %s\n", hreq->url);
 
   ret = http_request(hres, hreq, hses);
   if (ret < 0)

src/inputs/librespot-c/src/crypto.c

@@ -57,7 +57,7 @@ static const uint8_t prime_bytes[] =
 };
 
 static void
-crypto_log(const char *fmt, ...)
+crypto_debug(const char *fmt, ...)
 {
   return;
 }
@@ -238,14 +238,14 @@ crypto_decrypt(uint8_t *encrypted, size_t encrypted_len, struct crypto_cipher *c
   size_t header_len = sizeof(cipher->last_header);
   size_t payload_len;
 
-  crypto_log("Decrypting %zu bytes with nonce %u\n", encrypted_len, cipher->nonce);
+  crypto_debug("Decrypting %zu bytes with nonce %u\n", encrypted_len, cipher->nonce);
 //  crypto_hexdump("Key\n", cipher->key, sizeof(cipher->key));
 //  crypto_hexdump("Encrypted\n", encrypted, encrypted_len);
 
   // In case we didn't even receive the basics, header and mac, then return.
   if (encrypted_len < header_len + sizeof(mac))
     {
-      crypto_log("Waiting for %zu header bytes, have %zu\n", header_len + sizeof(mac), encrypted_len);
+      crypto_debug("Waiting for %zu header bytes, have %zu\n", header_len + sizeof(mac), encrypted_len);
       return 0;
     }
 
@@ -265,7 +265,7 @@ crypto_decrypt(uint8_t *encrypted, size_t encrypted_len, struct crypto_cipher *c
 
       payload_len = payload_len_get(cipher->last_header);
 
-//      crypto_log("Payload len is %zu\n", payload_len);
+//      crypto_debug("Payload len is %zu\n", payload_len);
 //      crypto_hexdump("Decrypted header\n", encrypted, header_len);
     }
 
@@ -276,7 +276,7 @@ crypto_decrypt(uint8_t *encrypted, size_t encrypted_len, struct crypto_cipher *c
   // Not enough data for decrypting the entire packet
   if (payload_len > encrypted_len)
     {
-      crypto_log("Waiting for %zu payload bytes, have %zu\n", payload_len, encrypted_len);
+      crypto_debug("Waiting for %zu payload bytes, have %zu\n", payload_len, encrypted_len);
       return 0;
     }
 
@@ -289,7 +289,7 @@ crypto_decrypt(uint8_t *encrypted, size_t encrypted_len, struct crypto_cipher *c
 //  crypto_hexdump("mac our\n", mac, sizeof(mac));
   if (memcmp(mac, encrypted + payload_len, sizeof(mac)) != 0)
     {
-      crypto_log("MAC VALIDATION FAILED\n"); // TODO
+      crypto_debug("MAC validation failed\n");
       memset(cipher->last_header, 0, header_len);
       return -1;
     }

src/inputs/librespot-c/src/librespot-c-internal.h

@@ -238,7 +238,7 @@ struct sp_message
 
 struct sp_server
 {
-  char *address; // e.g. ap-gue1.spotify.com
+  char address[256]; // e.g. ap-gue1.spotify.com
   unsigned short port; // normally 433 or 4070
   time_t last_connect_ts;
   time_t last_resolved_ts;

src/inputs/librespot-c/src/librespot-c.c

@@ -23,8 +23,8 @@
 
 
 /*
-Illustration of the general flow, where receive and writing the result are async
+Illustration of the general tcp flow, where receive and writing the result are
-operations. For some commands, e.g. open and seek, the entire sequence is
+async operations. For some commands, e.g. open and seek, the entire sequence is
 encapsulated in a sync command, which doesn't return until final "done, error or
 timeout". The command play is async, so all "done/error/timeout" is returned via
 callbacks. Also, play will loop the flow, i.e. after writing a chunk of data it
@@ -47,12 +47,6 @@ events for proceeding are activated directly.
 "timeout": receive or write took too long to complete
 */
 
-// TODO
-// - update comments
-// - try different server if connection refused
-// - Valgrind
-// - Handle connection error -> ap_resolve
-
 #include <pthread.h>
 #include <assert.h>
 
@@ -471,7 +465,7 @@ sequence_continue(struct sp_session *session)
   struct sp_message msg = { 0 };
   int ret;
 
-  sp_cb.logmsg("Preparing request '%s'\n", session->request->name);
+//  sp_cb.logmsg("Preparing request '%s'\n", session->request->name);
 
   // Checks if the dependencies for making the request are met - e.g. do we have
   // a connection and a valid token. If not, tries to satisfy them.