Fix #356

Author: cowtowncoder

release-notes/VERSION

@@ -28,6 +28,7 @@ JSON library.
  (reported by Brad H)
 #340: Making `WriterBasedJsonGenerator` non-final
  (requested by rfoltyns@github)
+#356: Improve indication of "source reference" in `JsonLocation` wrt `byte[]`,`char[]`
 
 2.8.7 (not yet released)
 

src/main/java/com/fasterxml/jackson/core/JsonLocation.java

@@ -5,6 +5,8 @@
 
 package com.fasterxml.jackson.core;
 
+import java.nio.charset.Charset;
+
 /**
  * Object that encapsulates Location information used for reporting
  * parsing (or potentially generation) errors, as well as current location
@@ -15,17 +17,29 @@ public class JsonLocation
 {
     private static final long serialVersionUID = 1L;
 
+    /**
+     * Include at most first 500 characters/bytes from contents; should be enough
+     * to give context, but not cause unfortunate side effects in things like
+     * logs.
+     *
+     * @since 2.9
+     */
+    public static final int MAX_CONTENT_SNIPPET = 500;
+    
     /**
      * Shared immutable "N/A location" that can be returned to indicate
-     * that no location information is available
+     * that no location information is available.
+     *<p>
+     * NOTE: before 2.9, Location was given as String "N/A"; with 2.9 it was
+     * removed so that source should be indicated as "UNKNOWN".
      */
-    public final static JsonLocation NA = new JsonLocation("N/A", -1L, -1L, -1, -1);
+    public final static JsonLocation NA = new JsonLocation(null, -1L, -1L, -1, -1);
 
-    final long _totalBytes;
-    final long _totalChars;
+    protected final long _totalBytes;
+    protected final long _totalChars;
 
-    final int _lineNr;
-    final int _columnNr;
+    protected final int _lineNr;
+    protected final int _columnNr;
 
     /**
      * Displayable description for input source: file path, URL.
@@ -64,6 +78,17 @@ public JsonLocation(Object sourceRef, long totalBytes, long totalChars,
      */
     public Object getSourceRef() { return _sourceRef; }
 
+    /**
+     * Accessor for getting a textual description of source reference
+     * (Object returned by {@link #getSourceRef()}), as included in
+     * description returned by {@link #toString()}.
+     *
+     * @since 2.9
+     */
+    public String getSourceDescription() {
+        return _appendSourceDesc(new StringBuilder(100)).toString();
+    }
+
     /**
      * @return Line number of the location (1-based)
      */
@@ -94,11 +119,7 @@ public String toString()
     {
         StringBuilder sb = new StringBuilder(80);
         sb.append("[Source: ");
-        if (_sourceRef == null) {
-            sb.append("UNKNOWN");
-        } else {
-            sb.append(_sourceRef.toString());
-        }
+        _appendSourceDesc(sb);
         sb.append("; line: ");
         sb.append(_lineNr);
         sb.append(", column: ");
@@ -107,6 +128,59 @@ public String toString()
         return sb.toString();
     }
 
+    protected StringBuilder _appendSourceDesc(StringBuilder sb)
+    {
+        final Object srcRef = _sourceRef;
+
+        if (srcRef == null) {
+            sb.append("UNKNOWN");
+            return sb;
+        }
+        // First, figure out what name to use as source type
+        Class<?> srcType = (srcRef instanceof Class<?>) ?
+                ((Class<?>) srcRef) : srcRef.getClass();
+        String tn = srcType.getName();
+        // standard JDK types without package
+        if (tn.startsWith("java.")) {
+            tn = srcType.getSimpleName();
+        } else if (srcRef instanceof byte[]) { // then some other special cases
+            tn = "byte[]";
+        } else if (srcRef instanceof char[]) {
+            tn = "char[]";
+        }
+        sb.append('(').append(tn).append(')');
+        // and then, include (part of) contents for selected types:
+        int len;
+        String charStr = " chars";
+
+        if (srcRef instanceof CharSequence) {
+            CharSequence cs = (CharSequence) srcRef;
+            len = cs.length();
+            len -= _append(sb, cs.subSequence(0, Math.min(len, MAX_CONTENT_SNIPPET)).toString());
+        } else if (srcRef instanceof char[]) {
+            char[] ch = (char[]) srcRef;
+            len = ch.length;
+            len -= _append(sb, new String(ch, 0, Math.min(len, MAX_CONTENT_SNIPPET)));
+        } else if (srcRef instanceof byte[]) {
+            byte[] b = (byte[]) srcRef;
+            int maxLen = Math.min(b.length, MAX_CONTENT_SNIPPET);
+            _append(sb, new String(b, 0, maxLen, Charset.forName("UTF-8")));
+            len = b.length - maxLen;
+            charStr = " bytes";
+        } else {
+            len = 0;
+        }
+        if (len > 0) {
+            sb.append("[truncated ").append(len).append(charStr).append(']');
+        }
+        return sb;
+    }
+
+    private int _append(StringBuilder sb, String content) {
+        sb.append('"').append(content).append('"');
+        return content.length();
+    }
+
     @Override
     public int hashCode()
     {

src/main/java/com/fasterxml/jackson/core/JsonParser.java

@@ -58,7 +58,7 @@ public enum Feature {
          * Feature is enabled by default.
          */
         AUTO_CLOSE_SOURCE(true),
-            
+
         // // // Support for non-standard data format constructs
 
         /**
@@ -169,6 +169,49 @@ public enum Feature {
          */
          ALLOW_NON_NUMERIC_NUMBERS(false),
 
+         /**
+          * Feature allows the support for "missing" values in a JSON array: missing
+          * value meaning sequence of two commas, without value in-between but only
+          * optional white space.
+          * Enabling this feature will expose "missing" values as {@link JsonToken#VALUE_NULL}
+          * tokens, which typically become Java nulls in arrays and {@link java.util.Collection}
+          * in data-binding.
+          * <p>
+          * For example, enabling this feature will represent a JSON array <code>["value1",,"value3",]</code>
+          * as <code>["value1", null, "value3", null]</code> 
+          * <p>
+          * Since the JSON specification does not allow missing values this is a non-compliant JSON
+          * feature and is disabled by default.
+          * 
+          * @since 2.8
+          */
+         ALLOW_MISSING_VALUES(false),
+
+         /**
+          * Feature that determines whether {@link JsonParser} will allow for a single trailing
+          * comma following the final value (in an Array) or member (in an Object). These commas
+          * will simply be ignored.
+          * <p>
+          * For example, when this feature is enabled, <code>[true,true,]</code> is equivalent to
+          * <code>[true, true]</code> and <code>{"a": true,}</code> is equivalent to
+          * <code>{"a": true}</code>.
+          * <p>
+          * When combined with <code>ALLOW_MISSING_VALUES</code>, this feature takes priority, and
+          * the final trailing comma in an array declaration does not imply a missing
+          * (<code>null</code>) value. For example, when both <code>ALLOW_MISSING_VALUES</code>
+          * and <code>ALLOW_TRAILING_COMMA</code> are enabled, <code>[true,true,]</code> is
+          * equivalent to <code>[true, true]</code>, and <code>[true,true,,]</code> is equivalent to
+          * <code>[true, true, null]</code>.
+          * <p>
+          * Since the JSON specification does not permit trailing commas, this is a non-standard
+          * feature, and as such disabled by default.
+          *
+          * @since 2.9
+          */
+         ALLOW_TRAILING_COMMA(false),
+
+         // // // Validity checks
+         
          /**
           * Feature that determines whether {@link JsonParser} will explicitly
           * check that no duplicate JSON Object field names are encountered.
@@ -211,46 +254,29 @@ public enum Feature {
           */
          IGNORE_UNDEFINED(false),
 
-         /**
-          * Feature allows the support for "missing" values in a JSON array: missing
-          * value meaning sequence of two commas, without value in-between but only
-          * optional white space.
-          * Enabling this feature will expose "missing" values as {@link JsonToken#VALUE_NULL}
-          * tokens, which typically become Java nulls in arrays and {@link java.util.Collection}
-          * in data-binding.
-          * <p>
-          * For example, enabling this feature will represent a JSON array <code>["value1",,"value3",]</code>
-          * as <code>["value1", null, "value3", null]</code> 
-          * <p>
-          * Since the JSON specification does not allow missing values this is a non-compliant JSON
-          * feature and is disabled by default.
-          * 
-          * @since 2.8
-          */
-         ALLOW_MISSING_VALUES(false),
+         // // // Other
 
          /**
-          * Feature that determines whether {@link JsonParser} will allow for a single trailing
-          * comma following the final value (in an Array) or member (in an Object). These commas
-          * will simply be ignored.
-          * <p>
-          * For example, when this feature is enabled, <code>[true,true,]</code> is equivalent to
-          * <code>[true, true]</code> and <code>{"a": true,}</code> is equivalent to
-          * <code>{"a": true}</code>.
-          * <p>
-          * When combined with <code>ALLOW_MISSING_VALUES</code>, this feature takes priority, and
-          * the final trailing comma in an array declaration does not imply a missing
-          * (<code>null</code>) value. For example, when both <code>ALLOW_MISSING_VALUES</code>
-          * and <code>ALLOW_TRAILING_COMMA</code> are enabled, <code>[true,true,]</code> is
-          * equivalent to <code>[true, true]</code>, and <code>[true,true,,]</code> is equivalent to
-          * <code>[true, true, null]</code>.
-          * <p>
-          * Since the JSON specification does not permit trailing commas, this is a non-standard
-          * feature, and as such disabled by default.
+          * Feature that determines whether {@link JsonLocation} instances should be constructed
+          * with reference to source or not. If source reference is included, its type and contents
+          * are included when `toString()` method is called (most notably when printing out parse
+          * exception with that location information). If feature is disabled, no source reference
+          * is passed and source is only indicated as "UNKNOWN".
+          *<p>
+          * Most common reason for disabling this feature is to avoid leaking information about
+          * internal information; this may be done for security reasons.
+          * Note that even if source reference is included, only parts of contents are usually
+          * printed, and not the whole contents. Further, many source reference types can not
+          * necessarily access contents (like streams), so only type is indicated, not contents.
+          *<p>
+          * Feature is enabled by default, meaning that "source reference" information is passed
+          * and some or all of the source content may be included in {@link JsonLocation} information
+          * constructed either when requested explicitly, or when needed for an exception.
           *
           * @since 2.9
           */
-         ALLOW_TRAILING_COMMA(false)
+         INCLUDE_SOURCE_IN_LOCATION(true),
+         
          ;
 
         /**

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

@@ -442,7 +442,7 @@ protected void _checkStdFeatureChanges(int newFeatureFlags, int changedFeatures)
      */
     @Override
     public JsonLocation getTokenLocation() {
-        return new JsonLocation(_ioContext.getSourceReference(),
+        return new JsonLocation(_getSourceReference(),
                 -1L, getTokenCharacterOffset(), // bytes, chars
                 getTokenLineNr(),
                 getTokenColumnNr());
@@ -455,7 +455,7 @@ public JsonLocation getTokenLocation() {
     @Override
     public JsonLocation getCurrentLocation() {
         int col = _inputPtr - _currInputRowStart + 1; // 1-based
-        return new JsonLocation(_ioContext.getSourceReference(),
+        return new JsonLocation(_getSourceReference(),
                 -1L, _currInputProcessed + _inputPtr, // bytes, chars
                 _currInputRow, col);
     }
@@ -504,7 +504,7 @@ public int getTokenColumnNr() {
 
     /*
     /**********************************************************
-    /* Abstract methods needed from sub-classes
+    /* Abstract methods for sub-classes to implement
     /**********************************************************
      */
 
@@ -543,7 +543,7 @@ protected void _handleEOF() throws JsonParseException {
             _reportInvalidEOF(String.format(
                     ": expected close marker for %s (start marker at %s)",
                     marker,
-                    _parsingContext.getStartLocation(_ioContext.getSourceReference())),
+                    _parsingContext.getStartLocation(_getSourceReference())),
                     null);
         }
     }
@@ -555,17 +555,6 @@ protected final int _eofAsNextChar() throws JsonParseException {
         _handleEOF();
         return -1;
     }
-    
-    /*
-    /**********************************************************
-    /* Internal/package methods: Error reporting
-    /**********************************************************
-     */
-    
-    protected void _reportMismatchedEndMarker(int actCh, char expCh) throws JsonParseException {
-        String startDesc = ""+_parsingContext.getStartLocation(_ioContext.getSourceReference());
-        _reportError("Unexpected close marker '"+((char) actCh)+"': expected '"+expCh+"' (for "+_parsingContext.typeDesc()+" starting at "+startDesc+")");
-    }
 
     /*
     /**********************************************************
@@ -1051,13 +1040,18 @@ protected void convertNumberToBigDecimal() throws IOException
         }
         _numTypesValid |= NR_BIGDECIMAL;
     }
-    
+
     /*
     /**********************************************************
-    /* Number handling exceptions
+    /* Internal/package methods: Error reporting
     /**********************************************************
-     */    
+     */
 
+    protected void _reportMismatchedEndMarker(int actCh, char expCh) throws JsonParseException {
+        String startDesc = ""+_parsingContext.getStartLocation(_getSourceReference());
+        _reportError("Unexpected close marker '"+((char) actCh)+"': expected '"+expCh+"' (for "+_parsingContext.typeDesc()+" starting at "+startDesc+")");
+    }
+
     protected void reportUnexpectedNumberChar(int ch, String comment) throws JsonParseException {
         String msg = "Unexpected character ("+_getCharDesc(ch)+") in numeric value";
         if (comment != null) {
@@ -1162,6 +1156,25 @@ protected IllegalArgumentException reportInvalidBase64Char(Base64Variant b64vari
         return new IllegalArgumentException(base);
     }
 
+    /*
+    /**********************************************************
+    /* Internal/package methods: other
+    /**********************************************************
+     */
+
+    /**
+     * Helper method used to encapsulate logic of including (or not) of
+     * "source reference" when constructing {@link JsonLocation} instances.
+     *
+     * @since 2.9
+     */
+    protected Object _getSourceReference() {
+        if (JsonParser.Feature.INCLUDE_SOURCE_IN_LOCATION.enabledIn(_features)) {
+            return _ioContext.getSourceReference();
+        }
+        return null;
+    }
+    
     /*
     /**********************************************************
     /* Stuff that was abstract and required before 2.8, but that

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

@@ -127,10 +127,8 @@ protected ParserMinimalBase() { }
 
     @Override
     public JsonToken nextValue() throws IOException {
-        /* Implementation should be as trivial as follows; only
-         * needs to change if we are to skip other tokens (for
-         * example, if comments were exposed as tokens)
-         */
+        // Implementation should be as trivial as follows; only needs to change if
+        // we are to skip other tokens (for example, if comments were exposed as tokens)
         JsonToken t = nextToken();
         if (t == JsonToken.FIELD_NAME) {
             t = nextToken();
@@ -147,9 +145,8 @@ public JsonParser skipChildren() throws IOException
         }
         int open = 1;
 
-        /* Since proper matching of start/end markers is handled
-         * by nextToken(), we'll just count nesting levels here
-         */
+        // Since proper matching of start/end markers is handled
+        // by nextToken(), we'll just count nesting levels here
         while (true) {
             JsonToken t = nextToken();
             if (t == null) {