Second attempt at solving #1173 (#1223)

Author: cowtowncoder

release-notes/VERSION-2.x

@@ -30,6 +30,9 @@ a pure JSON library.
  (contributed by @pjfanning)
 #1169: `ArrayIndexOutOfBoundsException` for specific invalid content,
   with Reader-based parser
+#1173: `JsonLocation` consistently off by one character for many
+  invalid JSON parsing cases
+ (reported by Paul B)
 #1179: Allow configuring `DefaultPrettyPrinter` separators for empty
   Arrays and Objects
  (contributed by Guillaume L)

src/main/java/com/fasterxml/jackson/core/base/ParserBase.java

@@ -1373,16 +1373,17 @@ protected char _handleUnrecognizedCharacterEscape(char ch) throws JsonProcessing
         if (ch == '\'' && isEnabled(Feature.ALLOW_SINGLE_QUOTES)) {
             return ch;
         }
-        _reportError("Unrecognized character escape "+_getCharDesc(ch));
-        return ch;
+        throw _constructReadException("Unrecognized character escape "+_getCharDesc(ch),
+                _currentLocationMinusOne());
     }
 
     protected void _reportMismatchedEndMarker(int actCh, char expCh) throws JsonParseException {
-        JsonReadContext ctxt = getParsingContext();
-        _reportError(String.format(
+        final JsonReadContext ctxt = getParsingContext();
+        final String msg = String.format(
                 "Unexpected close marker '%s': expected '%c' (for %s starting at %s)",
                 (char) actCh, expCh, ctxt.typeDesc(),
-                ctxt.startLocation(_contentReference())));
+                ctxt.startLocation(_contentReference()));
+        throw _constructReadException(msg, _currentLocationMinusOne());
     }
 
     /**
@@ -1402,7 +1403,7 @@ protected void _throwUnquotedSpace(int i, String ctxtDesc) throws JsonParseExcep
         if (!isEnabled(Feature.ALLOW_UNQUOTED_CONTROL_CHARS) || i > INT_SPACE) {
             char c = (char) i;
             String msg = "Illegal unquoted character ("+_getCharDesc(c)+"): has to be escaped using backslash to be included in "+ctxtDesc;
-            _reportError(msg);
+            throw _constructReadException(msg, _currentLocationMinusOne());
         }
     }
 

src/main/java/com/fasterxml/jackson/core/base/ParserMinimalBase.java

@@ -682,7 +682,7 @@ protected void _reportUnexpectedChar(int ch, String comment) throws JsonParseExc
         if (comment != null) {
             msg += ": "+comment;
         }
-        throw _constructReadException(msg, currentLocation());
+        throw _constructReadException(msg, _currentLocationMinusOne());
     }
 
     /**
@@ -698,7 +698,7 @@ protected <T> T _reportUnexpectedNumberChar(int ch, String comment) throws JsonP
         if (comment != null) {
             msg += ": "+comment;
         }
-        throw _constructReadException(msg, currentLocation());
+        throw _constructReadException(msg, _currentLocationMinusOne());
     }
 
     @Deprecated // @since 2.14
@@ -722,6 +722,23 @@ protected final JsonParseException _constructError(String msg, Throwable t) {
         return _constructReadException(msg, t);
     }
 
+    /**
+     * Factory method used to provide location for cases where we must read
+     * and consume a single "wrong" character (to possibly allow error recovery),
+     * but need to report accurate location for that character: if so, the
+     * current location is past location we want, and location we want will be
+     * "one location earlier".
+     *<p>
+     * Default implementation simply returns {@link #currentLocation()}
+     *
+     * @since 2.17
+     *
+     * @return Same as {@link #currentLocation()} except offset by -1
+     */
+    protected JsonLocation _currentLocationMinusOne() {
+        return currentLocation();
+    }
+    
     protected final static String _getCharDesc(int ch)
     {
         char c = (char) ch;

src/main/java/com/fasterxml/jackson/core/json/ReaderBasedJsonParser.java

@@ -2979,6 +2979,15 @@ public JsonLocation currentLocation() {
                 _currInputRow, col);
     }
 
+    @Override // @since 2.17
+    protected JsonLocation _currentLocationMinusOne() {
+        final int prevInputPtr = _inputPtr - 1;
+        final int col = prevInputPtr - _currInputRowStart + 1; // 1-based
+        return new JsonLocation(_contentReference(),
+                -1L, _currInputProcessed + prevInputPtr,
+                _currInputRow, col);
+    }
+
     @Override
     public JsonLocation currentTokenLocation()
     {

src/main/java/com/fasterxml/jackson/core/json/UTF8DataInputJsonParser.java

@@ -2975,6 +2975,12 @@ public JsonLocation currentLocation() {
                 _currInputRow, col);
     }
 
+    // Since we only know row, may as well return currentLocation()
+    @Override // @since 2.17
+    protected JsonLocation _currentLocationMinusOne() {
+        return currentLocation();
+    }
+
     @Override
     public JsonLocation currentTokenLocation() {
         // 03-Jan-2020, tatu: Should probably track this, similar to how

src/main/java/com/fasterxml/jackson/core/json/UTF8StreamJsonParser.java

@@ -3873,6 +3873,15 @@ public JsonLocation currentLocation()
                 _currInputRow, col);
     }
 
+    @Override // @since 2.17
+    protected JsonLocation _currentLocationMinusOne() {
+        final int prevInputPtr = _inputPtr - 1;
+        final int col = prevInputPtr - _currInputRowStart + 1; // 1-based
+        return new JsonLocation(_contentReference(),
+                _currInputProcessed + prevInputPtr, -1L, // bytes, chars
+                _currInputRow, col);
+    }
+
     @Override
     public JsonLocation currentTokenLocation()
     {

src/main/java/com/fasterxml/jackson/core/json/async/NonBlockingJsonParserBase.java

@@ -355,6 +355,16 @@ public JsonLocation currentLocation()
                 row, col);
     }
 
+    @Override // @since 2.17
+    protected JsonLocation _currentLocationMinusOne() {
+        final int prevInputPtr = _inputPtr - 1;
+        int row = Math.max(_currInputRow, _currInputRowAlt);
+        final int col = prevInputPtr - _currInputRowStart + 1; // 1-based
+        return new JsonLocation(_contentReference(),
+                _currInputProcessed + (prevInputPtr - _currBufferStart), -1L, // bytes, chars
+                row, col);
+    }
+
     @Override
     public JsonLocation currentTokenLocation()
     {

src/test/java/com/fasterxml/jackson/failing/LocationOfError1173Test.java renamed to src/test/java/com/fasterxml/jackson/core/read/loc/LocationOfError1173Test.java

@@ -1,4 +1,4 @@
-package com.fasterxml.jackson.failing;
+package com.fasterxml.jackson.core.read.loc;
 
 import java.io.*;
 import java.nio.charset.StandardCharsets;