From f2b0c40848638ef432b8d80aca4c15eef07b2bb4 Mon Sep 17 00:00:00 2001
From: philippe44 <philippe44@users.noreply.github.com>
Date: Sat, 24 Sep 2022 23:11:20 -0700
Subject: [PATCH] fix potential cspot on socket timeout & optimize HTTP server
 - release

---
 .../spotify/cspot/bell/src/HTTPServer.cpp     | 68 ++++++++++---------
 .../spotify/cspot/src/PlainConnection.cpp     |  2 +-
 2 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/components/spotify/cspot/bell/src/HTTPServer.cpp b/components/spotify/cspot/bell/src/HTTPServer.cpp
index 857323c8..6a5389a4 100644
--- a/components/spotify/cspot/bell/src/HTTPServer.cpp
+++ b/components/spotify/cspot/bell/src/HTTPServer.cpp
@@ -80,9 +80,8 @@ void bell::HTTPServer::listen() {
         socket(server->ai_family, server->ai_socktype, server->ai_protocol);
     struct sockaddr_in clientname;
     socklen_t incomingSockSize;
-    int i;
     int yes = true;
-    if (setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(int)) < 0) {
+    if (setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, (const char*) &yes, sizeof(int)) < 0) {
         throw std::runtime_error("setsockopt failed: " +
                                  std::string(strerror(errno)));
     }
@@ -100,62 +99,65 @@ void bell::HTTPServer::listen() {
     FD_ZERO(&activeFdSet);
     FD_SET(sockfd, &activeFdSet);
 
-    for (;;) {
+    for (int maxfd = sockfd;;) {
         /* Block until input arrives on one or more active sockets. */
         readFdSet = activeFdSet;
         struct timeval tv = {0, 100000};
-
-        if (select(FD_SETSIZE, &readFdSet, NULL, NULL, &tv) < 0) {
+ 
+        if (select(maxfd + 1, &readFdSet, NULL, NULL, &tv) < 0) {
             BELL_LOG(error, "http", "Error in select");
             perror("select");
             // exit(EXIT_FAILURE);
         }
 
-        /* Service all the sockets with input pending. */
-        for (i = 0; i < FD_SETSIZE; ++i)
-            if (FD_ISSET(i, &readFdSet)) {
-                if (i == sockfd) {
-                    /* Connection request on original socket. */
-                    int newFd;
-                    incomingSockSize = sizeof(clientname);
-                    newFd = accept(sockfd, (struct sockaddr *)&clientname,
-                                   &incomingSockSize);
-                    if (newFd < 0) {
-                        perror("accept");
-                        exit(EXIT_FAILURE);
-                    }
+        /* Service listening socket. */
+        if (FD_ISSET(sockfd, &readFdSet)) {
+            int newFd;
+            incomingSockSize = sizeof(clientname);
+            newFd = accept(sockfd, (struct sockaddr*)&clientname,
+                    &incomingSockSize);
+            if (newFd < 0) {
+                perror("accept");
+                exit(EXIT_FAILURE);
+             }
 
-                    FD_SET(newFd, &activeFdSet);
+             FD_SET(newFd, &activeFdSet);
 
-                    HTTPConnection conn = {.buffer = std::vector<uint8_t>(128),
-                                           .httpMethod = ""};
+             HTTPConnection conn = { .buffer = std::vector<uint8_t>(128),
+                                     .httpMethod = "" };
 
-                    this->connections.insert({newFd, conn});
-                } else {
-                    /* Data arriving on an already-connected socket. */
-                    readFromClient(i);
-                }
-            }
+             this->connections.insert({ newFd, conn });
+         }
 
+        /* Service other sockets and update set & max */
+        maxfd = sockfd;
         for (auto it = this->connections.cbegin();
-             it != this->connections.cend() /* not hoisted */;
-             /* no increment */) {
+            it != this->connections.cend() /* not hoisted */;
+            /* no increment */) {
+            int fd = (*it).first;
             if ((*it).second.toBeClosed) {
-                close((*it).first);
-                FD_CLR((*it).first, &activeFdSet);
+                close(fd);
+                FD_CLR(fd, &activeFdSet);
                 this->connections.erase(
                     it++); // or "it = m.erase(it)" since C++11
-            } else {
+            }
+            else {
+                if (fd != sockfd && FD_ISSET(fd, &readFdSet)) {
+                    /* Data arriving on an already-connected socket. */
+                    readFromClient(fd);
+                }
+                if (fd > maxfd) maxfd = fd;
                 ++it;
             }
         }
+
     }
 }
 
 void bell::HTTPServer::readFromClient(int clientFd) {
     HTTPConnection &conn = this->connections[clientFd];
 
-    int nbytes = recv(clientFd, &conn.buffer[0], conn.buffer.size(), 0);
+    int nbytes = recv(clientFd, (char*) &conn.buffer[0], conn.buffer.size(), 0);
     if (nbytes < 0) {
         BELL_LOG(error, "http", "Error reading from client");
         perror("recv");
diff --git a/components/spotify/cspot/src/PlainConnection.cpp b/components/spotify/cspot/src/PlainConnection.cpp
index d1f1200e..e1e86d77 100644
--- a/components/spotify/cspot/src/PlainConnection.cpp
+++ b/components/spotify/cspot/src/PlainConnection.cpp
@@ -126,7 +126,7 @@ std::vector<uint8_t> PlainConnection::readBlock(size_t size)
                 break;
             default:
                 if (retries++ > 4) throw std::runtime_error("Error in read");
-
+                goto READ;
             }
         }
         idx += n;