commit ebc00b4fdc21aa0dfd9a49fd59cc9ce2f2dd8a2a
parent cbb266b821deaa897dfebe121f6c5d58644b623b
Author: cfillion <cfillion@users.noreply.github.com>
Date: Sun, 3 Jan 2016 14:57:26 -0500
rewrite .ReaPackRemote file reading and validation code
Diffstat:
9 files changed, 201 insertions(+), 36 deletions(-)
diff --git a/src/manager.cpp b/src/manager.cpp
@@ -130,7 +130,6 @@ void Manager::setRemoteEnabled(const bool enabled)
return;
m_enableOverrides[remote.name()] = enabled;
-
m_list->replaceRow(m_list->currentIndex(), makeRow(remote));
}
diff --git a/src/reapack.cpp b/src/reapack.cpp
@@ -25,9 +25,6 @@
#include <reaper_plugin_functions.h>
-#include <fstream>
-#include <regex>
-
using namespace std;
ReaPack::ReaPack()
@@ -109,43 +106,31 @@ void ReaPack::synchronize()
void ReaPack::importRemote()
{
- static const int PATH_SIZE = 4096;
- char path[PATH_SIZE] = {0};
+ static const char *title = "ReaPack: Import remote repository";
- const char *title = "ReaPack: Import remote repository";
- if(!GetUserFileNameForRead(path, title, "ReaPackRemote"))
+ string path(4096, 0);
+ if(!GetUserFileNameForRead(&path[0], title, "ReaPackRemote"))
return;
- ifstream file(make_autostring(path));
+ Remote remote;
- if(!file) {
+ switch(Remote::fromFile(path, &remote)) {
+ case Remote::ReadFailure:
ShowMessageBox(strerror(errno), title, 0);
return;
- }
-
- string name;
- getline(file, name);
-
- string url;
- getline(file, url);
-
- file.close();
-
- static const regex namePattern("^[^~#%&*{}\\\\:<>?/+|\"]+$");
-
- smatch nameMatch;
- regex_match(name, nameMatch, namePattern);
-
- if(nameMatch.empty() || url.empty()) {
+ case Remote::InvalidName:
+ case Remote::InvalidUrl:
ShowMessageBox("Invalid .ReaPackRemote file!", title, 0);
return;
- }
+ default:
+ break;
+ };
RemoteList *remotes = m_config->remotes();
- Remote remote = remotes->get(name);
+ const Remote existing = remotes->get(remote.name());
- if(!remote.isNull()) {
+ if(!existing.isNull()) {
const int button = ShowMessageBox(
"This remote is already configured.\r\n"
"Do you want to overwrite the current remote?"
@@ -155,9 +140,6 @@ void ReaPack::importRemote()
return;
}
- remote.setName(name);
- remote.setUrl(url);
-
remotes->add(remote);
m_config->write();
diff --git a/src/remote.cpp b/src/remote.cpp
@@ -17,21 +17,87 @@
#include "remote.hpp"
+#include <fstream>
+#include <regex>
+
+#include "encoding.hpp"
+#include "errors.hpp"
+
using namespace std;
+static bool ValidateName(const string &name)
+{
+ static const regex pattern("^[^~#%&*{}\\\\:<>?/+|\"]+$");
+
+ smatch match;
+ regex_match(name, match, pattern);
+
+ return !match.empty();
+}
+
+static bool ValidateUrl(const string &url)
+{
+ return !url.empty();
+}
+
+auto Remote::fromFile(const string &path, Remote *remote) -> ReadCode
+{
+ ifstream file(make_autostring(path));
+
+ if(!file)
+ return ReadFailure;
+
+ string name;
+ getline(file, name);
+
+ string url;
+ getline(file, url);
+
+ file.close();
+
+
+ if(!ValidateName(name))
+ return InvalidName;
+ else if(!ValidateUrl(url))
+ return InvalidUrl;
+
+ remote->setName(name);
+ remote->setUrl(url);
+
+ return Success;
+}
+
Remote::Remote()
: m_enabled(true), m_frozen(false)
{
}
Remote::Remote(const string &name, const string &url, const bool enabled)
- : m_name(name), m_url(url), m_enabled(enabled), m_frozen(false)
+ : m_enabled(enabled), m_frozen(false)
+{
+ setName(name);
+ setUrl(url);
+}
+
+void Remote::setName(const string &name)
{
+ if(!ValidateName(name))
+ throw reapack_error("invalid name");
+ else
+ m_name = name;
+}
+
+void Remote::setUrl(const string &url)
+{
+ if(!ValidateUrl(url))
+ throw reapack_error("invalid url");
+ else
+ m_url = url;
}
void RemoteList::add(const Remote &remote)
{
- if(remote.isNull())
+ if(!remote.isValid())
return;
m_remotes[remote.name()] = remote;
diff --git a/src/remote.hpp b/src/remote.hpp
@@ -25,17 +25,27 @@
class Remote {
public:
+ enum ReadCode {
+ Success,
+ ReadFailure,
+ InvalidName,
+ InvalidUrl,
+ };
+
+ static ReadCode fromFile(const std::string &path, Remote *remote);
+
Remote();
Remote(const std::string &name, const std::string &url,
const bool enabled = true);
- void setName(const std::string &name) { m_name = name; }
+ void setName(const std::string &name);
const std::string &name() const { return m_name; }
- void setUrl(const std::string &url) { m_url = url; }
+ void setUrl(const std::string &url);
const std::string &url() const { return m_url; }
bool isNull() const { return m_name.empty() || m_url.empty(); }
+ bool isValid() const { return !isNull(); }
void enable() { setEnabled(true); }
void disable() { setEnabled(false); }
diff --git a/test/remote.cpp b/test/remote.cpp
@@ -2,6 +2,10 @@
#include <remote.hpp>
+#include <errors.hpp>
+
+#define RPATH "test/remote/"
+
using namespace std;
static const char *M = "[remote]";
@@ -14,6 +18,62 @@ TEST_CASE("construct remote", M) {
REQUIRE_FALSE(remote.isFrozen());
}
+TEST_CASE("construct invalid remote", M) {
+ SECTION("empty name") {
+ try {
+ Remote remote({}, "url");
+ FAIL();
+ }
+ catch(const reapack_error &e) {
+ REQUIRE(string(e.what()) == "invalid name");
+ }
+ }
+
+ SECTION("invalid name") {
+ try {
+ Remote remote("/", "url");
+ FAIL();
+ }
+ catch(const reapack_error &e) {
+ REQUIRE(string(e.what()) == "invalid name");
+ }
+ }
+
+ SECTION("empty url") {
+ try {
+ Remote remote("name", {});
+ FAIL();
+ }
+ catch(const reapack_error &e) {
+ REQUIRE(string(e.what()) == "invalid url");
+ }
+ }
+}
+
+TEST_CASE("set invalid values", M) {
+ Remote remote;
+
+ SECTION("name") {
+ try {
+ remote.setName("/");
+ FAIL();
+ }
+ catch(const reapack_error &e) {
+ REQUIRE(string(e.what()) == "invalid name");
+ }
+ }
+
+ SECTION("url") {
+ try {
+ remote.setUrl({});
+ FAIL();
+ }
+ catch(const reapack_error &e) {
+ REQUIRE(string(e.what()) == "invalid url");
+ }
+ }
+}
+
TEST_CASE("null remote", M) {
Remote remote;
REQUIRE(remote.isNull());
@@ -79,3 +139,44 @@ TEST_CASE("get remote by name", M) {
list.add({"hello", "world"});
REQUIRE_FALSE(list.get("hello").isNull());
}
+
+TEST_CASE("remote from file", M) {
+ Remote remote;
+
+ SECTION("not found") {
+ const auto code = Remote::fromFile(RPATH "404.ReaPackRemote", &remote);
+ REQUIRE(code == Remote::ReadFailure);
+ REQUIRE_FALSE(remote.isValid());
+ }
+
+ SECTION("valid") {
+ const auto code = Remote::fromFile(RPATH "test.ReaPackRemote", &remote);
+ REQUIRE(code == Remote::Success);
+ REQUIRE(remote.isValid());
+ REQUIRE(remote.name() == "name");
+ REQUIRE(remote.url() == "url");
+ }
+
+ SECTION("invalid name") {
+ const auto code = Remote::fromFile(
+ RPATH "invalid_name.ReaPackRemote", &remote);
+
+ REQUIRE(code == Remote::InvalidName);
+ REQUIRE_FALSE(remote.isValid());
+ }
+
+ SECTION("invalid url") {
+ const auto code = Remote::fromFile(
+ RPATH "missing_url.ReaPackRemote", &remote);
+
+ REQUIRE(code == Remote::InvalidUrl);
+ REQUIRE_FALSE(remote.isValid());
+ }
+
+ SECTION("unicode name") {
+ const auto code = Remote::fromFile(
+ RPATH "Новая папка.ReaPackRemote", &remote);
+
+ REQUIRE(code == Remote::Success);
+ }
+};
diff --git a/test/remote/invalid_name.ReaPackRemote b/test/remote/invalid_name.ReaPackRemote
@@ -0,0 +1,2 @@
+whatever &|%{}
+url
diff --git a/test/remote/missing_url.ReaPackRemote b/test/remote/missing_url.ReaPackRemote
@@ -0,0 +1 @@
+hello world
diff --git a/test/remote/test.ReaPackRemote b/test/remote/test.ReaPackRemote
@@ -0,0 +1,2 @@
+name
+url
diff --git a/test/remote/Новая папка.ReaPackRemote b/test/remote/Новая папка.ReaPackRemote
@@ -0,0 +1,2 @@
+name
+url
