Improve FEN parser

src/Fen.cpp

@@ -5,19 +5,30 @@ namespace chessarbiter {
 std::string FENParser::normalize_rank(std::string fen_rank) {
   std::string normalized;
   for (char &c : fen_rank) {
-    if (IS_DIGIT(c)) {
+    if (FEN__IS_DIGIT(c)) {
       for (char i = 0; i < (c - '0'); i++) {
         normalized += ' ';
       }
     } else {
+      // Check validity
+      char c2 = std::tolower(c);
+      if (!(c2 == 'p' || c2 == 'r' || c2 == 'n' || c2 == 'b' || c2 == 'q' ||
+            c2 == 'k' || c2 == ' ')) {
+        throw InvalidFEN();
+      }
+      // Add
       normalized += c;
     }
   }
+  // Check validity
+  if (normalized.size() != 8) {
+    throw InvalidFEN();
+  }
   return (normalized);
 }
 
 char FENParser::NextToken(std::string fen, char loc) {
-  while (loc < fen.size() && IS_BLANK(fen[loc])) {
+  while (loc < fen.size() && FEN__IS_BLANK(fen[loc])) {
     loc++;
   }
   return (loc);
@@ -99,6 +110,7 @@ FEN FENParser::Parse(std::string fen) {
   // Parse board
   char loc = 0;
   for (char rank = 0; rank < 8; rank++) {
+    FEN__CHECK_LOC();
     char newloc = NextRank(fen, loc);
     parsed.board += normalize_rank(fen.substr(loc, newloc - loc));
     loc = newloc + 1;
@@ -106,15 +118,20 @@ FEN FENParser::Parse(std::string fen) {
 
   // Parse player to move
   loc = NextToken(fen, loc);
+  if (!(fen[loc] == 'w' || fen[loc] == 'b')) {
+    throw InvalidFEN();
+  }
   parsed.player = fen[loc] == 'b';
+  FEN__CHECK_LOC();
 
   // Parse castling
   loc = NextToken(fen, loc + 1);
   char length = 0;
   char cur_loc = loc;
-  while (!IS_BLANK(fen[cur_loc])) {
+  while (!FEN__IS_BLANK(fen[cur_loc])) {
     length++;
     cur_loc++;
+    FEN__CHECK_LOC();
   }
   parsed.white_castle_short = false;
   parsed.white_castle_long = false;
@@ -135,6 +152,10 @@ FEN FENParser::Parse(std::string fen) {
     case 'q':
       parsed.black_castle_long = true;
       break;
+    case '-':
+      break;
+    default:
+      throw InvalidFEN();
     }
   }
 
@@ -143,22 +164,37 @@ FEN FENParser::Parse(std::string fen) {
   if (fen[loc] != '-') {
     parsed.en_passant = fen.substr(loc, 2);
     loc++;
+    // Check format
+    char f = parsed.en_passant[0];
+    char r = parsed.en_passant[1];
+    if (!((f >= 'a' && f <= 'h') && (r >= '1' && r <= '8'))) {
+      throw InvalidFEN();
+    }
   }
   loc++;
+  FEN__CHECK_LOC();
 
   // Parse half move counter
   loc = NextToken(fen, loc);
   std::string halfmove;
-  while (!IS_BLANK(fen[loc])) {
+  while (!FEN__IS_BLANK(fen[loc])) {
+    if (!FEN__IS_DIGIT(fen[loc])) {
+      throw InvalidFEN();
+    }
     halfmove += fen[loc];
     loc++;
+    FEN__CHECK_LOC();
   }
   parsed.halfmove = stoi(halfmove);
 
   // Parse move counter
   loc = NextToken(fen, loc);
   std::string move;
-  while (loc < fen.size() && !IS_BLANK(fen[loc])) {
+  while (loc < fen.size() && !FEN__IS_BLANK(fen[loc])) {
+    if (!FEN__IS_DIGIT(fen[loc])) {
+      throw InvalidFEN();
+    }
+    FEN__CHECK_LOC();
     move += fen[loc];
     loc++;
   }

src/Fen.hpp

@@ -2,10 +2,11 @@
 #include <sstream>
 #include <string>
 
-#define IS_DIGIT(c)                                                            \
+#define FEN__IS_DIGIT(c)                                                            \
   (c == '0' || c == '1' || c == '2' || c == '3' || c == '4' || c == '5' ||     \
    c == '6' || c == '7' || c == '8' || c == '9')
-#define IS_BLANK(c) (c == ' ' || c == '\n' || c == '\t' || c == '\r')
+#define FEN__IS_BLANK(c) (c == ' ' || c == '\n' || c == '\t' || c == '\r')
+#define FEN__CHECK_LOC() {if(loc>=fen.size()){throw InvalidFEN();}}
 
 namespace chessarbiter {
 
@@ -33,9 +34,13 @@ private:
   static char NextRank(std::string fen, char loc);
 
 public:
-  /// @brief Parse a FEN from a string
+  /// @brief Parse a FEN from a string (can throw InvalidFEN)
   static FEN Parse(std::string);
   /// @brief Generate a fen string from the FEN object
   static std::string Serialize(FEN fen);
 };
+
+struct InvalidFEN : public std::exception {
+  const char *what() const throw() { return "No piece found"; }
+};
 } // namespace chessarbiter

tests/fen.cpp

@@ -3,48 +3,36 @@
 
 using namespace chessarbiter;
 
-TEST_CASE("Serializer", "[fen/serialize]") {
-  FEN f;
-  f.board = "p    p p"
-            "p      p"
-            "        "
-            "QQQQQQQQ"
-            "kpkpkpkp"
-            "        "
-            "p p r b "
-            "       R";
-  REQUIRE(FENParser::Serialize(f) ==
-          "p4p1p/p6p/8/QQQQQQQQ/kpkpkpkp/8/p1p1r1b1/7R w KQkq - 0 1");
-
-  f.white_castle_short = false;
-  f.white_castle_long = false;
-  f.black_castle_short = false;
-  f.black_castle_long = false;
-  REQUIRE(FENParser::Serialize(f) ==
-          "p4p1p/p6p/8/QQQQQQQQ/kpkpkpkp/8/p1p1r1b1/7R w - - 0 1");
-
-  f.en_passant = "a3";
-  REQUIRE(FENParser::Serialize(f) ==
-          "p4p1p/p6p/8/QQQQQQQQ/kpkpkpkp/8/p1p1r1b1/7R w - a3 0 1");
-
-  f.player = true;
-  REQUIRE(FENParser::Serialize(f) ==
-          "p4p1p/p6p/8/QQQQQQQQ/kpkpkpkp/8/p1p1r1b1/7R b - a3 0 1");
-
-  f.halfmove = 5;
-  REQUIRE(FENParser::Serialize(f) ==
-          "p4p1p/p6p/8/QQQQQQQQ/kpkpkpkp/8/p1p1r1b1/7R b - a3 5 1");
-
-  f.move = 5;
-  REQUIRE(FENParser::Serialize(f) ==
-          "p4p1p/p6p/8/QQQQQQQQ/kpkpkpkp/8/p1p1r1b1/7R b - a3 5 5");
-}
-
 TEST_CASE("Parse", "[fen/parse]") {
   FEN f;
+  // Start game FEN
+  REQUIRE_NOTHROW(FENParser::Parse(
+      "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1"));
+
+  // Throw tests
+  REQUIRE_THROWS_AS(
+      FENParser::Parse(
+          "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR z KQkq - 0 1"),
+      InvalidFEN);
+  REQUIRE_THROWS_AS(
+      FENParser::Parse(
+          "rnbqkbnr/ppppppp2/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1"),
+      InvalidFEN);
+  REQUIRE_THROWS_AS(
+      FENParser::Parse(
+          "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w pQkq - 0 1"),
+      InvalidFEN);
+  REQUIRE_THROWS_AS(
+      FENParser::Parse("rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP w KQkq - 0 1"),
+      InvalidFEN);
+  REQUIRE_THROWS_AS(
+      FENParser::Parse(
+          "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 1a 1"),
+      InvalidFEN);
+
+  // Tests
   f = FENParser::Parse(
       "p4p1p/p6p/8/QQQQQQQQ/kpkpkpkp/8/p1p1r1b1/7R b - a3 5 5");
-
   REQUIRE(f.board == "p    p p"
                      "p      p"
                      "        "
@@ -89,3 +77,40 @@ TEST_CASE("Parse", "[fen/parse]") {
   CHECK(f.black_castle_short == true);
   CHECK(f.black_castle_long == true);
 }
+
+TEST_CASE("Serializer", "[fen/serialize]") {
+  FEN f;
+  f.board = "p    p p"
+            "p      p"
+            "        "
+            "QQQQQQQQ"
+            "kpkpkpkp"
+            "        "
+            "p p r b "
+            "       R";
+  REQUIRE(FENParser::Serialize(f) ==
+          "p4p1p/p6p/8/QQQQQQQQ/kpkpkpkp/8/p1p1r1b1/7R w KQkq - 0 1");
+
+  f.white_castle_short = false;
+  f.white_castle_long = false;
+  f.black_castle_short = false;
+  f.black_castle_long = false;
+  REQUIRE(FENParser::Serialize(f) ==
+          "p4p1p/p6p/8/QQQQQQQQ/kpkpkpkp/8/p1p1r1b1/7R w - - 0 1");
+
+  f.en_passant = "a3";
+  REQUIRE(FENParser::Serialize(f) ==
+          "p4p1p/p6p/8/QQQQQQQQ/kpkpkpkp/8/p1p1r1b1/7R w - a3 0 1");
+
+  f.player = true;
+  REQUIRE(FENParser::Serialize(f) ==
+          "p4p1p/p6p/8/QQQQQQQQ/kpkpkpkp/8/p1p1r1b1/7R b - a3 0 1");
+
+  f.halfmove = 5;
+  REQUIRE(FENParser::Serialize(f) ==
+          "p4p1p/p6p/8/QQQQQQQQ/kpkpkpkp/8/p1p1r1b1/7R b - a3 5 1");
+
+  f.move = 5;
+  REQUIRE(FENParser::Serialize(f) ==
+          "p4p1p/p6p/8/QQQQQQQQ/kpkpkpkp/8/p1p1r1b1/7R b - a3 5 5");
+}