Yep Needed 2 checks too since we worked on it heavily but the changes in defaults changed a bit the cases we can have I guess.
Le 20 févr. 2018 19:40, "Mark Struberg" <[email protected]> a écrit : > 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() { >
