SongFilter: Parse() throws exception on error

4 files changed, 52 insertions, 42 deletions

diff --git a/src/SongFilter.cxx b/src/SongFilter.cxx
index de43b2fad..dd201d959 100644
--- a/src/SongFilter.cxx
+++ b/src/SongFilter.cxx
@@ -177,7 +177,6 @@ SongFilter::~SongFilter()
 	/* this destructor exists here just so it won't get inlined */
 }
 
-gcc_pure
 static std::chrono::system_clock::time_point
 ParseTimeStamp(const char *s)
 {
@@ -189,53 +188,39 @@ ParseTimeStamp(const char *s)
 		/* it's an integral UNIX time stamp */
 		return std::chrono::system_clock::from_time_t((time_t)value);
 
-	try {
-		/* try ISO 8601 */
-		return ParseTimePoint(s, "%FT%TZ");
-	} catch (...) {
-		return std::chrono::system_clock::time_point::min();
-	}
+	/* try ISO 8601 */
+	return ParseTimePoint(s, "%FT%TZ");
 }
 
-bool
+void
 SongFilter::Parse(const char *tag_string, const char *value, bool fold_case)
 {
 	unsigned tag = locate_parse_type(tag_string);
 	if (tag == TAG_NUM_OF_ITEM_TYPES)
-		return false;
+		throw std::runtime_error("Unknown filter type");
 
 	if (tag == LOCATE_TAG_BASE_TYPE) {
 		if (!uri_safe_local(value))
-			return false;
+			throw std::runtime_error("Bad URI");
 
 		/* case folding doesn't work with "base" */
 		fold_case = false;
 	}
 
-	if (tag == LOCATE_TAG_MODIFIED_SINCE) {
-		const auto t = ParseTimeStamp(value);
-		if (IsNegative(t))
-			return false;
-
-		items.push_back(Item(tag, t));
-		return true;
-	}
-
-	items.push_back(Item(tag, value, fold_case));
-	return true;
+	if (tag == LOCATE_TAG_MODIFIED_SINCE)
+		items.push_back(Item(tag, ParseTimeStamp(value)));
+	else
+		items.push_back(Item(tag, value, fold_case));
 }
 
-bool
+void
 SongFilter::Parse(ConstBuffer<const char *> args, bool fold_case)
 {
 	if (args.size == 0 || args.size % 2 != 0)
-		return false;
+		throw std::runtime_error("Incorrect number of filter arguments");
 
 	for (unsigned i = 0; i < args.size; i += 2)
-		if (!Parse(args[i], args[i + 1], fold_case))
-			return false;
-
-	return true;
+		Parse(args[i], args[i + 1], fold_case);
 }
 
 bool
diff --git a/src/SongFilter.hxx b/src/SongFilter.hxx
index 7303d90e3..3f2abcd14 100644
--- a/src/SongFilter.hxx
+++ b/src/SongFilter.hxx
@@ -112,10 +112,13 @@ public:
 
 private:
 	gcc_nonnull(2,3)
-	bool Parse(const char *tag, const char *value, bool fold_case=false);
+	void Parse(const char *tag, const char *value, bool fold_case=false);
 
 public:
-	bool Parse(ConstBuffer<const char *> args, bool fold_case=false);
+	/**
+	 * Throws on error.
+	 */
+	void Parse(ConstBuffer<const char *> args, bool fold_case=false);
 
 	gcc_pure
 	bool Match(const DetachedSong &song) const noexcept;
diff --git a/src/command/DatabaseCommands.cxx b/src/command/DatabaseCommands.cxx
index 9ab41958b..1b6e59769 100644
--- a/src/command/DatabaseCommands.cxx
+++ b/src/command/DatabaseCommands.cxx
@@ -31,6 +31,7 @@
 #include "tag/ParseName.hxx"
 #include "tag/Mask.hxx"
 #include "util/ConstBuffer.hxx"
+#include "util/Exception.hxx"
 #include "util/StringAPI.hxx"
 #include "SongFilter.hxx"
 #include "BulkEdit.hxx"
@@ -96,8 +97,11 @@ handle_match(Client &client, Request args, Response &r, bool fold_case)
 	}
 
 	SongFilter filter;
-	if (!filter.Parse(args, fold_case)) {
-		r.Error(ACK_ERROR_ARG, "incorrect arguments");
+	try {
+		filter.Parse(args, fold_case);
+	} catch (...) {
+		r.Error(ACK_ERROR_ARG,
+			GetFullMessage(std::current_exception()).c_str());
 		return CommandResult::ERROR;
 	}
 
@@ -126,8 +130,11 @@ static CommandResult
 handle_match_add(Client &client, Request args, Response &r, bool fold_case)
 {
 	SongFilter filter;
-	if (!filter.Parse(args, fold_case)) {
-		r.Error(ACK_ERROR_ARG, "incorrect arguments");
+	try {
+		filter.Parse(args, fold_case);
+	} catch (...) {
+		r.Error(ACK_ERROR_ARG,
+			GetFullMessage(std::current_exception()).c_str());
 		return CommandResult::ERROR;
 	}
 
@@ -157,8 +164,11 @@ handle_searchaddpl(Client &client, Request args, Response &r)
 	const char *playlist = args.shift();
 
 	SongFilter filter;
-	if (!filter.Parse(args, true)) {
-		r.Error(ACK_ERROR_ARG, "incorrect arguments");
+	try {
+		filter.Parse(args, true);
+	} catch (...) {
+		r.Error(ACK_ERROR_ARG,
+			GetFullMessage(std::current_exception()).c_str());
 		return CommandResult::ERROR;
 	}
 
@@ -187,9 +197,14 @@ handle_count(Client &client, Request args, Response &r)
 	}
 
 	SongFilter filter;
-	if (!args.empty() && !filter.Parse(args, false)) {
-		r.Error(ACK_ERROR_ARG, "incorrect arguments");
-		return CommandResult::ERROR;
+	if (!args.empty()) {
+		try {
+			filter.Parse(args, false);
+		} catch (...) {
+			r.Error(ACK_ERROR_ARG,
+				GetFullMessage(std::current_exception()).c_str());
+			return CommandResult::ERROR;
+		}
 	}
 
 	PrintSongCount(r, client.GetPartition(), "", &filter, group);
@@ -255,8 +270,11 @@ handle_list(Client &client, Request args, Response &r)
 
 	if (!args.empty()) {
 		filter.reset(new SongFilter());
-		if (!filter->Parse(args, false)) {
-			r.Error(ACK_ERROR_ARG, "not able to parse args");
+		try {
+			filter->Parse(args, false);
+		} catch (...) {
+			r.Error(ACK_ERROR_ARG,
+				GetFullMessage(std::current_exception()).c_str());
 			return CommandResult::ERROR;
 		}
 	}
diff --git a/src/command/QueueCommands.cxx b/src/command/QueueCommands.cxx
index 3457842c7..89dd885f3 100644
--- a/src/command/QueueCommands.cxx
+++ b/src/command/QueueCommands.cxx
@@ -35,6 +35,7 @@
 #include "Instance.hxx"
 #include "BulkEdit.hxx"
 #include "util/ConstBuffer.hxx"
+#include "util/Exception.hxx"
 #include "util/StringAPI.hxx"
 #include "util/NumberParser.hxx"
 
@@ -264,8 +265,11 @@ handle_playlist_match(Client &client, Request args, Response &r,
 		      bool fold_case)
 {
 	SongFilter filter;
-	if (!filter.Parse(args, fold_case)) {
-		r.Error(ACK_ERROR_ARG, "incorrect arguments");
+	try {
+		filter.Parse(args, fold_case);
+	} catch (...) {
+		r.Error(ACK_ERROR_ARG,
+			GetFullMessage(std::current_exception()).c_str());
 		return CommandResult::ERROR;
 	}