Issue #163 fix

src/main/java/org/gedcom4j/parser/LinePieces.java

@@ -63,6 +63,11 @@ class LinePieces {
     /** The characters in the line */
     private final char[] chars;
 
+    /**
+     * The number of the line we are breaking into pieces
+     */
+    private final int lineNum;
+
     /**
      * Constructor that makes a {@link LinePieces} object from a line of text input from a GEDCOM file
      * 
@@ -74,27 +79,21 @@ class LinePieces {
      *             if the line of text cannot be split into pieces
      */
     LinePieces(String lineToParse, int lineNum) throws GedcomParserException {
-
+        this.lineNum = lineNum;
         chars = lineToParse.toCharArray();
-
-        processLevel(lineNum);
-
+        processLevel();
         processXrefId();
-
         processTag();
-
         processRemainder();
     }
 
     /**
      * Process the level portion of the line
      * 
-     * @param lineNum
-     *            the line number we're currently reading
      * @throws GedcomParserException
      *             if the line does not begin with a 1 or 2 digit number for level, followed by a space
      */
-    private void processLevel(int lineNum) throws GedcomParserException {
+    private void processLevel() throws GedcomParserException {
         try {
             char c2 = chars[1]; // 2nd character in line
 
@@ -116,6 +115,10 @@ private void processLevel(int lineNum) throws GedcomParserException {
             throw new GedcomParserException("Line " + lineNum
                     + " does not begin with a 1 or 2 digit number for the level followed by a space: " + new String(chars), e);
         }
+        if (level < 0 || level > 99) {
+            throw new GedcomParserException("Line " + lineNum
+                    + " does not begin with a 1 or 2 digit number for the level followed by a space: " + new String(chars));
+        }
     }
 
     /**
@@ -129,22 +132,31 @@ private void processRemainder() {
 
     /**
      * Process the tag portion of the line
+     * 
+     * @throws GedcomParserException
+     *             if no tag could be found on the line
      */
-    private void processTag() {
+    private void processTag() throws GedcomParserException {
         // Parse the tag
         StringBuilder t = new StringBuilder();
         while (currCharIdx < chars.length && chars[currCharIdx] != ' ') {
             t.append(chars[currCharIdx++]);
         }
         if (t.length() > 0) {
             tag = t.toString().intern();
+        } else {
+            throw new GedcomParserException("All GEDCOM lines are required to have a tag value, but no tag could be found on line "
+                    + lineNum);
         }
     }
 
     /**
      * Process the XREF ID portion of the line, if there is one
+     * 
+     * @throws GedcomParserException
+     *             if the XREF is not properly terminated with an @ sign
      */
-    private void processXrefId() {
+    private void processXrefId() throws GedcomParserException {
         // Take care of the id, if any
         StringBuilder i = null;
         if ('@' == (chars[currCharIdx])) {
@@ -156,7 +168,11 @@ private void processXrefId() {
             }
             currCharIdx++;
         }
+
         if (i != null) {
+            if (i.charAt(i.length() - 1) != '@') {
+                throw new GedcomParserException("XRef ID begins with @ sign but is not terminated with one on line " + lineNum);
+            }
             id = i.toString().intern();
         }
     }

src/test/java/org/gedcom4j/parser/LinePiecesTest.java

@@ -0,0 +1,115 @@
+package org.gedcom4j.parser;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+import org.gedcom4j.exception.GedcomParserException;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+/**
+ * Test for {@link LinePieces}
+ * 
+ * @author frizbog
+ */
+public class LinePiecesTest {
+
+    /**
+     * A rule that can be used to check exceptions
+     */
+    @Rule
+    public ExpectedException thrown = ExpectedException.none();
+
+    /**
+     * Negative test case when the level is non-numeric
+     * 
+     * @throws GedcomParserException
+     *             if anything goes wrong
+     */
+    @SuppressWarnings("unused")
+    @Test
+    public void testLinePiecesNegativeBadLevel() throws GedcomParserException {
+        thrown.expect(GedcomParserException.class);
+        thrown.expectMessage("Line 5 does not begin with a 1 or 2 digit number for the level followed by a space:");
+        new LinePieces("BAD TAGG additional stuff", 5);
+    }
+
+    /**
+     * Negative test case when the xref is not terminated
+     * 
+     * @throws GedcomParserException
+     *             if anything goes wrong
+     */
+    @SuppressWarnings("unused")
+    @Test
+    public void testLinePiecesNegativeBadXref() throws GedcomParserException {
+        thrown.expect(GedcomParserException.class);
+        thrown.expectMessage("XRef ID begins with @ sign but is not terminated with one on line 5");
+        new LinePieces("4 @XREF TAGG additional stuff", 5);
+    }
+
+    /**
+     * Negative test case when there is no tag
+     * 
+     * @throws GedcomParserException
+     *             if anything goes wrong
+     */
+    @SuppressWarnings("unused")
+    @Test
+    public void testLinePiecesNegativeNothingAfterXref() throws GedcomParserException {
+        thrown.expect(GedcomParserException.class);
+        thrown.expectMessage("All GEDCOM lines are required to have a tag value, but no tag could be found on line 5");
+        new LinePieces("4 @XREF@", 5);
+    }
+
+    /**
+     * Simple happy-path positive test for {@link LinePieces} constructor
+     * 
+     * @throws GedcomParserException
+     *             if anything goes wrong
+     */
+    @Test
+    public void testLinePiecesPositive() throws GedcomParserException {
+        LinePieces lp = new LinePieces("4 @XREF@ TAGG additional stuff", 5);
+        assertNotNull(lp);
+        assertEquals(4, lp.level);
+        assertEquals("@XREF@", lp.id);
+        assertEquals("TAGG", lp.tag);
+        assertEquals("additional stuff", lp.remainder);
+    }
+
+    /**
+     * Simple happy-path positive test for {@link LinePieces} constructor
+     * 
+     * @throws GedcomParserException
+     *             if anything goes wrong
+     */
+    @Test
+    public void testLinePiecesPositiveNothingAfterTag() throws GedcomParserException {
+        LinePieces lp = new LinePieces("4 TAGG", 5);
+        assertNotNull(lp);
+        assertEquals(4, lp.level);
+        assertNull(lp.id);
+        assertEquals("TAGG", lp.tag);
+        assertNull(lp.remainder);
+    }
+
+    /**
+     * Simple happy-path positive test for {@link LinePieces} constructor
+     * 
+     * @throws GedcomParserException
+     *             if anything goes wrong
+     */
+    @Test
+    public void testLinePiecesPositiveNoXref() throws GedcomParserException {
+        LinePieces lp = new LinePieces("4 TAGG additional stuff", 5);
+        assertNotNull(lp);
+        assertEquals(4, lp.level);
+        assertNull(lp.id);
+        assertEquals("TAGG", lp.tag);
+        assertEquals("additional stuff", lp.remainder);
+    }
+
+}