added version to protocol buffer message

changed client handling
only non public ips can connect (can change that in options)

ext/libclementine-remote/remotecontrolmessages.proto

@@ -34,13 +34,14 @@ enum EngineState {
 }
 
 message Message {
-  required MsgType msgType = 1;
+  required int32 version = 1;
+  required MsgType msgType = 2;
   
-  optional EngineState state = 2;
-  optional ClementineInfos infos = 3;
-  optional SongMetadata currentSong = 4;
-  optional int32 volume = 5;
-  repeated Playlist playlists = 6;
+  optional EngineState state = 3;
+  optional ClementineInfos infos = 4;
+  optional SongMetadata currentSong = 5;
+  optional int32 volume = 6;
+  repeated Playlist playlists = 7;
 }
 
 message ClementineInfos {

src/CMakeLists.txt

@@ -218,6 +218,7 @@ set(SOURCES
   networkremote/networkremote.cpp
   networkremote/incomingdataparser.cpp
   networkremote/outgoingdatacreator.cpp
+  networkremote/remoteclient.cpp
 
   playlist/dynamicplaylistcontrols.cpp
   playlist/playlist.cpp
@@ -496,6 +497,7 @@ set(HEADERS
   networkremote/networkremote.h
   networkremote/incomingdataparser.h
   networkremote/outgoingdatacreator.h
+  networkremote/remoteclient.h
 
   playlist/dynamicplaylistcontrols.h
   playlist/playlist.h

src/networkremote/incomingdataparser.h

@@ -11,9 +11,11 @@ public:
   IncomingDataParser(Application* app);
   ~IncomingDataParser();
 
-  void Parse(const QByteArray& pb_data);
   bool close_connection();
 
+public slots:
+  void Parse(const QByteArray& pb_data);
+
 signals:
   void SendClementineInfos();
   void SendFirstData();

src/networkremote/networkremote.cpp

@@ -26,6 +26,7 @@
 
 const char* NetworkRemote::kSettingsGroup = "NetworkRemote";
 const int NetworkRemote::kDefaultServerPort = 5500;
+const int NetworkRemote::kProtocolBufferVersion = 1;
 
 NetworkRemote::NetworkRemote(Application* app)
   : app_(app)
@@ -46,6 +47,14 @@ void NetworkRemote::ReadSettings() {
   s.beginGroup(NetworkRemote::kSettingsGroup);
   use_remote_ = s.value("use_remote").toBool();
   port_       = s.value("port").toInt();
+
+  // Use only non public ips must be true be default
+  if (s.contains("only_non_public_ip")) {
+    only_non_public_ip_ = s.value("only_non_public_ip").toBool();
+  } else {
+    only_non_public_ip_ = true;
+  }
+
   if (port_ == 0) {
     port_ = kDefaultServerPort;
   }
@@ -54,6 +63,7 @@ void NetworkRemote::ReadSettings() {
 
 void NetworkRemote::SetupServer() {
   server_ = new QTcpServer();
+  server_ipv6_ = new QTcpServer();
   incoming_data_parser_  = new IncomingDataParser(app_);
   outgoing_data_creator_ = new OutgoingDataCreator(app_);
 
@@ -80,8 +90,10 @@ void NetworkRemote::StartServer() {
   qLog(Info) << "Starting network remote";
 
   connect(server_, SIGNAL(newConnection()), this, SLOT(AcceptConnection()));
+  connect(server_ipv6_, SIGNAL(newConnection()), this, SLOT(AcceptConnection()));
 
   server_->listen(QHostAddress::Any, port_);
+  server_ipv6_->listen(QHostAddress::AnyIPv6, port_);
 
   qLog(Info) << "Listening on port " << port_;
 }
@@ -89,15 +101,8 @@ void NetworkRemote::StartServer() {
 void NetworkRemote::StopServer() {
   if (server_->isListening()) {
     server_->close();
+    server_ipv6_->close();
     clients_.clear();
-    expected_length_.clear();
-    reading_protobuf_.clear();
-
-    // Delete all QBuffers
-    foreach (QBuffer* b, buffer_) {
-      delete b;
-    }
-    buffer_.clear();
   }
 }
 
@@ -130,64 +135,50 @@ void NetworkRemote::AcceptConnection() {
     connect(app_->player()->engine(), SIGNAL(StateChanged(Engine::State)),
             outgoing_data_creator_, SLOT(StateChanged(Engine::State)));
   }
-  QTcpSocket* client = server_->nextPendingConnection();
 
-  clients_.push_back(client);
-  reading_protobuf_.append(false);
-  expected_length_.append(0);
-  QBuffer* buf = new QBuffer();
-  buf->open(QIODevice::ReadWrite);
-  buffer_.push_back(buf);
-
-  // Connect to the slot IncomingData when receiving data
-  connect(client, SIGNAL(readyRead()), this, SLOT(IncomingData()));
-}
-
-void NetworkRemote::IncomingData() {
-  QTcpSocket* client =  static_cast<QTcpSocket*>(QObject::sender());
-
-  int which = clients_.indexOf(client);
-  qDebug() << "Which:" << which;
-
-  while (client->bytesAvailable()) {
-    if (!reading_protobuf_[which]) {
-      // Read the length of the next message
-      QDataStream s(client);
-      s >> expected_length_[which];
-
-      reading_protobuf_[which] = true;
+  if (server_->hasPendingConnections()) {
+    QTcpSocket* client_socket = server_->nextPendingConnection();
+    // Check if our ip is in private scope
+    if (only_non_public_ip_
+     && !IpIsPrivate(client_socket->peerAddress().toIPv4Address())) {
+      qLog(Info) << "Got a connection from public ip" <<
+                  client_socket->peerAddress().toString();
+    } else {
+      CreateRemoteClient(client_socket);
+      // TODO: Check private ips for ipv6
     }
 
-    // Read some of the message
-    buffer_[which]->write(
-      client->read(expected_length_[which] - buffer_[which]->size()));
-
-    // Did we get everything?
-    if (buffer_[which]->size() == expected_length_[which]) {
-      // Parse the message
-      incoming_data_parser_->Parse(buffer_[which]->data());
-
-      // Clear the buffer
-      buffer_[which]->close();
-      buffer_[which]->setData(QByteArray());
-      buffer_[which]->open(QIODevice::ReadWrite);
-      reading_protobuf_[which] = false;
-    }
+  } else {
+    // No checks on ipv6
+    CreateRemoteClient(server_ipv6_->nextPendingConnection());
   }
 }
 
-void NetworkRemote::RemoveClient(int index) {
-  // Remove client
-  QTcpSocket* client = clients_[index];
-  delete client;
-  clients_.removeAt(index);
+bool NetworkRemote::IpIsPrivate(int ip) {
+  int private_local = QHostAddress("127.0.0.1").toIPv4Address();
+  int private_a = QHostAddress("10.0.0.0").toIPv4Address();
+  int private_b = QHostAddress("172.16.0.0").toIPv4Address();
+  int private_c = QHostAddress("192.168.0.0").toIPv4Address();
 
-  // Remove QBuffer
-  QBuffer* buf = buffer_[index];
-  delete buf;
-  buffer_.removeAt(index);
-
-  // Remove other QList entries
-  reading_protobuf_.removeAt(index);
-  expected_length_.removeAt(index);
+  // Check if we have a private ip address
+  if (ip == private_local
+   || (ip >= private_a && ip < private_a + 16777216)
+   || (ip >= private_b && ip < private_b + 1048576)
+   || (ip >= private_c && ip < private_c + 65536)) {
+    return true;
+  } else {
+    return false;
+  }
+}
+
+void NetworkRemote::CreateRemoteClient(QTcpSocket *client_socket) {
+  if (client_socket) {
+    // Add the client to the list
+    RemoteClient* client = new RemoteClient(app_, client_socket);
+    clients_.push_back(client);
+
+    // Connect the signal to parse data
+    connect(client, SIGNAL(Parse(QByteArray)),
+            incoming_data_parser_, SLOT(Parse(QByteArray)));
+  }
 }

src/networkremote/networkremote.h

@@ -1,7 +1,6 @@
 #ifndef NETWORKREMOTE_H
 #define NETWORKREMOTE_H
 
-#include <QtNetwork>
 #include <QTcpServer>
 #include <QTcpSocket>
 
@@ -9,43 +8,41 @@
 #include "core/application.h"
 #include "incomingdataparser.h"
 #include "outgoingdatacreator.h"
-
-class Application;
+#include "remoteclient.h"
 
 class NetworkRemote : public QThread {
     Q_OBJECT
 public:
   static const char* kSettingsGroup;
   static const int kDefaultServerPort;
+  static const int kProtocolBufferVersion;
 
   NetworkRemote(Application* app);
   ~NetworkRemote();
 
-  void RemoveClient(int index);
-
 public slots:
   void SetupServer();
   void StartServer();
   void ReloadSettings();
   void AcceptConnection();
-  void IncomingData();
 
 private:
   QTcpServer* server_;
+  QTcpServer* server_ipv6_;
   IncomingDataParser* incoming_data_parser_;
   OutgoingDataCreator* outgoing_data_creator_;
   int port_;
   bool use_remote_;
+  bool only_non_public_ip_;
   bool signals_connected_;
   Application* app_;
 
-  QList<QTcpSocket*> clients_;
-  QList<bool> reading_protobuf_;
-  QList<quint32> expected_length_;
-  QList<QBuffer*> buffer_;
+  QList<RemoteClient*> clients_;
 
   void StopServer();
   void ReadSettings();
+  void CreateRemoteClient(QTcpSocket* client_socket);
+  bool IpIsPrivate(int ip);
 };
 
 #endif // NETWORKREMOTE_H

src/networkremote/outgoingdatacreator.cpp

@@ -20,8 +20,7 @@
 #include "core/logging.h"
 
 OutgoingDataCreator::OutgoingDataCreator(Application* app)
-  : app_(app),
-    clients_(NULL)
+  : app_(app)
 {
   // Create Keep Alive Timer
   keep_alive_timer_ = new QTimer(this);
@@ -32,7 +31,7 @@ OutgoingDataCreator::OutgoingDataCreator(Application* app)
 OutgoingDataCreator::~OutgoingDataCreator() {
 }
 
-void OutgoingDataCreator::SetClients(QList<QTcpSocket*>* clients) {
+void OutgoingDataCreator::SetClients(QList<RemoteClient*>* clients) {
   clients_ = clients;
   // After we got some clients, start the keep alive timer
   // Default: every 10 seconds
@@ -41,24 +40,21 @@ void OutgoingDataCreator::SetClients(QList<QTcpSocket*>* clients) {
 
 void OutgoingDataCreator::SendDataToClients(pb::remote::Message* msg) {
   // Check if we have clients to send data to
-  if (!clients_ || clients_->empty()) {
+  if (clients_->empty()) {
     return;
   }
 
-  QTcpSocket* sock;
-  foreach(sock, *clients_) {
+  // Add the Version number
+  msg->set_version(NetworkRemote::kProtocolBufferVersion);
+
+  RemoteClient* client;
+  foreach(client, *clients_) {
     // Check if the client is still active
-    if (sock->state() == QTcpSocket::ConnectedState) {
-      std::string data = msg->SerializeAsString();
-
-      QDataStream s(sock);
-      s << qint32(data.length());
-      s.writeRawData(data.data(), data.length());
-
-      sock->flush();
-
+    if (client->State() == QTcpSocket::ConnectedState) {
+      client->SendData(msg);
     } else {
-      app_->network_remote()->RemoveClient(clients_->indexOf(sock));
+      clients_->removeAt(clients_->indexOf(client));
+      delete client;
     }
   }
 }
@@ -141,7 +137,7 @@ void OutgoingDataCreator::CurrentSongChanged(const Song& song, const QString& ur
   current_uri_   = uri;
   current_image_ = img;
 
-  if (clients_) {
+  if (!clients_->empty()) {
     // Create the message
     pb::remote::Message msg;
     msg.set_msgtype(pb::remote::CURRENT_METAINFOS);

src/networkremote/outgoingdatacreator.h

@@ -13,6 +13,7 @@
 #include "playlist/playlist.h"
 #include "playlist/playlistmanager.h"
 #include "remotecontrolmessages.pb.h"
+#include "remoteclient.h"
 
 class OutgoingDataCreator : public QObject {
     Q_OBJECT
@@ -20,7 +21,7 @@ public:
   OutgoingDataCreator(Application* app);
   ~OutgoingDataCreator();
 
-  void SetClients(QList<QTcpSocket*>* clients);
+  void SetClients(QList<RemoteClient*>* clients);
 
 public slots:
   void SendClementineInfos();
@@ -36,7 +37,7 @@ public slots:
 
 private:
   Application* app_;
-  QList<QTcpSocket*>* clients_;
+  QList<RemoteClient*>* clients_;
   Song current_song_;
   QString current_uri_;
   QImage current_image_;

src/networkremote/remoteclient.cpp

@@ -0,0 +1,83 @@
+/* This file is part of Clementine.
+   Copyright 2013, Andreas Muttscheller <asfa194@gmail.com>
+
+   Clementine is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   Clementine is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "core/logging.h"
+
+#include "remoteclient.h"
+
+#include <QDataStream>
+
+RemoteClient::RemoteClient(Application* app, QTcpSocket* client)
+  : app_(app),
+    client_(client)
+{
+  // Open the buffer
+  buffer_.setData(QByteArray());
+  buffer_.open(QIODevice::ReadWrite);
+  reading_protobuf_ = false;
+
+  // Connect to the slot IncomingData when receiving data
+  connect(client, SIGNAL(readyRead()), this, SLOT(IncomingData()));
+}
+
+
+RemoteClient::~RemoteClient() {
+}
+
+void RemoteClient::IncomingData() {
+  while (client_->bytesAvailable()) {
+    if (!reading_protobuf_) {
+      // Read the length of the next message
+      QDataStream s(client_);
+      s >> expected_length_;
+      reading_protobuf_ = true;
+    }
+
+    // Read some of the message
+    buffer_.write(
+      client_->read(expected_length_ - buffer_.size()));
+
+    // Did we get everything?
+    if (buffer_.size() == expected_length_) {
+      // Parse the message
+      emit Parse(buffer_.data());
+
+      // Clear the buffer
+      buffer_.close();
+      buffer_.setData(QByteArray());
+      buffer_.open(QIODevice::ReadWrite);
+      reading_protobuf_ = false;
+    }
+  }
+}
+
+void RemoteClient::SendData(pb::remote::Message *msg) {
+  // Serialize the message
+  std::string data = msg->SerializeAsString();
+
+  // write the length of the data first
+  QDataStream s(client_);
+  s << qint32(data.length());
+  s.writeRawData(data.data(), data.length());
+
+  // Flush data
+  client_->flush();
+}
+
+QAbstractSocket::SocketState RemoteClient::State() {
+  return client_->state();
+}

src/networkremote/remoteclient.h

@@ -0,0 +1,35 @@
+#ifndef REMOTECLIENT_H
+#define REMOTECLIENT_H
+
+#include <QAbstractSocket>
+#include <QTcpSocket>
+#include <QBuffer>
+
+#include "core/application.h"
+#include "incomingdataparser.h"
+
+class RemoteClient : public QObject {
+    Q_OBJECT
+public:
+  RemoteClient(Application* app, QTcpSocket* client);
+  ~RemoteClient();
+
+  void SendData(pb::remote::Message* msg);
+  QAbstractSocket::SocketState State();
+
+private slots:
+  void IncomingData();
+
+signals:
+  void Parse(const QByteArray& pb_data);
+
+private:
+  Application* app_;
+
+  QTcpSocket* client_;
+  bool reading_protobuf_;
+  quint32 expected_length_;
+  QBuffer buffer_;
+};
+
+#endif // REMOTECLIENT_H

src/ui/networkremotesettingspage.cpp

@@ -51,6 +51,13 @@ void NetworkRemoteSettingsPage::Load() {
   }
 
   ui_->use_remote->setChecked(s.value("use_remote").toBool());
+  if (s.contains("only_non_public_ip")) {
+    ui_->only_non_public_ip->setChecked(s.value("only_non_public_ip").toBool());
+  } else {
+    // Default yes
+    ui_->only_non_public_ip->setChecked(true);
+    s.setValue("only_non_public_ip", true);
+  }
 
   s.endGroup();
 }
@@ -61,6 +68,7 @@ void NetworkRemoteSettingsPage::Save() {
   s.beginGroup(NetworkRemote::kSettingsGroup);
   s.setValue("port", ui_->remote_port->value());
   s.setValue("use_remote", ui_->use_remote->isChecked());
+  s.setValue("only_non_public_ip", ui_->only_non_public_ip->isChecked());
 
   s.endGroup();
 

src/ui/networkremotesettingspage.ui

@@ -6,7 +6,7 @@
    <rect>
     <x>0</x>
     <y>0</y>
-    <width>400</width>
+    <width>475</width>
     <height>300</height>
    </rect>
   </property>
@@ -33,7 +33,7 @@
       <item>
        <layout class="QHBoxLayout" name="horizontalLayout_5">
         <item>
-         <widget class="QLabel" name="label_15">
+         <widget class="QLabel" name="label_remote_port">
           <property name="minimumSize">
            <size>
             <width>171</width>
@@ -63,6 +63,26 @@
         </item>
        </layout>
       </item>
+      <item>
+       <layout class="QHBoxLayout" name="horizontalLayout">
+        <item>
+         <widget class="QCheckBox" name="only_non_public_ip">
+          <property name="toolTip">
+           <string>Only accept connections from clients within the ip ranges:
+10.x.x.x
+172.16.0.0 - 172.31.255.255
+192.168.x.x</string>
+          </property>
+          <property name="text">
+           <string>Accept non public clients only</string>
+          </property>
+          <property name="checked">
+           <bool>true</bool>
+          </property>
+         </widget>
+        </item>
+       </layout>
+      </item>
      </layout>
     </widget>
    </item>