Oh I now get it. It's not fallbackBuffer.length but contentLength.
So this was a case where the extend was still not enough.Agree with the
fix.LieGrue,strub
On Tuesday, 20 February 2018, 19:22:53 CET, Mark Struberg
<[email protected]> wrote:
This looks wrong
- final char[] newArray = new char[fallBackCopyBuffer.length +
getBufferExtends(fallBackCopyBuffer.length)];
+ final char[] newArray = new char[fallBackCopyBuffer.length +
Math.max(getBufferExtends(fallBackCopyBuffer.length), length)];
that means with the new logic we always at least _double_ the old size! e.g.
10m + max(2.5m, 10m)
That's just wrong. That kind of screws anything which wants to extend less than
the old size.
LieGrue,strub
On Tuesday, 20 February 2018, 18:43:48 CET, Romain Manni-Bucau
<[email protected]> wrote:
Hi guys,
reviwes welcomed since it touches a sensitive area
long story short: the resizing was using size/4 but we can have read way
more data so adjusting it more if better
also the isClose was skipping buffer reuse it seems
Romain
---------- Forwarded message ----------
From: <[email protected]>
Date: 2018-02-20 18:42 GMT+01:00
Subject: johnzon git commit: JOHNZON-158 ensure we resize correctly the
buffer even when sizes of the internal buffers are not in the expected area
+ ensure we call close correctly in terms of scope to avoid surprises and
leaking buffers
To: [email protected]
Repository: johnzon
Updated Branches:
refs/heads/master 57f307cc6 -> d579e1381
JOHNZON-158 ensure we resize correctly the buffer even when sizes of the
internal buffers are not in the expected area + ensure we call close
correctly in terms of scope to avoid surprises and leaking buffers
Project: http://git-wip-us.apache.org/repos/asf/johnzon/repo
Commit: http://git-wip-us.apache.org/repos/asf/johnzon/commit/d579e138
Tree: http://git-wip-us.apache.org/repos/asf/johnzon/tree/d579e138
Diff: http://git-wip-us.apache.org/repos/asf/johnzon/diff/d579e138
Branch: refs/heads/master
Commit: d579e1381cf1842f08f35f8bf216c349dc123771
Parents: 57f307c
Author: Romain Manni-Bucau <[email protected]>
Authored: Tue Feb 20 18:42:05 2018 +0100
Committer: Romain Manni-Bucau <[email protected]>
Committed: Tue Feb 20 18:42:05 2018 +0100
----------------------------------------------------------------------
.../johnzon/core/JsonStreamParserImpl.java | 4 +-
.../jsonb/DynamicBufferResizingTest.java | 93 ++++++++++++++++++++
.../java/org/apache/johnzon/mapper/Mapper.java | 85 ++++++++----------
.../johnzon/mapper/MappingGeneratorImpl.java | 7 +-
.../johnzon/mapper/MappingParserImpl.java | 11 ++-
.../apache/johnzon/core/TestBufferProvider.java | 2 +-
6 files changed, 143 insertions(+), 59 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/johnzon/blob/
d579e138/johnzon-core/src/main/java/org/apache/johnzon/
core/JsonStreamParserImpl.java
----------------------------------------------------------------------
diff --git
a/johnzon-core/src/main/java/org/apache/johnzon/core/JsonStreamParserImpl.java
b/johnzon-core/src/main/java/org/apache/johnzon/core/
JsonStreamParserImpl.java
index 44c4ee9..4d0571a 100644
--- a/johnzon-core/src/main/java/org/apache/johnzon/core/
JsonStreamParserImpl.java
+++ b/johnzon-core/src/main/java/org/apache/johnzon/core/
JsonStreamParserImpl.java
@@ -170,7 +170,7 @@ public class JsonStreamParserImpl extends
JohnzonJsonParserImpl implements JsonC
throw new ArrayIndexOutOfBoundsException("Buffer too
small for such a long string");
}
- final char[] newArray = new char[fallBackCopyBuffer.length
+ getBufferExtends(fallBackCopyBuffer.length)];
+ final char[] newArray = new char[fallBackCopyBuffer.length
+ Math.max(getBufferExtends(fallBackCopyBuffer.length), length)];
// TODO: log to adjust size once?
System.arraycopy(fallBackCopyBuffer, 0, newArray, 0,
fallBackCopyBufferLength);
System.arraycopy(buffer, startOfValueInBuffer, newArray,
fallBackCopyBufferLength, length);
@@ -192,7 +192,7 @@ public class JsonStreamParserImpl extends
JohnzonJsonParserImpl implements JsonC
* @return the amount of bytes the current buffer should get extended
with
*/
protected int getBufferExtends(int currentLength) {
- return currentLength/4;
+ return currentLength / 4;
}
http://git-wip-us.apache.org/repos/asf/johnzon/blob/
d579e138/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/
DynamicBufferResizingTest.java
----------------------------------------------------------------------
diff --git
a/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/DynamicBufferResizingTest.java
b/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/
DynamicBufferResizingTest.java
new file mode 100644
index 0000000..6345a9b
--- /dev/null
+++ b/johnzon-jsonb/src/test/java/org/apache/johnzon/jsonb/
DynamicBufferResizingTest.java
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.johnzon.jsonb;
+
+import static org.junit.Assert.assertEquals;
+
+import javax.json.bind.Jsonb;
+import javax.json.bind.JsonbBuilder;
+import javax.json.bind.JsonbConfig;
+import javax.json.bind.annotation.JsonbCreator;
+import javax.json.bind.annotation.JsonbProperty;
+import javax.json.bind.annotation.JsonbPropertyOrder;
+import javax.json.bind.config.BinaryDataStrategy;
+
+import org.junit.Test;
+
+public class DynamicBufferResizingTest {
+ @Test
+ public void main() {
+ final JsonbConfig config = new JsonbConfig()
+ .withFormatting(Boolean.TRUE)
+ .withBinaryDataStrategy(BinaryDataStrategy.BASE_64);
+ Jsonb jsonb = JsonbBuilder.create(config);
+
+ final Request request = new Request("Screenshot.png", "image/png",
new byte[558140]);
+ String json = jsonb.toJson(request);
+
+ // the first call works
+ for (int i = 0; i < 10; i++) { // originally the second call was
failling
+ final Request fromJson = jsonb.fromJson(json, Request.class);
+ assertEquals("Screenshot.png", fromJson.name);
+ assertEquals("image/png", fromJson.mimeType);
+ assertEquals(558140, fromJson.body.length);
+ }
+ }
+
+ @JsonbPropertyOrder(value = {"name", "mimeType"})
+ public static class Request {
+ private String name;
+ private String mimeType;
+ private byte[] body;
+
+ @JsonbCreator
+ public Request(
+ final @JsonbProperty("name") String name,
+ final @JsonbProperty("mimeType") String mimeType,
+ final @JsonbProperty("body") byte[] body) {
+ this.name = name;
+ this.mimeType = mimeType;
+ this.body = body;
+ }
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
+
+ public String getMimeType() {
+ return mimeType;
+ }
+
+ public void setMimeType(String mimeType) {
+ this.mimeType = mimeType;
+ }
+
+ public byte[] getBody() {
+ return body;
+ }
+
+ public void setBody(byte[] body) {
+ this.body = body;
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/johnzon/blob/
d579e138/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/Mapper.java
----------------------------------------------------------------------
diff --git a/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/Mapper.java
b/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/Mapper.java
index 4bcc775..da28ae7 100644
--- a/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/Mapper.java
+++ b/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/Mapper.java
@@ -21,7 +21,6 @@ package org.apache.johnzon.mapper;
import org.apache.johnzon.mapper.internal.JsonPointerTracker;
import org.apache.johnzon.mapper.reflection.JohnzonCollectionType;
-import javax.json.JsonException;
import javax.json.JsonReader;
import javax.json.JsonReaderFactory;
import javax.json.JsonValue;
@@ -86,9 +85,10 @@ public class Mapper implements Closeable {
}
public <T> void writeArray(final Collection<T> object, final Writer
stream) {
- JsonGenerator generator = generatorFactory.
createGenerator(stream(stream));
- boolean dedup = Boolean.TRUE.equals(config.isDeduplicateObjects());
- writeObject(object, generator, null, dedup ? new
JsonPointerTracker(null, "/") : null);
+ try (final JsonGenerator generator = generatorFactory.
createGenerator(stream(stream))) {
+ boolean dedup = Boolean.TRUE.equals(config.
isDeduplicateObjects());
+ writeObject(object, generator, null, dedup ? new
JsonPointerTracker(null, "/") : null);
+ }
}
public <T> void writeIterable(final Iterable<T> object, final
OutputStream stream) {
@@ -96,9 +96,10 @@ public class Mapper implements Closeable {
}
public <T> void writeIterable(final Iterable<T> object, final Writer
stream) {
- JsonGenerator generator = generatorFactory.
createGenerator(stream(stream));
- boolean dedup = Boolean.TRUE.equals(config.isDeduplicateObjects());
- writeObject(object, generator, null, dedup ? new
JsonPointerTracker(null, "/") : null);
+ try (final JsonGenerator generator = generatorFactory.
createGenerator(stream(stream))) {
+ boolean dedup = Boolean.TRUE.equals(config.
isDeduplicateObjects());
+ writeObject(object, generator, null, dedup ? new
JsonPointerTracker(null, "/") : null);
+ }
}
public void writeObject(final Object object, final Writer stream) {
@@ -128,9 +129,10 @@ public class Mapper implements Closeable {
return;
}
- final JsonGenerator generator = generatorFactory.
createGenerator(stream(stream));
-
- writeObject(object, generator, null,
isDeduplicateObjects(object.getClass())
? new JsonPointerTracker(null, "/") : null);
+ try (final JsonGenerator generator = generatorFactory.
createGenerator(stream(stream))) {
+ writeObject(object, generator, null,
+ isDeduplicateObjects(object.getClass()) ? new
JsonPointerTracker(null, "/") : null);
+ }
}
private boolean isDeduplicateObjects(Class<?> rootType) {
@@ -156,29 +158,7 @@ public class Mapper implements Closeable {
private void writeObject(final Object object, final JsonGenerator
generator, final Collection<String> ignored, JsonPointerTracker
jsonPointer) {
MappingGeneratorImpl mappingGenerator = new
MappingGeneratorImpl(config, generator, mappings, jsonPointer != null);
-
- Throwable originalException = null;
- try {
- mappingGenerator.doWriteObject(object, generator, true,
ignored, jsonPointer);
- } catch (final Error | RuntimeException e) {
- originalException = e;
- } finally {
-
- try {
- generator.close();
- } catch (JsonException e) {
-
- if (originalException != null) {
- if (RuntimeException.class.isInstance(originalException))
{
- throw RuntimeException.class.cast(
originalException);
- }
- throw Error.class.cast(originalException); //
stackoverflow falls here
- } else {
- throw e;
- }
- }
- }
-
+ mappingGenerator.doWriteObject(object, generator, true, ignored,
jsonPointer);
}
public String writeArrayAsString(final Collection<?> instance) {
@@ -205,15 +185,22 @@ public class Mapper implements Closeable {
}
public <T> T readObject(final Reader stream, final Type clazz) {
- return mapObject(clazz, readerFactory.createReader(
stream(stream)));
+ try (final JsonReader reader =
readerFactory.createReader(stream(stream)))
{
+ return mapObject(clazz, reader);
+ }
}
public <T> T readObject(final InputStream stream, final Type clazz) {
- return mapObject(clazz, charset == null ?
readerFactory.createReader(stream(stream)):
readerFactory.createReader(stream(stream),
charset));
+ try (final JsonReader reader = charset == null ?
readerFactory.createReader(stream(stream)) : readerFactory.createReader(
+ stream(stream), charset)) {
+ return mapObject(clazz, reader);
+ }
}
public <T> Collection<T> readCollection(final InputStream stream,
final ParameterizedType genericType) {
- return mapObject(genericType, charset == null ?
readerFactory.createReader(stream(stream)):
readerFactory.createReader(stream(stream),
charset));
+ try (final JsonReader reader = charset == null ?
readerFactory.createReader(stream(stream)):
readerFactory.createReader(stream(stream),
charset)) {
+ return mapObject(genericType, reader);
+ }
}
public <T> T readJohnzonCollection(final InputStream stream, final
JohnzonCollectionType<T> genericType) {
@@ -225,27 +212,33 @@ public class Mapper implements Closeable {
}
public <T> Collection<T> readCollection(final Reader stream, final
ParameterizedType genericType) {
- return mapObject(genericType, readerFactory.createReader(
stream(stream)));
+ try (final JsonReader reader =
readerFactory.createReader(stream(stream)))
{
+ return mapObject(genericType, reader);
+ }
}
public <T> T[] readArray(final Reader stream, final Class<T> clazz) {
- final JsonReader reader = readerFactory.createReader(
stream(stream));
- return (T[]) mapArray(clazz, reader);
+ try (final JsonReader reader =
readerFactory.createReader(stream(stream)))
{
+ return (T[]) mapArray(clazz, reader);
+ }
}
public <T> T readTypedArray(final InputStream stream, final Class<?>
elementType, final Class<T> arrayType) {
- final JsonReader reader = charset == null ?
readerFactory.createReader(stream(stream)):
readerFactory.createReader(stream(stream),
charset);
- return arrayType.cast(mapArray(elementType, reader));
+ try (final JsonReader reader = charset == null ?
readerFactory.createReader(stream(stream)):
readerFactory.createReader(stream(stream),
charset)) {
+ return arrayType.cast(mapArray(elementType, reader));
+ }
}
public <T> T readTypedArray(final Reader stream, final Class<?>
elementType, final Class<T> arrayType) {
- final JsonReader reader = readerFactory.createReader(
stream(stream));
- return arrayType.cast(mapArray(elementType, reader));
+ try (final JsonReader reader =
readerFactory.createReader(stream(stream)))
{
+ return arrayType.cast(mapArray(elementType, reader));
+ }
}
public <T> T[] readArray(final InputStream stream, final Class<T>
clazz) {
- final JsonReader reader = charset == null ?
readerFactory.createReader(stream(stream)):
readerFactory.createReader(stream(stream),
charset);
- return (T[]) mapArray(clazz, reader);
+ try (final JsonReader reader = charset == null ?
readerFactory.createReader(stream(stream)):
readerFactory.createReader(stream(stream),
charset)) {
+ return (T[]) mapArray(clazz, reader);
+ }
}
private Object mapArray(final Class<?> clazz, final JsonReader reader)
{
@@ -282,7 +275,7 @@ public class Mapper implements Closeable {
c.close();
} catch (final IOException e) {
if (errors == null) {
- errors = new ArrayList<Exception>();
+ errors = new ArrayList<>();
}
errors.add(e);
}
http://git-wip-us.apache.org/repos/asf/johnzon/blob/
d579e138/johnzon-mapper/src/main/java/org/apache/johnzon/
mapper/MappingGeneratorImpl.java
----------------------------------------------------------------------
diff --git a/johnzon-mapper/src/main/java/org/apache/johnzon/
mapper/MappingGeneratorImpl.java b/johnzon-mapper/src/main/
java/org/apache/johnzon/mapper/MappingGeneratorImpl.java
index 773e615..3fa054a 100644
--- a/johnzon-mapper/src/main/java/org/apache/johnzon/
mapper/MappingGeneratorImpl.java
+++ b/johnzon-mapper/src/main/java/org/apache/johnzon/
mapper/MappingGeneratorImpl.java
@@ -18,16 +18,15 @@
*/
package org.apache.johnzon.mapper;
-import org.apache.johnzon.mapper.internal.AdapterKey;
import org.apache.johnzon.mapper.internal.JsonPointerTracker;
import javax.json.JsonValue;
import javax.json.stream.JsonGenerator;
-import javax.xml.bind.DatatypeConverter;
import java.lang.reflect.Array;
import java.lang.reflect.InvocationTargetException;
import java.math.BigDecimal;
import java.math.BigInteger;
+import java.util.Base64;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
@@ -329,12 +328,12 @@ public class MappingGeneratorImpl implements
MappingGenerator {
}
if(config.isTreatByteArrayAsBase64() && (type == byte[].class
/*|| type == Byte[].class*/)) {
- String base64EncodedByteArray =
DatatypeConverter.printBase64Binary((byte[])
value);
+ String base64EncodedByteArray =
Base64.getEncoder().encodeToString((byte[])
value);
generator.write(key, base64EncodedByteArray);
return;
}
if(config.isTreatByteArrayAsBase64URL() && (type ==
byte[].class /*|| type == Byte[].class*/)) {
- generator.write(key, String.valueOf(Adapter.class.
cast(config.getAdapters().get(new AdapterKey(byte[].class,
String.class))).to(value)));
+ generator.write(key,
Base64.getUrlEncoder().encodeToString((byte[])
value));
return;
}
http://git-wip-us.apache.org/repos/asf/johnzon/blob/
d579e138/johnzon-mapper/src/main/java/org/apache/johnzon/
mapper/MappingParserImpl.java
----------------------------------------------------------------------
diff --git
a/johnzon-mapper/src/main/java/org/apache/johnzon/mapper/MappingParserImpl.java
b/johnzon-mapper/src/main/java/org/apache/johnzon/
mapper/MappingParserImpl.java
index 18af2ec..8e74e3c 100644
--- a/johnzon-mapper/src/main/java/org/apache/johnzon/
mapper/MappingParserImpl.java
+++ b/johnzon-mapper/src/main/java/org/apache/johnzon/
mapper/MappingParserImpl.java
@@ -33,7 +33,6 @@ import javax.json.JsonReader;
import javax.json.JsonString;
import javax.json.JsonStructure;
import javax.json.JsonValue;
-import javax.xml.bind.DatatypeConverter;
import java.lang.reflect.Array;
import java.lang.reflect.InvocationTargetException;
@@ -45,6 +44,7 @@ import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.ArrayDeque;
import java.util.ArrayList;
+import java.util.Base64;
import java.util.Collection;
import java.util.Collections;
import java.util.Deque;
@@ -125,10 +125,6 @@ public class MappingParserImpl implements
MappingParser {
return readObject(jsonReader.readValue(), targetType);
} catch (final NoSuchMethodError noSuchMethodError) { // jsonp 1.0
fallback - mainly for tests
return readObject(jsonReader.read(), targetType);
- } finally {
- if (config.isClose()) {
- jsonReader.close();
- }
}
}
@@ -482,7 +478,10 @@ public class MappingParserImpl implements
MappingParser {
}
if (config.isTreatByteArrayAsBase64() && jsonValue.getValueType()
== JsonValue.ValueType.STRING && (type == byte[].class /*|| type ==
Byte[].class*/)) {
- return DatatypeConverter.parseBase64Binary(((JsonString)
jsonValue).getString());
+ return Base64.getDecoder().decode(((JsonString)
jsonValue).getString());
+ }
+ if (config.isTreatByteArrayAsBase64URL() &&
jsonValue.getValueType() == JsonValue.ValueType.STRING && (type ==
byte[].class /*|| type == Byte[].class*/)) {
+ return Base64.getUrlDecoder().decode(((JsonString)
jsonValue).getString());
}
if (Object.class == type) { // handling specific types here to
keep exception in standard handling
http://git-wip-us.apache.org/repos/asf/johnzon/blob/
d579e138/johnzon-mapper/src/test/java/org/apache/johnzon/
core/TestBufferProvider.java
----------------------------------------------------------------------
diff --git
a/johnzon-mapper/src/test/java/org/apache/johnzon/core/TestBufferProvider.java
b/johnzon-mapper/src/test/java/org/apache/johnzon/core/
TestBufferProvider.java
index 532d32c..93733d9 100644
--- a/johnzon-mapper/src/test/java/org/apache/johnzon/core/
TestBufferProvider.java
+++ b/johnzon-mapper/src/test/java/org/apache/johnzon/core/
TestBufferProvider.java
@@ -69,7 +69,7 @@ public class TestBufferProvider implements
BufferStrategy.BufferProvider<char[]>
}
public int newBufferCalls() {
- return releaseCalls.get();
+ return newBufferCalls.get();
}
public void clear() {
