diff --git a/java/src/org/openqa/selenium/grid/graphql/GraphqlHandler.java b/java/src/org/openqa/selenium/grid/graphql/GraphqlHandler.java index 3cef83ef69faf..5f66163772f33 100644 --- a/java/src/org/openqa/selenium/grid/graphql/GraphqlHandler.java +++ b/java/src/org/openqa/selenium/grid/graphql/GraphqlHandler.java @@ -53,6 +53,7 @@ import org.openqa.selenium.grid.sessionqueue.NewSessionQueue; import org.openqa.selenium.internal.Require; import org.openqa.selenium.json.Json; +import org.openqa.selenium.json.JsonOutput; import org.openqa.selenium.remote.http.Contents; import org.openqa.selenium.remote.http.HttpHandler; import org.openqa.selenium.remote.http.HttpRequest; @@ -179,7 +180,7 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException { response = new HttpResponse() .addHeader("Content-Type", JSON_UTF_8) - .setContent(utf8String(JSON.toJson(result.toSpecification()))); + .setContent(utf8String(toCompactJson(result.toSpecification()))); HTTP_RESPONSE.accept(span, response); HTTP_RESPONSE_EVENT.accept(attributeMap, response); @@ -191,7 +192,7 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException { response = new HttpResponse() .setStatus(HTTP_INTERNAL_ERROR) - .setContent(utf8String(JSON.toJson(result.getErrors()))); + .setContent(utf8String(toCompactJson(result.getErrors()))); HTTP_RESPONSE.accept(span, response); HTTP_RESPONSE_EVENT.accept(attributeMap, response); @@ -271,4 +272,12 @@ public int weigh(String key, CompletableFuture value) { return (int) Math.min(boundedWeight, Integer.MAX_VALUE); } } + + private static String toCompactJson(Object value) { + StringBuilder json = new StringBuilder(); + try (JsonOutput out = JSON.newOutput(json)) { + out.setPrettyPrint(false).write(value); + } + return json.toString(); + } } diff --git a/java/src/org/openqa/selenium/json/CollectionCoercer.java b/java/src/org/openqa/selenium/json/CollectionCoercer.java index 0101083931fba..fe16a5c4b2635 100644 --- a/java/src/org/openqa/selenium/json/CollectionCoercer.java +++ b/java/src/org/openqa/selenium/json/CollectionCoercer.java @@ -62,12 +62,15 @@ public BiFunction apply(Type type) { throw new IllegalArgumentException("Unhandled type: " + type.getClass()); } + // Resolve the element coercer once rather than paying a cache lookup per element. + BiFunction valueCoercer = coercer.lazyResolve(valueType); + return (jsonInput, setting) -> { jsonInput.beginArray(); I toReturn = supplier.get(); Consumer consumer = consumerFactory.apply(toReturn); while (jsonInput.hasNext()) { - consumer.accept(coercer.coerce(jsonInput, valueType, setting)); + consumer.accept(valueCoercer.apply(jsonInput, setting)); } jsonInput.endArray(); diff --git a/java/src/org/openqa/selenium/json/ConstructorCoercer.java b/java/src/org/openqa/selenium/json/ConstructorCoercer.java index b892660509ca6..ac7eaafacaad9 100644 --- a/java/src/org/openqa/selenium/json/ConstructorCoercer.java +++ b/java/src/org/openqa/selenium/json/ConstructorCoercer.java @@ -148,6 +148,13 @@ private Map getParameterIndexes(Parameter[] parameters) { } private Object coerceValue(Object value, Type type, PropertySetting setting) { + if (value != null && type instanceof Class) { + Object direct = tryDirectCoercion(value, (Class) type); + if (direct != null) { + return direct; + } + } + StringWriter rawJson = new StringWriter(); try (JsonOutput output = new JsonOutput(rawJson)) { output.write(value); @@ -158,6 +165,70 @@ private Object coerceValue(Object value, Type type, PropertySetting setting) { } } + /** + * Coerce the already-parsed value directly when the target is a scalar type, mirroring the + * behavior of the corresponding {@link TypeCoercer}s without serializing the value back to JSON + * text and re-parsing it. + * + * @return the coerced value; {@code null} if the caller must fall back to the JSON round trip + */ + private static Object tryDirectCoercion(Object value, Class target) { + if (target == String.class) { + return (value instanceof String || value instanceof Number || value instanceof Boolean) + ? String.valueOf(value) + : null; + } + + if ((target == Boolean.class || target == boolean.class) && value instanceof Boolean) { + return value; + } + + if (value instanceof Number) { + Number number = (Number) value; + if (target == Integer.class || target == int.class) { + return number.intValue(); + } + if (target == Long.class || target == long.class) { + return number.longValue(); + } + if (target == Double.class || target == double.class) { + return number.doubleValue(); + } + if (target == Float.class || target == float.class) { + return number.floatValue(); + } + if (target == Short.class || target == short.class) { + return number.shortValue(); + } + if (target == Byte.class || target == byte.class) { + return number.byteValue(); + } + if (target == Number.class) { + if (number instanceof Long) { + return number; + } + double doubleValue = number.doubleValue(); + if (doubleValue % 1 != 0 || doubleValue < Long.MIN_VALUE || doubleValue > Long.MAX_VALUE) { + return doubleValue; + } + return number.longValue(); + } + return null; + } + + if (target.isEnum() && value instanceof String) { + for (Object constant : target.getEnumConstants()) { + if (constant.toString().equalsIgnoreCase((String) value)) { + return constant; + } + } + throw new JsonException( + String.format("Unable to find matching enum value for %s in %s", value, target)); + } + + return null; + } + private class ConstructorCandidate { private final Constructor constructor; private final Parameter[] parameters; diff --git a/java/src/org/openqa/selenium/json/Input.java b/java/src/org/openqa/selenium/json/Input.java index 9a282811ca8ae..0ca78a0f00589 100644 --- a/java/src/org/openqa/selenium/json/Input.java +++ b/java/src/org/openqa/selenium/json/Input.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.io.Reader; import java.io.UncheckedIOException; +import org.jspecify.annotations.Nullable; import org.openqa.selenium.internal.Require; /** @@ -38,12 +39,12 @@ class Input { public static final int EOF = -1; /** the number of chars to buffer */ - private static final int BUFFER_SIZE = 4096; + private static final int BUFFER_SIZE = 16384; /** the number of chars to remember, safe to set to 0 */ private static final int MEMORY_SIZE = 128; - private final Reader source; + private final @Nullable Reader source; /** a buffer used to minimize read calls and to keep the chars to remember */ private final char[] buffer; @@ -66,6 +67,23 @@ public Input(Reader source) { this.position = -1; } + /** + * Initialize a new instance of the {@link Input} class that reads directly from a string. + * + *

The entire input is buffered up front, sized exactly to the source. This avoids the large + * read buffer and per-chunk {@link Reader} calls of the streaming constructor, which dominate the + * cost of parsing the small payloads typical of WebDriver commands and responses. + * + * @param source string that supplies the input to be processed + */ + public Input(String source) { + Require.nonNull("Source", source); + this.source = null; + this.buffer = source.toCharArray(); + this.filled = buffer.length; + this.position = -1; + } + /** * Extract the next character from the input without consuming it. * @@ -73,6 +91,10 @@ public Input(Reader source) { * input is exhausted */ public int peek() { + int next = position + 1; + if (next < filled) { + return buffer[next]; + } return fill() ? buffer[position + 1] : EOF; } @@ -83,9 +105,125 @@ public int peek() { * input is exhausted */ public int read() { + int next = position + 1; + if (next < filled) { + position = next; + return buffer[next]; + } return fill() ? buffer[++position] : EOF; } + /** + * Attempt to consume the body of a JSON string (with the leading quote already consumed) directly + * from the buffered input, including its closing quote. + * + *

This is the fast path for strings that contain no escape sequences and whose closing quote + * is already buffered: the result is created straight from the buffer without copying through a + * {@link StringBuilder}. When the string cannot be read this way, nothing is consumed. + * + * @return the string body; {@code null} if the caller must fall back to reading char by char + */ + public @Nullable String readSimpleString() { + if (!fill()) { + return null; + } + + int start = position + 1; + for (int i = start; i < filled; i++) { + char c = buffer[i]; + if (c == '"') { + position = i; + return new String(buffer, start, i - start); + } + if (c == '\\' || c < 0x20) { + return null; + } + } + + return null; + } + + /** + * Consume characters that need no special handling within a JSON string, appending them to the + * supplied builder in bulk. Stops before the next '"', '\\', or control character, which is left + * unconsumed. + * + * @param sink {@link StringBuilder} that accumulates the string body + * @return the unconsumed special character as an unsigned UTF-16 code unit; {@link #EOF} if input + * is exhausted + */ + public int appendStringContent(StringBuilder sink) { + while (fill()) { + int start = position + 1; + for (int i = start; i < filled; i++) { + char c = buffer[i]; + if (c == '"' || c == '\\' || c < 0x20) { + sink.append(buffer, start, i - start); + position = i - 1; + return c; + } + } + sink.append(buffer, start, filled - start); + position = filled - 1; + } + + return EOF; + } + + /** + * Consume whitespace characters in bulk, leaving the first non-whitespace character unconsumed. + */ + public void skipWhitespace() { + while (fill()) { + int start = position + 1; + for (int i = start; i < filled; i++) { + if (!isWhitespace(buffer[i])) { + position = i - 1; + return; + } + } + position = filled - 1; + } + } + + /** + * Test for whitespace, checking the JSON whitespace characters (RFC 8259 §2) directly before + * consulting {@link Character#isWhitespace}, whose table lookups are measurably slower and which + * is retained only to stay lenient about exotic whitespace between tokens. + */ + private static boolean isWhitespace(char c) { + if (c == ' ' || c == '\n' || c == '\t' || c == '\r') { + return true; + } + return Character.isWhitespace(c); + } + + /** + * Consume ASCII digits, appending them to the supplied builder in bulk. Stops before the first + * non-digit character, which is left unconsumed. + * + * @param sink {@link StringBuilder} that accumulates the digits + * @return the unconsumed non-digit character as an unsigned UTF-16 code unit; {@link #EOF} if + * input is exhausted + */ + public int appendDigits(StringBuilder sink) { + while (fill()) { + int start = position + 1; + for (int i = start; i < filled; i++) { + char c = buffer[i]; + if (c < '0' || c > '9') { + sink.append(buffer, start, i - start); + position = i - 1; + return c; + } + } + sink.append(buffer, start, filled - start); + position = filled - 1; + } + + return EOF; + } + /** * Return a string containing the most recently consumed input characters. * @@ -129,6 +267,11 @@ public String toString() { * @throws UncheckedIOException if an I/O exception is encountered */ private boolean fill() { + if (source == null) { + // String-backed input is fully buffered on construction. + return filled > position + 1; + } + // do we need to fill the buffer? while (filled == position + 1) { try { diff --git a/java/src/org/openqa/selenium/json/InstanceCoercer.java b/java/src/org/openqa/selenium/json/InstanceCoercer.java index 04c18aa6e4563..67ae9454881a4 100644 --- a/java/src/org/openqa/selenium/json/InstanceCoercer.java +++ b/java/src/org/openqa/selenium/json/InstanceCoercer.java @@ -19,6 +19,11 @@ import static java.util.stream.Collectors.toMap; +import java.lang.invoke.CallSite; +import java.lang.invoke.LambdaMetafactory; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Method; @@ -28,10 +33,12 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.BiConsumer; import java.util.function.BiFunction; import java.util.function.Function; import java.util.stream.Stream; +import org.jspecify.annotations.Nullable; import org.openqa.selenium.internal.Require; class InstanceCoercer extends TypeCoercer { @@ -55,24 +62,16 @@ public boolean test(Class aClass) { @Override public BiFunction apply(Type type) { Constructor constructor = getConstructor(type); + // The writers depend only on the type and the property-setting strategy, so compute them + // once per strategy rather than reflecting over the type for every instance created. + Map> writersBySetting = new ConcurrentHashMap<>(); return (jsonInput, setter) -> { try { Object instance = constructor.newInstance(); - Map allWriters; - switch (setter) { - case BY_FIELD: - allWriters = getFieldWriters(constructor); - break; - - case BY_NAME: - allWriters = getBeanWriters(constructor); - break; - - default: - throw new JsonException("Cannot determine how to find fields: " + setter); - } + Map allWriters = + writersBySetting.computeIfAbsent(setter, key -> getWriters(constructor, key)); jsonInput.beginObject(); @@ -85,7 +84,7 @@ public BiFunction apply(Type type) { continue; } - Object value = coercer.coerce(jsonInput, writer.type, setter); + Object value = writer.coercion.apply(jsonInput, setter); writer.writer.accept(instance, value); } @@ -98,6 +97,27 @@ public BiFunction apply(Type type) { }; } + private Map getWriters( + Constructor constructor, PropertySetting setter) { + Map writers; + switch (setter) { + case BY_FIELD: + writers = getFieldWriters(constructor); + break; + + case BY_NAME: + writers = getBeanWriters(constructor); + break; + + default: + throw new JsonException("Cannot determine how to find fields: " + setter); + } + + // Resolve the property coercers once per type rather than per value read. + writers.values().forEach(writer -> writer.coercion = coercer.lazyResolve(writer.type)); + return writers; + } + private Map getFieldWriters(Constructor constructor) { List fields = new LinkedList<>(); for (Class current = constructor.getDeclaringClass(); @@ -169,6 +189,7 @@ private static Class getClss(Type type) { private static class TypeAndWriter { private final Type type; private final BiConsumer writer; + private BiFunction coercion; TypeAndWriter(Type type, BiConsumer writer) { this.type = type; @@ -229,18 +250,52 @@ public TypeAndWriter apply(SimplePropertyDescriptor desc) { private static class SimplePropertyWriter implements BiConsumer { private final SimplePropertyDescriptor desc; private final Method method; + private final @Nullable BiConsumer compiled; SimplePropertyWriter(SimplePropertyDescriptor desc, Method method) { this.desc = desc; this.method = method; + this.compiled = compileSetter(method); + } + + /** + * Compile the setter to a direct call via {@link LambdaMetafactory}, which is considerably + * faster than reflective invocation. Returns {@code null} for anything that cannot be compiled + * (inaccessible types and the like), in which case {@link Method#invoke} is used, deferring + * accessibility problems to invocation time just as reflective calls always have. + */ + private static @Nullable BiConsumer compileSetter(Method method) { + try { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + MethodHandle handle = lookup.unreflect(method); + CallSite site = + LambdaMetafactory.metafactory( + lookup, + "accept", + MethodType.methodType(BiConsumer.class), + MethodType.methodType(void.class, Object.class, Object.class), + handle, + handle.type().changeReturnType(void.class)); + @SuppressWarnings("unchecked") + BiConsumer setter = + (BiConsumer) site.getTarget().invokeExact(); + return setter; + } catch (Throwable t) { + return null; + } } @Override public void accept(Object instance, Object value) { - method.setAccessible(true); try { + if (compiled != null) { + compiled.accept(instance, value); + return; + } + + method.setAccessible(true); method.invoke(instance, value); - } catch (ReflectiveOperationException e) { + } catch (Exception e) { throw new JsonException( String.format( "Cannot call method %s.%s(%s)", diff --git a/java/src/org/openqa/selenium/json/Json.java b/java/src/org/openqa/selenium/json/Json.java index dc392a387491a..180fe8e50c385 100644 --- a/java/src/org/openqa/selenium/json/Json.java +++ b/java/src/org/openqa/selenium/json/Json.java @@ -17,12 +17,8 @@ package org.openqa.selenium.json; -import java.io.IOException; import java.io.Reader; -import java.io.StringReader; -import java.io.StringWriter; import java.io.UncheckedIOException; -import java.io.Writer; import java.lang.reflect.Type; import java.util.List; import java.util.Map; @@ -114,7 +110,14 @@ public class Json { /** Specifier for {@code Object} input/output type */ public static final Type OBJECT_TYPE = new TypeToken() {}.getType(); - private final JsonTypeCoercer fromJson = new JsonTypeCoercer(); + /** + * The default coercer is stateless apart from its cache of resolved coercion functions, so a + * single shared instance lets that cache accumulate across the many short-lived {@code Json} + * instances created throughout the codebase. + */ + private static final JsonTypeCoercer DEFAULT_COERCER = new JsonTypeCoercer(); + + private final JsonTypeCoercer fromJson = DEFAULT_COERCER; /** * Serialize the specified object to JSON string representation.
@@ -137,13 +140,12 @@ public String toJson(@Nullable Object toConvert) { * @throws JsonException if an I/O exception is encountered */ public String toJson(@Nullable Object toConvert, int maxDepth) { - try (Writer writer = new StringWriter(); - JsonOutput jsonOutput = newOutput(writer)) { + // A StringBuilder rather than a StringWriter: the latter synchronizes every append. + StringBuilder builder = new StringBuilder(); + try (JsonOutput jsonOutput = newOutput(builder)) { jsonOutput.write(toConvert, maxDepth); - return writer.toString(); - } catch (IOException e) { - throw new JsonException("Cannot convert " + toConvert + " to json", e); } + return builder.toString(); } /** @@ -172,13 +174,26 @@ public T toType(String source, Type typeOfT) { * @throws JsonException if an I/O exception is encountered */ public T toType(String source, Type typeOfT, PropertySetting setter) { - try (StringReader reader = new StringReader(source)) { - return toType(reader, typeOfT, setter); + Require.nonNull("Mechanism for setting properties", setter); + + // Read the string directly rather than through a StringReader: string-backed input skips the + // Reader indirection and the large read buffer, which dominate the cost of parsing the small + // payloads typical of WebDriver commands and responses. + try (JsonInput json = new JsonInput(source, fromJson, PropertySetting.BY_NAME)) { + return fromJson.coerce(json, typeOfT, setter); } catch (JsonException e) { - throw new JsonException("Unable to parse: " + source, e); + throw new JsonException("Unable to parse: " + abbreviate(source), e); } } + /** + * Limit how much of the source is echoed into an exception message; sources can be many + * megabytes, and the parse position is already reported by the underlying cause. + */ + private static String abbreviate(String text) { + return text.length() <= 512 ? text : text.substring(0, 512) + "..."; + } + /** * Deserialize the JSON string supplied by the specified {@code Reader} into an object of the * specified type.
diff --git a/java/src/org/openqa/selenium/json/JsonInput.java b/java/src/org/openqa/selenium/json/JsonInput.java index d4b23c9760b47..b98f0a9133697 100644 --- a/java/src/org/openqa/selenium/json/JsonInput.java +++ b/java/src/org/openqa/selenium/json/JsonInput.java @@ -24,13 +24,12 @@ import java.io.Reader; import java.io.UncheckedIOException; import java.lang.reflect.Type; -import java.math.BigDecimal; import java.time.Instant; -import java.util.ArrayDeque; import java.util.ArrayList; -import java.util.Deque; +import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.function.BiFunction; import java.util.function.Function; import org.jspecify.annotations.Nullable; import org.openqa.selenium.internal.Require; @@ -38,21 +37,28 @@ /** * The JsonInput class defines the operations used to deserialize JSON strings into Java * objects. + * + *

Instances of this class are not thread-safe: each instance wraps a single character stream and + * must be confined to one thread. */ public class JsonInput implements Closeable { - private final Reader source; + private final @Nullable Reader source; private boolean readPerformed = false; private JsonTypeCoercer coercer; private PropertySetting setter; private final Input input; - // Used when reading maps and collections so that we handle de-nesting and - // figuring out whether we're expecting a NAME properly. - private final Deque stack = new ArrayDeque<>(); + // Stack of open containers, used to handle de-nesting and to figure out + // whether we're expecting a NAME. Kept as plain arrays (rather than a deque + // of objects) because the top of the stack is touched for every value read. + private byte[] containerState = new byte[16]; // Parallel stack tracking whether the current container has seen at least // one element. Used by hasNext() to enforce comma separators between // elements while remaining lenient about a single trailing comma. - private final Deque containerHasElement = new ArrayDeque<>(); + private boolean[] containerHasElement = new boolean[16]; + private int containerDepth; + // Memoized type of the pending token; cleared whenever the token is consumed. + private @Nullable JsonType peekedType; JsonInput(Reader source, JsonTypeCoercer coercer, PropertySetting setter) { @@ -62,6 +68,14 @@ public class JsonInput implements Closeable { this.setter = Require.nonNull("Setter", setter); } + JsonInput(String source, JsonTypeCoercer coercer, PropertySetting setter) { + + this.source = null; + this.coercer = Require.nonNull("Coercer", coercer); + this.input = new Input(Require.nonNull("Source", source)); + this.setter = Require.nonNull("Setter", setter); + } + /** * Change how property setting is done. It's polite to set the value back once done processing. * @@ -93,14 +107,12 @@ public JsonInput addCoercers(TypeCoercer... coercers) { * @throws JsonException if this {@code JsonInput} has already begun processing its input */ public JsonInput addCoercers(Iterable> coercers) { - synchronized (this) { - if (readPerformed) { - throw new JsonException("JsonInput has already been used and may not be modified"); - } - - this.coercer = new JsonTypeCoercer(coercer, coercers); + if (readPerformed) { + throw new JsonException("JsonInput has already been used and may not be modified"); } + this.coercer = new JsonTypeCoercer(coercer, coercers); + return this; } @@ -111,6 +123,10 @@ public JsonInput addCoercers(Iterable> coercers) { */ @Override public void close() { + if (source == null) { + return; + } + try { source.close(); } catch (IOException e) { @@ -126,15 +142,24 @@ public void close() { * @throws UncheckedIOException if an I/O exception is encountered */ public JsonType peek() { + // A single token is typically peeked at several times on its way through the coercers, so + // the computed type is memoized until the token is consumed. + JsonType type = peekedType; + if (type != null) { + return type; + } + skipWhitespace(input); switch (input.peek()) { case 'f': case 't': - return JsonType.BOOLEAN; + type = JsonType.BOOLEAN; + break; case 'n': - return JsonType.NULL; + type = JsonType.NULL; + break; case '-': case '0': @@ -147,30 +172,40 @@ public JsonType peek() { case '7': case '8': case '9': - return JsonType.NUMBER; + type = JsonType.NUMBER; + break; case '"': - return isReadingName() ? JsonType.NAME : JsonType.STRING; + type = isReadingName() ? JsonType.NAME : JsonType.STRING; + break; case '{': - return JsonType.START_MAP; + type = JsonType.START_MAP; + break; case '}': - return JsonType.END_MAP; + type = JsonType.END_MAP; + break; case '[': - return JsonType.START_COLLECTION; + type = JsonType.START_COLLECTION; + break; case ']': - return JsonType.END_COLLECTION; + type = JsonType.END_COLLECTION; + break; case Input.EOF: - return JsonType.END; + type = JsonType.END; + break; default: int c = input.read(); throw new JsonException("Unable to determine type from: " + (char) c + ". " + input); } + + peekedType = type; + return type; } /** @@ -243,9 +278,7 @@ public Number nextNumber() { throw new JsonException("Leading zeros are not permitted in JSON numbers. " + input); } } else if (first >= '1' && first <= '9') { - while (isDigit(input.peek())) { - builder.append((char) input.read()); - } + input.appendDigits(builder); } else { throw new JsonException("Expected digit but saw " + describeChar(first) + ". " + input); } @@ -261,9 +294,7 @@ public Number nextNumber() { + ". " + input); } - while (isDigit(input.peek())) { - builder.append((char) input.read()); - } + input.appendDigits(builder); } // Optional exponent part: ('e' | 'E') ('+' | '-')? 1*DIGIT @@ -280,17 +311,21 @@ public Number nextNumber() { + ". " + input); } - while (isDigit(input.peek())) { - builder.append((char) input.read()); - } + input.appendDigits(builder); } try { // Fast path for integers: Long-valued when no fraction/exponent was present. if (!isDecimal) { + // At most 18 digits (plus a sign) always fits in a long, so the allocation-free parse + // cannot overflow. Longer inputs take the String path, which reports overflow with the + // historical NumberFormatException message. + if (builder.length() <= (builder.charAt(0) == '-' ? 19 : 18)) { + return Long.parseLong(builder, 0, builder.length(), 10); + } return Long.valueOf(builder.toString()); } - double value = new BigDecimal(builder.toString()).doubleValue(); + double value = parseDouble(builder); if (Double.isInfinite(value) || Double.isNaN(value)) { throw new JsonException("Number is out of range for a double: " + builder + ". " + input); } @@ -304,6 +339,96 @@ private static boolean isDigit(int c) { return c >= '0' && c <= '9'; } + /** Powers of ten that are exactly representable as doubles. */ + private static final double[] POW_10 = { + 1e0, 1e1, 1e2, 1e3, 1e4, 1e5, 1e6, 1e7, 1e8, 1e9, 1e10, 1e11, 1e12, 1e13, 1e14, 1e15, 1e16, + 1e17, 1e18, 1e19, 1e20, 1e21, 1e22 + }; + + /** + * Parse a JSON number that contains a fraction or exponent, as lexed into {@code raw} by {@link + * #nextNumber}. + * + *

Uses Clinger's fast path where possible: when the significand has at most 15 digits it is + * exactly representable as a double, as are powers of ten up to 10^22, so a single floating-point + * multiply or divide performs the one correctly-rounded step the conversion needs. Everything + * else (long significands, large exponents) falls back to {@link Double#parseDouble}, so results + * are always bit-for-bit identical to the JDK. + */ + private static double parseDouble(StringBuilder raw) { + int length = raw.length(); + int index = 0; + boolean negative = false; + if (raw.charAt(0) == '-') { + negative = true; + index = 1; + } + + long significand = 0; + int digits = 0; + int fractionDigits = 0; + + while (index < length) { + char c = raw.charAt(index); + if (c < '0' || c > '9') { + break; + } + significand = significand * 10 + (c - '0'); + digits++; + index++; + } + + if (index < length && raw.charAt(index) == '.') { + index++; + while (index < length) { + char c = raw.charAt(index); + if (c < '0' || c > '9') { + break; + } + significand = significand * 10 + (c - '0'); + digits++; + fractionDigits++; + index++; + } + } + + int exponent = 0; + if (index < length) { + // By construction the remainder is ('e' | 'E') ('+' | '-')? 1*DIGIT. + index++; + boolean exponentNegative = false; + char sign = raw.charAt(index); + if (sign == '+' || sign == '-') { + exponentNegative = sign == '-'; + index++; + } + while (index < length) { + // Clamp rather than overflow; anything this large falls back below anyway. + if (exponent < 100_000) { + exponent = exponent * 10 + (raw.charAt(index) - '0'); + } + index++; + } + if (exponentNegative) { + exponent = -exponent; + } + } + + int netExponent = exponent - fractionDigits; + + if (digits <= 15 && netExponent >= -22 && netExponent <= 22) { + double value = (double) significand; + if (netExponent > 0) { + value = value * POW_10[netExponent]; + } else if (netExponent < 0) { + value = value / POW_10[-netExponent]; + } + return negative ? -value : value; + } + + return Double.parseDouble(raw.toString()); + } + private static String describeChar(int c) { return c == Input.EOF ? "" : "'" + (char) c + "'"; } @@ -351,19 +476,20 @@ public void nextEnd() { * @throws UncheckedIOException if an I/O exception is encountered */ public boolean hasNext() { - if (stack.isEmpty()) { + if (containerDepth == 0) { throw new JsonException( "Unable to determine if an item has next when not in a container type. " + input); } skipWhitespace(input); - boolean seenElement = Boolean.TRUE.equals(containerHasElement.peekFirst()); + boolean seenElement = containerDepth > 0 && containerHasElement[containerDepth - 1]; if (input.peek() == ',') { if (!seenElement) { throw new JsonException("Unexpected ',' before first element of container. " + input); } input.read(); + peekedType = null; // We've moved past the separator, so we're once again expecting an element rather than // another comma. Clear the flag so a repeat hasNext() before reading is a no-op. clearSeenElement(); @@ -390,8 +516,7 @@ public boolean hasNext() { */ public void beginArray() { expect(JsonType.START_COLLECTION); - stack.addFirst(Container.COLLECTION); - containerHasElement.addFirst(false); + pushContainer(COLLECTION); input.read(); } @@ -402,13 +527,12 @@ public void beginArray() { */ public void endArray() { expect(JsonType.END_COLLECTION); - if (stack.peekFirst() != Container.COLLECTION) { + if (topContainer() != COLLECTION) { // The only other thing we could be closing is a map throw new JsonException( "Attempt to close a JSON List, but a JSON Object was expected. " + input); } - stack.removeFirst(); - containerHasElement.removeFirst(); + containerDepth--; input.read(); } @@ -419,8 +543,7 @@ public void endArray() { */ public void beginObject() { expect(JsonType.START_MAP); - stack.addFirst(Container.MAP_NAME); - containerHasElement.addFirst(false); + pushContainer(MAP_NAME); input.read(); } @@ -431,11 +554,10 @@ public void beginObject() { */ public void endObject() { expect(JsonType.END_MAP); - if (stack.peekFirst() != Container.MAP_NAME) { + if (topContainer() != MAP_NAME) { throw new JsonException("Attempt to close a JSON Map, but not ready to. " + input); } - stack.removeFirst(); - containerHasElement.removeFirst(); + containerDepth--; input.read(); } @@ -540,12 +662,14 @@ public T readMapElement(String key) { * @throws JsonException if coercion of the next element to the specified type fails * @throws UncheckedIOException if an I/O exception is encountered */ + @SuppressWarnings("unchecked") public List readArray(Type type) { List toReturn = new ArrayList<>(); + BiFunction elementCoercer = coercer.resolve(type); beginArray(); while (hasNext()) { - toReturn.add(coercer.coerce(this, type, setter)); + toReturn.add((T) elementCoercer.apply(this, setter)); } endArray(); @@ -558,7 +682,7 @@ public List readArray(Type type) { * @return {@code true} is awaiting a property name; otherwise {@code false} */ private boolean isReadingName() { - return stack.peekFirst() == Container.MAP_NAME; + return topContainer() == MAP_NAME; } /** @@ -574,15 +698,17 @@ private void expect(JsonType type) { "Expected to read a " + type + " but instead have: " + peek() + ". " + input); } + // The pending token is about to be consumed, so the memoized type is no longer valid. + peekedType = null; + // Special map handling. Woo! - Container top = stack.peekFirst(); + byte top = topContainer(); if (type == JsonType.NAME) { - if (top == Container.MAP_NAME) { - stack.removeFirst(); - stack.addFirst(Container.MAP_VALUE); + if (top == MAP_NAME) { + containerState[containerDepth - 1] = MAP_VALUE; return; - } else if (top != null) { + } else if (top != NONE) { throw new JsonException("Unexpected attempt to read name. " + input); } @@ -594,26 +720,37 @@ private void expect(JsonType type) { // Closing the container - don't treat as a new element in it. return; } - if (top == Container.MAP_VALUE) { - stack.removeFirst(); - stack.addFirst(Container.MAP_NAME); + if (top == MAP_VALUE) { + containerState[containerDepth - 1] = MAP_NAME; markElementRead(); - } else if (top == Container.COLLECTION) { + } else if (top == COLLECTION) { markElementRead(); } } + private byte topContainer() { + return containerDepth == 0 ? NONE : containerState[containerDepth - 1]; + } + + private void pushContainer(byte state) { + if (containerDepth == containerState.length) { + containerState = Arrays.copyOf(containerState, containerDepth * 2); + containerHasElement = Arrays.copyOf(containerHasElement, containerDepth * 2); + } + containerState[containerDepth] = state; + containerHasElement[containerDepth] = false; + containerDepth++; + } + private void markElementRead() { - if (!containerHasElement.isEmpty()) { - containerHasElement.removeFirst(); - containerHasElement.addFirst(true); + if (containerDepth > 0) { + containerHasElement[containerDepth - 1] = true; } } private void clearSeenElement() { - if (!containerHasElement.isEmpty()) { - containerHasElement.removeFirst(); - containerHasElement.addFirst(false); + if (containerDepth > 0) { + containerHasElement[containerDepth - 1] = false; } } @@ -653,25 +790,30 @@ private void clearSeenElement() { private String readString() { input.read(); // Skip leading quote + // Fast path: an escape-free string that is fully buffered needs no intermediate copies. + String simple = input.readSimpleString(); + if (simple != null) { + return simple; + } + StringBuilder builder = new StringBuilder(); while (true) { - int c = input.read(); + int c = input.appendStringContent(builder); switch (c) { case Input.EOF: throw new JsonException("Unterminated string: " + builder + ". " + input); case '"': // terminate string + input.read(); return builder.toString(); case '\\': // quoted char + input.read(); readEscape(builder); break; default: // RFC 8259 §7: characters U+0000..U+001F MUST be escaped. - if (c < 0x20) { - throw new JsonException( - String.format( - "Illegal unescaped control character U+%04X in string. %s", c, input)); - } - builder.append((char) c); + input.read(); + throw new JsonException( + String.format("Illegal unescaped control character U+%04X in string. %s", c, input)); } } } @@ -743,19 +885,18 @@ private void readEscape(StringBuilder builder) { * @throws UncheckedIOException if an I/O exception is encountered */ private void skipWhitespace(Input input) { - while (input.peek() != Input.EOF && Character.isWhitespace(input.peek())) { - input.read(); - } + input.skipWhitespace(); } - /** Used to track the current container processing state. */ - private enum Container { + /** Container processing states: not in a container. */ + private static final byte NONE = 0; - /** Processing a JSON array */ - COLLECTION, - /** Processing a JSON object property name */ - MAP_NAME, - /** Processing a JSON object property value */ - MAP_VALUE, - } + /** Container processing states: processing a JSON array. */ + private static final byte COLLECTION = 1; + + /** Container processing states: processing a JSON object property name. */ + private static final byte MAP_NAME = 2; + + /** Container processing states: processing a JSON object property value. */ + private static final byte MAP_VALUE = 3; } diff --git a/java/src/org/openqa/selenium/json/JsonOutput.java b/java/src/org/openqa/selenium/json/JsonOutput.java index f3bfe9bab8b12..bb2badddefc6c 100644 --- a/java/src/org/openqa/selenium/json/JsonOutput.java +++ b/java/src/org/openqa/selenium/json/JsonOutput.java @@ -29,19 +29,15 @@ import java.time.format.DateTimeFormatter; import java.util.ArrayDeque; import java.util.Collection; -import java.util.Collections; import java.util.Date; import java.util.Deque; -import java.util.LinkedHashMap; import java.util.Map; import java.util.Optional; import java.util.UUID; -import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.stream.Stream; import org.jspecify.annotations.Nullable; import org.openqa.selenium.internal.Require; import org.openqa.selenium.logging.LogLevelMapping; @@ -54,6 +50,9 @@ public class JsonOutput implements Closeable { private static final Logger LOG = Logger.getLogger(JsonOutput.class.getName()); static final int MAX_DEPTH = 100; + /** Number of chars of escaped output to accumulate before flushing to the appendable. */ + private static final int ESCAPE_BUFFER_SIZE = 4096; + private static final Predicate> GSON_ELEMENT; static { @@ -74,190 +73,200 @@ public class JsonOutput implements Closeable { // https://github.com/google/gson/issues/341 so we escape those as well. // It's legal to escape any character, so to be nice to HTML parsers, // we'll also escape "<" and "&" - private static final Map ESCAPES; + private static final String[] ASCII_ESCAPES = buildAsciiEscapes(); - static { - // increased initial capacity to avoid hash collisions, especially for the following ranges: - // '0' to '9', 'a' to 'z', 'A' to 'Z' - Map builder = new LinkedHashMap<>(128); + private static String[] buildAsciiEscapes() { + String[] escapes = new String[128]; for (int i = 0; i <= 0x1f; i++) { // We want nice looking escapes for these, which are called out // by json.org - if (!(i == '\b' || i == '\f' || i == '\n' || i == '\r' || i == '\t')) { - builder.put(i, String.format("\\u%04x", i)); - } + escapes[i] = String.format("\\u%04x", i); } - builder.put((int) '"', "\\\""); - builder.put((int) '\\', "\\\\"); - builder.put((int) '/', "\\u002f"); - builder.put((int) '\b', "\\b"); - builder.put((int) '\f', "\\f"); - builder.put((int) '\n', "\\n"); - builder.put((int) '\r', "\\r"); - builder.put((int) '\t', "\\t"); + escapes['"'] = "\\\""; + escapes['\\'] = "\\\\"; + escapes['/'] = "\\u002f"; + escapes['\b'] = "\\b"; + escapes['\f'] = "\\f"; + escapes['\n'] = "\\n"; + escapes['\r'] = "\\r"; + escapes['\t'] = "\\t"; - builder.put((int) '\u2028', "\\u2028"); - builder.put((int) '<', String.format("\\u%04x", (int) '<')); - builder.put((int) '&', String.format("\\u%04x", (int) '&')); - ESCAPES = Collections.unmodifiableMap(builder); + escapes['<'] = String.format("\\u%04x", (int) '<'); + escapes['&'] = String.format("\\u%04x", (int) '&'); + + return escapes; + } + + /** + * The serialization strategy depends only on the class of the value being written, so resolve it + * once per class rather than probing (potentially with reflection) for every value written. + */ + private static final ClassValue CONVERTERS = + new ClassValue() { + @Override + protected DepthAwareConsumer computeValue(Class type) { + return resolveConverter(type); + } + }; + + // Note: the order of the checks matters, and mirrors the historical converter list. + private static DepthAwareConsumer resolveConverter(Class cls) { + if (CharSequence.class.isAssignableFrom(cls)) { + return (out, obj, maxDepth, depthRemaining) -> out.writeString(obj); + } + if (Number.class.isAssignableFrom(cls)) { + return (out, obj, maxDepth, depthRemaining) -> out.append(obj.toString()); + } + if (Boolean.class.isAssignableFrom(cls)) { + return (out, obj, maxDepth, depthRemaining) -> out.append((Boolean) obj ? "true" : "false"); + } + if (Date.class.isAssignableFrom(cls)) { + return (out, obj, maxDepth, depthRemaining) -> + out.append(String.valueOf(MILLISECONDS.toSeconds(((Date) obj).getTime()))); + } + if (Instant.class.isAssignableFrom(cls)) { + return (out, obj, maxDepth, depthRemaining) -> + out.writeString(DateTimeFormatter.ISO_INSTANT.format((Instant) obj)); + } + if (Enum.class.isAssignableFrom(cls)) { + return (out, obj, maxDepth, depthRemaining) -> out.writeString(obj); + } + if (File.class.isAssignableFrom(cls)) { + return (out, obj, maxDepth, depthRemaining) -> out.append(((File) obj).getAbsolutePath()); + } + if (URI.class.isAssignableFrom(cls)) { + return (out, obj, maxDepth, depthRemaining) -> out.writeString(obj.toString()); + } + if (URL.class.isAssignableFrom(cls)) { + return (out, obj, maxDepth, depthRemaining) -> out.writeString(((URL) obj).toExternalForm()); + } + if (UUID.class.isAssignableFrom(cls)) { + return (out, obj, maxDepth, depthRemaining) -> out.writeString(obj.toString()); + } + if (Level.class.isAssignableFrom(cls)) { + return (out, obj, maxDepth, depthRemaining) -> + out.writeString(LogLevelMapping.getName((Level) obj)); + } + if (GSON_ELEMENT.test(cls)) { + return (out, obj, maxDepth, depthRemaining) -> { + LOG.log( + Level.WARNING, + "Attempt to convert JsonElement from GSON. This functionality is deprecated. " + + "Diagnostic stacktrace follows", + new JsonException("Stack trace to determine cause of warning")); + out.append(obj.toString()); + }; + } + + // Special handling of asMap and toJson + Method toJson = getMethod(cls, "toJson"); + if (toJson != null) { + return (out, obj, maxDepth, depthRemaining) -> + out.convertUsingMethod(toJson, obj, maxDepth, depthRemaining); + } + Method asMap = getMethod(cls, "asMap"); + if (asMap != null) { + return (out, obj, maxDepth, depthRemaining) -> + out.convertUsingMethod(asMap, obj, maxDepth, depthRemaining); + } + Method toMap = getMethod(cls, "toMap"); + if (toMap != null) { + return (out, obj, maxDepth, depthRemaining) -> + out.convertUsingMethod(toMap, obj, maxDepth, depthRemaining); + } + + // And then the collection types + if (Collection.class.isAssignableFrom(cls)) { + return (out, obj, maxDepth, depthRemaining) -> { + if (depthRemaining < 1) { + throw new JsonException( + "Reached the maximum depth of " + maxDepth + " while writing JSON"); + } + out.beginArray(); + for (Object o : (Collection) obj) { + if (o instanceof Optional && ((Optional) o).isEmpty()) { + continue; + } + out.write0(o, maxDepth, depthRemaining - 1); + } + out.endArray(); + }; + } + + if (Map.class.isAssignableFrom(cls)) { + return (out, obj, maxDepth, depthRemaining) -> { + if (depthRemaining < 1) { + throw new JsonException( + "Reached the maximum depth of " + maxDepth + " while writing JSON"); + } + out.beginObject(); + ((Map) obj) + .forEach( + (key, value) -> { + if (value instanceof Optional && ((Optional) value).isEmpty()) { + return; + } + out.name(String.valueOf(key)).write0(value, maxDepth, depthRemaining - 1); + }); + out.endObject(); + }; + } + + if (cls.isArray()) { + return (out, obj, maxDepth, depthRemaining) -> { + if (depthRemaining < 1) { + throw new JsonException( + "Reached the maximum depth of " + maxDepth + " while writing JSON"); + } + out.beginArray(); + for (Object o : (Object[]) obj) { + if (o instanceof Optional && ((Optional) o).isEmpty()) { + continue; + } + out.write0(o, maxDepth, depthRemaining - 1); + } + out.endArray(); + }; + } + + if (Optional.class.isAssignableFrom(cls)) { + return (out, obj, maxDepth, depthRemaining) -> { + Optional optional = (Optional) obj; + if (optional.isEmpty()) { + out.append("null"); + return; + } + + out.write0(optional.get(), maxDepth, depthRemaining); + }; + } + + // Finally, attempt to convert as an object + return (out, obj, maxDepth, depthRemaining) -> { + if (depthRemaining < 1) { + throw new JsonException("Reached the maximum depth of " + maxDepth + " while writing JSON"); + } + out.mapObject(obj, maxDepth, depthRemaining - 1); + }; } - private final Map>, DepthAwareConsumer> converters; private final Appendable appendable; - private final Consumer appender; private final Deque stack; private String indent = ""; private String lineSeparator = "\n"; private String indentBy = " "; + private String separator = ",\n"; + private String objectStart = "{\n"; + private String arrayStart = "[\n"; private boolean writeClassName = true; JsonOutput(Appendable appendable) { this.appendable = Require.nonNull("Underlying appendable", appendable); - this.appender = - str -> { - try { - appendable.append(str); - } catch (IOException e) { - throw new JsonException("Unable to write to underlying appendable", e); - } - }; - this.stack = new ArrayDeque<>(); this.stack.addFirst(new Root()); - - Map>, DepthAwareConsumer> builder = new LinkedHashMap<>(); - builder.put( - CharSequence.class::isAssignableFrom, - (obj, maxDepth, depthRemaining) -> append(asString(obj))); - builder.put( - Number.class::isAssignableFrom, (obj, maxDepth, depthRemaining) -> append(obj.toString())); - builder.put( - Boolean.class::isAssignableFrom, - (obj, maxDepth, depthRemaining) -> append((Boolean) obj ? "true" : "false")); - builder.put( - Date.class::isAssignableFrom, - (obj, maxDepth, depthRemaining) -> - append(String.valueOf(MILLISECONDS.toSeconds(((Date) obj).getTime())))); - builder.put( - Instant.class::isAssignableFrom, - (obj, maxDepth, depthRemaining) -> - append(asString(DateTimeFormatter.ISO_INSTANT.format((Instant) obj)))); - builder.put( - Enum.class::isAssignableFrom, (obj, maxDepth, depthRemaining) -> append(asString(obj))); - builder.put( - File.class::isAssignableFrom, - (obj, maxDepth, depthRemaining) -> append(((File) obj).getAbsolutePath())); - builder.put( - URI.class::isAssignableFrom, - (obj, maxDepth, depthRemaining) -> append(asString((obj).toString()))); - builder.put( - URL.class::isAssignableFrom, - (obj, maxDepth, depthRemaining) -> append(asString(((URL) obj).toExternalForm()))); - builder.put( - UUID.class::isAssignableFrom, - (obj, maxDepth, depthRemaining) -> append(asString(obj.toString()))); - builder.put( - Level.class::isAssignableFrom, - (obj, maxDepth, depthRemaining) -> append(asString(LogLevelMapping.getName((Level) obj)))); - builder.put( - GSON_ELEMENT, - (obj, maxDepth, depthRemaining) -> { - LOG.log( - Level.WARNING, - "Attempt to convert JsonElement from GSON. This functionality is deprecated. " - + "Diagnostic stacktrace follows", - new JsonException("Stack trace to determine cause of warning")); - append(obj.toString()); - }); - // Special handling of asMap and toJson - builder.put( - cls -> getMethod(cls, "toJson") != null, - (obj, maxDepth, depthRemaining) -> - convertUsingMethod("toJson", obj, maxDepth, depthRemaining)); - builder.put( - cls -> getMethod(cls, "asMap") != null, - (obj, maxDepth, depthRemaining) -> - convertUsingMethod("asMap", obj, maxDepth, depthRemaining)); - builder.put( - cls -> getMethod(cls, "toMap") != null, - (obj, maxDepth, depthRemaining) -> - convertUsingMethod("toMap", obj, maxDepth, depthRemaining)); - - // And then the collection types - builder.put( - Collection.class::isAssignableFrom, - (obj, maxDepth, depthRemaining) -> { - if (depthRemaining < 1) { - throw new JsonException( - "Reached the maximum depth of " + maxDepth + " while writing JSON"); - } - beginArray(); - ((Collection) obj) - .stream() - .filter(o -> (!(o instanceof Optional) || ((Optional) o).isPresent())) - .forEach(o -> write0(o, maxDepth, depthRemaining - 1)); - endArray(); - }); - - builder.put( - Map.class::isAssignableFrom, - (obj, maxDepth, depthRemaining) -> { - if (depthRemaining < 1) { - throw new JsonException( - "Reached the maximum depth of " + maxDepth + " while writing JSON"); - } - beginObject(); - ((Map) obj) - .forEach( - (key, value) -> { - if (value instanceof Optional && ((Optional) value).isEmpty()) { - return; - } - name(String.valueOf(key)).write0(value, maxDepth, depthRemaining - 1); - }); - endObject(); - }); - builder.put( - Class::isArray, - (obj, maxDepth, depthRemaining) -> { - if (depthRemaining < 1) { - throw new JsonException( - "Reached the maximum depth of " + maxDepth + " while writing JSON"); - } - beginArray(); - Stream.of((Object[]) obj) - .filter(o -> (!(o instanceof Optional) || ((Optional) o).isPresent())) - .forEach(o -> write0(o, maxDepth, depthRemaining - 1)); - endArray(); - }); - - builder.put( - Optional.class::isAssignableFrom, - (obj, maxDepth, depthRemaining) -> { - Optional optional = (Optional) obj; - if (optional.isEmpty()) { - append("null"); - return; - } - - write0(optional.get(), maxDepth, depthRemaining); - }); - - // Finally, attempt to convert as an object - builder.put( - cls -> true, - (obj, maxDepth, depthRemaining) -> { - if (depthRemaining < 1) { - throw new JsonException( - "Reached the maximum depth of " + maxDepth + " while writing JSON"); - } - mapObject(obj, maxDepth, depthRemaining - 1); - }); - - this.converters = Collections.unmodifiableMap(builder); } /** @@ -271,6 +280,9 @@ public class JsonOutput implements Closeable { public JsonOutput setPrettyPrint(boolean enablePrettyPrinting) { this.lineSeparator = enablePrettyPrinting ? "\n" : ""; this.indentBy = enablePrettyPrinting ? " " : ""; + this.separator = "," + lineSeparator; + this.objectStart = "{" + lineSeparator; + this.arrayStart = "[" + lineSeparator; return this; } @@ -292,7 +304,7 @@ public JsonOutput writeClassName(boolean writeClassName) { * @return this {@link JsonOutput} object */ public JsonOutput beginObject() { - stack.getFirst().write("{" + lineSeparator); + stack.getFirst().write(objectStart); indent += indentBy; stack.addFirst(new JsonObject()); return this; @@ -329,11 +341,13 @@ public JsonOutput endObject() { stack.removeFirst(); indent = indent.substring(0, indent.length() - indentBy.length()); - if (topOfStack.isEmpty) { - appender.accept(indent + "}"); + if (!topOfStack.isEmpty) { + rawAppend(lineSeparator); + rawAppend(indent); } else { - appender.accept(lineSeparator + indent + "}"); + rawAppend(indent); } + rawAppend("}"); return this; } @@ -343,7 +357,7 @@ public JsonOutput endObject() { * @return this {@link JsonOutput} object */ public JsonOutput beginArray() { - append("[" + lineSeparator); + append(arrayStart); indent += indentBy; stack.addFirst(new JsonCollection()); return this; @@ -364,11 +378,13 @@ public JsonOutput endArray() { stack.removeFirst(); indent = indent.substring(0, indent.length() - indentBy.length()); - if (topOfStack.isEmpty) { - appender.accept(indent + "]"); + if (!topOfStack.isEmpty) { + rawAppend(lineSeparator); + rawAppend(indent); } else { - appender.accept(lineSeparator + indent + "]"); + rawAppend(indent); } + rawAppend("]"); return this; } @@ -402,12 +418,19 @@ private JsonOutput write0(@Nullable Object input, int maxDepth, int depthRemaini append("null"); return this; } - converters.entrySet().stream() - .filter(entry -> entry.getKey().test(input.getClass())) - .findFirst() - .map(Map.Entry::getValue) - .orElseThrow(() -> new JsonException("Unable to write " + input)) - .consume(input, maxDepth, depthRemaining); + + // Fast paths for the classes that dominate real payloads, mirroring the corresponding + // converters and skipping the per-class strategy lookup. + Class cls = input.getClass(); + if (cls == String.class) { + writeString(input); + } else if (cls == Long.class || cls == Integer.class || cls == Double.class) { + append(input.toString()); + } else if (cls == Boolean.class) { + append((Boolean) input ? "true" : "false"); + } else { + CONVERTERS.get(cls).consume(this, input, maxDepth, depthRemaining); + } return this; } @@ -438,30 +461,88 @@ private JsonOutput append(String text) { return this; } + private void rawAppend(String text) { + if (text.isEmpty()) { + return; + } + + try { + appendable.append(text); + } catch (IOException e) { + throw new JsonException("Unable to write to underlying appendable", e); + } + } + /** - * Return a quoted JSON string representing the specified Java object. + * Write the specified Java object as a quoted JSON string, handling any bookkeeping required by + * the enclosing JSON container. * * @param obj Java object to be represented - * @return quoted JSON string */ - private String asString(Object obj) { - StringBuilder toReturn = new StringBuilder("\""); - - String.valueOf(obj) - .chars() - .forEach( - i -> { - String escaped = ESCAPES.get(i); - if (escaped != null) { - toReturn.append(escaped); - } else { - toReturn.append((char) i); - } - }); - - toReturn.append('"'); - - return toReturn.toString(); + private void writeString(Object obj) { + String value = String.valueOf(obj); + stack.getFirst().beginValue(value); + writeEscaped(value); + } + + /** + * Write a quoted JSON string directly to the underlying appendable, escaping as needed. Runs of + * characters that need no escaping are appended in bulk so no escaped copy of the value is ever + * materialized. + * + * @param value string to be written + */ + private void writeEscaped(String value) { + try { + appendable.append('"'); + + int length = value.length(); + int plainStart = 0; + // Escaped output is batched in here before being flushed to the appendable, so that a + // heavily escaped value doesn't degrade into per-character appendable calls. Bounded by + // the flush below, so no full escaped copy of the value is ever materialized. + StringBuilder buffered = null; + + for (int i = 0; i < length; i++) { + char c = value.charAt(i); + String escape; + if (c < 128) { + escape = ASCII_ESCAPES[c]; + } else if (c == '\u2028') { + escape = "\\u2028"; + } else { + escape = null; + } + + if (escape != null) { + if (buffered == null) { + buffered = new StringBuilder(Math.min(length + 16, ESCAPE_BUFFER_SIZE + 16)); + } + buffered.append(value, plainStart, i).append(escape); + if (buffered.length() >= ESCAPE_BUFFER_SIZE) { + appendable.append(buffered); + buffered.setLength(0); + } + plainStart = i + 1; + } + } + + if (buffered == null) { + // No escaping was needed; write the whole value through unmodified. + appendable.append(value); + } else { + if (buffered.length() > 0) { + appendable.append(buffered); + } + if (plainStart < length) { + appendable.append(value, plainStart, length); + } + } + + appendable.append('"'); + } catch (IOException e) { + throw new JsonException("Unable to write to underlying appendable", e); + } } /** @@ -473,7 +554,7 @@ private String asString(Object obj) { * @return {@link Method} object with 'accessible' flag set * @throws JsonException if a security violation is encountered */ - private @Nullable Method getMethod(Class clazz, String methodName) { + private static @Nullable Method getMethod(Class clazz, String methodName) { if (Object.class.equals(clazz)) { return null; } @@ -493,27 +574,20 @@ private String asString(Object obj) { /** * Convert the specified Java object using the indicated zero-argument method of this object. * - * @param methodName method name + * @param method zero-argument method that produces the serializable form of the object * @param toConvert Java object to be converted * @param maxDepth maximum depth of nested object traversal * @param depthRemaining allowed traversal depth remaining * @return this {@link JsonOutput} object * @throws JsonException *

    - *
  • if the specified method isn't found - *
  • if a security violation is encountered *
  • if a reflective operation fails *
  • if maximum traversal depth is exceeded *
*/ private JsonOutput convertUsingMethod( - String methodName, Object toConvert, int maxDepth, int depthRemaining) { + Method method, Object toConvert, int maxDepth, int depthRemaining) { try { - Method method = getMethod(toConvert.getClass(), methodName); - if (method == null) { - throw new JsonException( - String.format("Unable to read object %s using method %s", toConvert, methodName)); - } Object value = method.invoke(toConvert); return write0(value, maxDepth, depthRemaining); @@ -566,22 +640,31 @@ private abstract class Node { protected boolean isEmpty = true; /** - * Write the specified text to the appender of this JSON output object.
- * NOTE: If prior text has been written to this container, the new text is prefixed with - * a comma and the defined line separator (either {@literal } or empty string) to - * delimit a new object property or array item. + * Perform the bookkeeping needed before a new value is written to this container.
+ * NOTE: If prior text has been written to this container, a comma and the defined line + * separator (either {@literal } or empty string) are emitted to delimit a new object + * property or array item. * - * @param text text to be appended to the output + * @param value the value about to be written, used only for error reporting */ - public void write(String text) { + void beginValue(Object value) { if (isEmpty) { isEmpty = false; } else { - appender.accept("," + lineSeparator); + rawAppend(separator); } - appender.accept(indent); - appender.accept(text); + rawAppend(indent); + } + + /** + * Write the specified text to the appendable of this JSON output object. + * + * @param text text to be appended to the output + */ + public void write(String text) { + beginValue(text); + rawAppend(text); } } @@ -589,18 +672,17 @@ public void write(String text) { private class Root extends Node { /** - * Write the specified text to the appender of this JSON output object. + * {@inheritDoc} * - * @param text text to be appended to the output * @throws JsonException if this {@link JsonOutput} has already been used. */ @Override - public void write(String text) { + void beginValue(Object value) { if (!isEmpty) { throw new JsonException("Only allowed to write one value to a json stream"); } - super.write(text); + super.beginValue(value); } } @@ -612,7 +694,7 @@ private class JsonObject extends Node { private boolean isNameNext = true; /** - * Writes the name of a JSON property followed by a colon to the appender of this JSON output + * Writes the name of a JSON property followed by a colon to the appendable of this JSON output * object. * * @param name JSON object property name @@ -623,24 +705,22 @@ public void name(String name) { throw new JsonException("Unexpected attempt to set name of json object: " + name); } isNameNext = false; - super.write(asString(name)); - appender.accept(": "); + super.beginValue(name); + writeEscaped(name); + rawAppend(": "); } /** - * Write the value of a JSON property to the appender of this JSON output object. + * {@inheritDoc} * - * @param text JSON object property value * @throws JsonException if not expecting a JSON property value */ @Override - public void write(String text) { + void beginValue(Object value) { if (isNameNext) { - throw new JsonException("Unexpected attempt to write value before name: " + text); + throw new JsonException("Unexpected attempt to write value before name: " + value); } isNameNext = true; - - appender.accept(text); } } @@ -651,13 +731,14 @@ public void write(String text) { private interface DepthAwareConsumer { /** - * Consume the specified Java object, emitting its JSON representation to the appender of this - * {@link JsonOutput}. + * Consume the specified Java object, emitting its JSON representation to the appendable of the + * supplied {@link JsonOutput}. * + * @param out {@link JsonOutput} to write to * @param object Java object to be serialized * @param maxDepth maximum depth of nested object traversal * @param depthRemaining allowed traversal depth remaining */ - void consume(Object object, int maxDepth, int depthRemaining); + void consume(JsonOutput out, Object object, int maxDepth, int depthRemaining); } } diff --git a/java/src/org/openqa/selenium/json/JsonTypeCoercer.java b/java/src/org/openqa/selenium/json/JsonTypeCoercer.java index 0c5d2f696d94a..17a18f64ee33d 100644 --- a/java/src/org/openqa/selenium/json/JsonTypeCoercer.java +++ b/java/src/org/openqa/selenium/json/JsonTypeCoercer.java @@ -39,7 +39,6 @@ import java.util.stream.StreamSupport; import org.openqa.selenium.Capabilities; import org.openqa.selenium.MutableCapabilities; -import org.openqa.selenium.internal.Require; /** * The JsonTypeCoercer class manages a collection of type coercers, providing a single source @@ -49,6 +48,31 @@ class JsonTypeCoercer { private final Set> additionalCoercers; private final Set> coercers; + + /** + * Cache of resolved coercion functions for plain {@link Class} keys, which are the overwhelmingly + * common case. A {@link ClassValue} stores each entry on the class itself, so classes (and their + * class loaders) remain eligible for garbage collection — important for callers that deserialize + * into dynamically generated classes, since {@link Json} shares a single coercer for the life of + * the JVM. + */ + private final ClassValue> classCoercers = + new ClassValue>() { + @Override + protected BiFunction computeValue(Class type) { + return buildCoercer(type); + } + }; + + /** + * Cache of resolved coercion functions for parameterized types passed directly to {@link #coerce} + * — in practice, the handful of {@link TypeToken} constants in the codebase. Unlike {@link + * #classCoercers}, entries here live as long as this coercer does, so the map is capped: if a + * caller funnels many distinct types through it (say, types built around generated classes), it + * is cleared rather than allowed to pin those classes forever. + */ + private static final int MAX_CACHED_GENERIC_TYPES = 256; + private final Map> knownCoercers = new ConcurrentHashMap<>(); @@ -140,41 +164,106 @@ private JsonTypeCoercer(Stream> coercers) { } T coerce(JsonInput json, Type typeOfT, PropertySetting setter) { - BiFunction coercer = - knownCoercers.computeIfAbsent(typeOfT, this::buildCoercer); + @SuppressWarnings("unchecked") + T result = (T) resolve(typeOfT).apply(json, setter); - if (json.peek() == JsonType.NULL && !isOptional(typeOfT)) { - @SuppressWarnings("unchecked") - T next = (T) json.nextNull(); - return next; + return result; + } + + /** + * Resolve the coercion function for the specified type, caching the result. Callers that coerce + * many values of the same type (containers, in particular) should resolve once and reuse the + * returned function rather than paying a cache lookup per element. + * + * @param type data type for deserialization (class or {@link TypeToken}) + * @return {@link BiFunction} object to deserialize the specified Java type + */ + BiFunction resolve(Type type) { + if (type instanceof Class) { + return classCoercers.get((Class) type); } - // We need to keep null checkers happy, apparently. - @SuppressWarnings("unchecked") - T result = (T) Require.nonNull("Coercer", coercer).apply(json, setter); + // Plain get first: this is almost always a hit, and avoids the capturing lambda that + // computeIfAbsent would allocate on every call. + BiFunction coercer = knownCoercers.get(type); + if (coercer == null) { + if (knownCoercers.size() >= MAX_CACHED_GENERIC_TYPES) { + knownCoercers.clear(); + } + coercer = knownCoercers.computeIfAbsent(type, this::buildCoercer); + } + return coercer; + } - return result; + /** + * Return a function that resolves the coercer for the specified type on first use. Unlike {@link + * #resolve}, this is safe to call from within a {@link TypeCoercer#apply} implementation, where + * an eager resolution would recursively update the coercer cache mid-computation. + * + * @param type data type for deserialization (class or {@link TypeToken}) + * @return {@link BiFunction} object to deserialize the specified Java type + */ + BiFunction lazyResolve(Type type) { + return new LazyCoercer(this, type); + } + + private static class LazyCoercer implements BiFunction { + private final JsonTypeCoercer coercer; + private final Type type; + private volatile BiFunction delegate; + + LazyCoercer(JsonTypeCoercer coercer, Type type) { + this.coercer = coercer; + this.type = type; + } + + @Override + public Object apply(JsonInput json, PropertySetting setter) { + BiFunction resolved = delegate; + if (resolved == null) { + if (type instanceof Class) { + resolved = coercer.resolve(type); + } else { + // Memoize here rather than in the shared map: this LazyCoercer is reachable only from + // the coercion function for its owning type, so a parameterized type mentioning a + // generated class dies with that class instead of being pinned by the cache. + resolved = coercer.buildCoercer(type); + } + delegate = resolved; + } + return resolved.apply(json, setter); + } } /** * Extract the coercer that supports the specified type from the collection managed by this {@code - * JsonTypeCoercer}, returning a coercion function for the client to use. + * JsonTypeCoercer}, returning a coercion function for the client to use. The returned function + * takes care of JSON null handling, so it can be invoked directly. * * @param type data type for deserialization (class or {@link TypeToken}) * @return {@link BiFunction} object to deserialize the specified Java type */ private BiFunction buildCoercer(Type type) { - return coercers.stream() - .filter(coercer -> coercer.test(narrow(type))) - .findFirst() - .map( - coercer -> { - @SuppressWarnings("unchecked") - BiFunction funct = - (BiFunction) coercer.apply(type); - return funct; - }) - .orElseThrow(() -> new JsonException("Unable to find type coercer for " + type)); + TypeCoercer matched = + coercers.stream() + .filter(coercer -> coercer.test(narrow(type))) + .findFirst() + .orElseThrow(() -> new JsonException("Unable to find type coercer for " + type)); + + @SuppressWarnings("unchecked") + BiFunction inner = + (BiFunction) matched.apply(type); + + if (matched.handlesNull() || isOptional(type)) { + return inner; + } + + return (json, setter) -> { + if (json.peek() == JsonType.NULL) { + return json.nextNull(); + } + return inner.apply(json, setter); + }; } private boolean isOptional(Type type) { diff --git a/java/src/org/openqa/selenium/json/MapCoercer.java b/java/src/org/openqa/selenium/json/MapCoercer.java index ad254741a5dbf..76a7ae828504e 100644 --- a/java/src/org/openqa/selenium/json/MapCoercer.java +++ b/java/src/org/openqa/selenium/json/MapCoercer.java @@ -63,12 +63,17 @@ public BiFunction apply(Type type) { throw new IllegalArgumentException("Unhandled type: " + type.getClass()); } + // JSON should always have a string key, so we can take the fastpath + boolean stringKey = String.class.equals(keyType); + // Resolve the element coercers once rather than paying a cache lookup per entry. + BiFunction keyCoercer = + stringKey ? null : coercer.lazyResolve(keyType); + BiFunction valueCoercer = coercer.lazyResolve(valueType); + return (jsonInput, setting) -> { jsonInput.beginObject(); I toReturn = supplier.get(); BiConsumer consumer = consumerFactory.apply(toReturn); - // JSON should always have a string key, so we can take the fastpath - boolean stringKey = String.class.equals(keyType); while (jsonInput.hasNext()) { Object key; @@ -76,9 +81,9 @@ public BiFunction apply(Type type) { if (stringKey) { key = jsonInput.nextName(); } else { - key = coercer.coerce(jsonInput, keyType, setting); + key = keyCoercer.apply(jsonInput, setting); } - Object value = coercer.coerce(jsonInput, valueType, setting); + Object value = valueCoercer.apply(jsonInput, setting); consumer.accept(key, value); } diff --git a/java/src/org/openqa/selenium/json/ObjectCoercer.java b/java/src/org/openqa/selenium/json/ObjectCoercer.java index 65980294197c0..322d181d42705 100644 --- a/java/src/org/openqa/selenium/json/ObjectCoercer.java +++ b/java/src/org/openqa/selenium/json/ObjectCoercer.java @@ -35,39 +35,48 @@ public boolean test(Class type) { return Object.class.equals(type); } + @Override + boolean handlesNull() { + return true; + } + @Override public BiFunction apply(Type type) { - return (jsonInput, setting) -> { - Type target; + // Resolve the possible target coercers once rather than paying a cache lookup per value. + BiFunction booleanCoercer = + coercer.lazyResolve(Boolean.class); + BiFunction stringCoercer = + coercer.lazyResolve(String.class); + BiFunction numberCoercer = + coercer.lazyResolve(Number.class); + BiFunction listCoercer = coercer.lazyResolve(List.class); + BiFunction mapCoercer = coercer.lazyResolve(Json.MAP_TYPE); + return (jsonInput, setting) -> { switch (jsonInput.peek()) { + case NULL: + return jsonInput.nextNull(); + case BOOLEAN: - target = Boolean.class; - break; + return booleanCoercer.apply(jsonInput, setting); case NAME: case STRING: - target = String.class; - break; + return stringCoercer.apply(jsonInput, setting); case NUMBER: - target = Number.class; - break; + return numberCoercer.apply(jsonInput, setting); case START_COLLECTION: - target = List.class; - break; + return listCoercer.apply(jsonInput, setting); case START_MAP: - target = Json.MAP_TYPE; - break; + return mapCoercer.apply(jsonInput, setting); default: throw new JsonException( "Object coercer cannot determine proper type: " + jsonInput.peek()); } - - return coercer.coerce(jsonInput, target, setting); }; } } diff --git a/java/src/org/openqa/selenium/json/SimplePropertyDescriptor.java b/java/src/org/openqa/selenium/json/SimplePropertyDescriptor.java index 301e33eed8742..971f7cb95231d 100644 --- a/java/src/org/openqa/selenium/json/SimplePropertyDescriptor.java +++ b/java/src/org/openqa/selenium/json/SimplePropertyDescriptor.java @@ -17,6 +17,11 @@ package org.openqa.selenium.json; +import java.lang.invoke.CallSite; +import java.lang.invoke.LambdaMetafactory; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; import java.lang.reflect.Method; import java.util.HashMap; import java.util.Locale; @@ -60,7 +65,23 @@ public String toString() { return String.format("%s.%s", clazz.getSimpleName(), name); } + /** + * The descriptors depend only on the class, so compute them once per class rather than scanning + * the class's methods on every call. + */ + private static final ClassValue DESCRIPTOR_CACHE = + new ClassValue() { + @Override + protected SimplePropertyDescriptor[] computeValue(Class type) { + return computePropertyDescriptors(type); + } + }; + public static SimplePropertyDescriptor[] getPropertyDescriptors(Class clazz) { + return DESCRIPTOR_CACHE.get(clazz).clone(); + } + + private static SimplePropertyDescriptor[] computePropertyDescriptors(Class clazz) { Map properties = new HashMap<>(); properties.put("class", new SimplePropertyDescriptor(clazz, "class", GET_CLASS_NAME, null)); @@ -96,17 +117,7 @@ public static SimplePropertyDescriptor[] getPropertyDescriptors(Class clazz) Function read = null; if (readMethod != null) { - final Method finalReadMethod = readMethod; - - read = - obj -> { - try { - finalReadMethod.setAccessible(true); - return finalReadMethod.invoke(obj); - } catch (ReflectiveOperationException e) { - throw new JsonException(e); - } - }; + read = compileGetter(readMethod); } if (propertyName != null && (readMethod != null || writeMethod != null)) { @@ -128,6 +139,48 @@ public static SimplePropertyDescriptor[] getPropertyDescriptors(Class clazz) return properties.values().toArray(pdsArray); } + /** + * Produce a function that invokes the supplied getter. Where possible the getter is compiled to a + * direct call via {@link LambdaMetafactory}, which is considerably faster than reflective + * invocation; anything that cannot be compiled (inaccessible types, static methods, and the like) + * falls back to {@link Method#invoke}, deferring accessibility problems to invocation time just + * as reflective calls always have. + */ + private static Function compileGetter(Method method) { + try { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + MethodHandle handle = lookup.unreflect(method); + CallSite site = + LambdaMetafactory.metafactory( + lookup, + "apply", + MethodType.methodType(Function.class), + MethodType.methodType(Object.class, Object.class), + handle, + handle.type()); + @SuppressWarnings("unchecked") + Function getter = + (Function) site.getTarget().invokeExact(); + return obj -> { + try { + return getter.apply(obj); + } catch (Exception e) { + // Reflective invocation wrapped getter failures in a JsonException; stay consistent. + throw new JsonException(e); + } + }; + } catch (Throwable t) { + return obj -> { + try { + method.setAccessible(true); + return method.invoke(obj); + } catch (ReflectiveOperationException e) { + throw new JsonException(e); + } + }; + } + } + private static String uncapitalize(String s) { return s.substring(0, 1).toLowerCase(Locale.ENGLISH) + s.substring(1); } diff --git a/java/src/org/openqa/selenium/json/TypeCoercer.java b/java/src/org/openqa/selenium/json/TypeCoercer.java index d3bde66d2f84e..7ff7618292585 100644 --- a/java/src/org/openqa/selenium/json/TypeCoercer.java +++ b/java/src/org/openqa/selenium/json/TypeCoercer.java @@ -30,4 +30,12 @@ public abstract class TypeCoercer @Override public abstract BiFunction apply(Type type); + + /** + * Whether the functions produced by {@link #apply} handle a pending JSON null themselves. When + * {@code false}, {@link JsonTypeCoercer} consumes the null before the function is invoked. + */ + boolean handlesNull() { + return false; + } } diff --git a/java/src/org/openqa/selenium/remote/NewSessionPayload.java b/java/src/org/openqa/selenium/remote/NewSessionPayload.java index 9f49eb00ca449..2a40ad5e974ce 100644 --- a/java/src/org/openqa/selenium/remote/NewSessionPayload.java +++ b/java/src/org/openqa/selenium/remote/NewSessionPayload.java @@ -169,6 +169,7 @@ private void validate() throws IOException { public void writeTo(Appendable appendable) throws IOException { try (JsonOutput json = new Json().newOutput(appendable)) { + json.setPrettyPrint(false); json.beginObject(); // Now for the w3c capabilities diff --git a/java/src/org/openqa/selenium/remote/codec/AbstractHttpCommandCodec.java b/java/src/org/openqa/selenium/remote/codec/AbstractHttpCommandCodec.java index dd16eb6ea578a..67ed6b9ed36e2 100644 --- a/java/src/org/openqa/selenium/remote/codec/AbstractHttpCommandCodec.java +++ b/java/src/org/openqa/selenium/remote/codec/AbstractHttpCommandCodec.java @@ -96,6 +96,7 @@ import org.openqa.selenium.UnsupportedCommandException; import org.openqa.selenium.internal.Require; import org.openqa.selenium.json.Json; +import org.openqa.selenium.json.JsonOutput; import org.openqa.selenium.net.Urls; import org.openqa.selenium.remote.Command; import org.openqa.selenium.remote.CommandCodec; @@ -239,8 +240,11 @@ public HttpRequest encode(Command command) { HttpRequest request = new HttpRequest(spec.method, uri); if (HttpMethod.POST == spec.method) { - String content = json.toJson(parameters); - byte[] data = content.getBytes(UTF_8); + StringBuilder content = new StringBuilder(); + try (JsonOutput out = json.newOutput(content)) { + out.setPrettyPrint(false).write(parameters); + } + byte[] data = content.toString().getBytes(UTF_8); request.setHeader(HttpHeader.ContentLength.getName(), String.valueOf(data.length)); request.setHeader(HttpHeader.ContentType.getName(), JSON_UTF_8); diff --git a/java/src/org/openqa/selenium/remote/codec/AbstractHttpResponseCodec.java b/java/src/org/openqa/selenium/remote/codec/AbstractHttpResponseCodec.java index fd24b55e1a0d2..e6b72a1eec156 100644 --- a/java/src/org/openqa/selenium/remote/codec/AbstractHttpResponseCodec.java +++ b/java/src/org/openqa/selenium/remote/codec/AbstractHttpResponseCodec.java @@ -28,6 +28,7 @@ import java.util.function.Supplier; import org.openqa.selenium.json.Json; import org.openqa.selenium.json.JsonException; +import org.openqa.selenium.json.JsonOutput; import org.openqa.selenium.remote.ErrorCodes; import org.openqa.selenium.remote.Response; import org.openqa.selenium.remote.ResponseCodec; @@ -54,7 +55,11 @@ public HttpResponse encode(Supplier factory, Response response) { int responseStatus = requireNonNullElse(response.getStatus(), 0); int status = responseStatus == ErrorCodes.SUCCESS ? HTTP_OK : HTTP_INTERNAL_ERROR; - byte[] data = json.toJson(getValueToEncode(response)).getBytes(UTF_8); + StringBuilder content = new StringBuilder(); + try (JsonOutput out = json.newOutput(content)) { + out.setPrettyPrint(false).write(getValueToEncode(response)); + } + byte[] data = content.toString().getBytes(UTF_8); HttpResponse httpResponse = factory.get(); httpResponse.setStatus(status); diff --git a/java/src/org/openqa/selenium/remote/http/Contents.java b/java/src/org/openqa/selenium/remote/http/Contents.java index e21e93ae94e31..75bb7bb7d795a 100644 --- a/java/src/org/openqa/selenium/remote/http/Contents.java +++ b/java/src/org/openqa/selenium/remote/http/Contents.java @@ -160,6 +160,7 @@ public static Reader reader(HttpMessage message) { public static Supplier asJson(Object obj) { ByteArrayOutputStream output = new ByteArrayOutputStream(); try (JsonOutput out = JSON.newOutput(new OutputStreamWriter(output, UTF_8))) { + out.setPrettyPrint(false); out.writeClassName(false); out.write(obj); } diff --git a/java/test/org/openqa/selenium/json/NumberParsingTest.java b/java/test/org/openqa/selenium/json/NumberParsingTest.java new file mode 100644 index 0000000000000..16e07f5472808 --- /dev/null +++ b/java/test/org/openqa/selenium/json/NumberParsingTest.java @@ -0,0 +1,127 @@ +// Licensed to the Software Freedom Conservancy (SFC) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The SFC 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.openqa.selenium.json; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.openqa.selenium.json.PropertySetting.BY_NAME; + +import java.io.StringReader; +import java.util.Random; +import org.junit.jupiter.api.Test; + +/** + * The decimal parser takes a fast path for values whose significand fits exactly in a double; these + * tests pin its results bit-for-bit to {@link Double#parseDouble}. + */ +class NumberParsingTest { + + @Test + void readsDoubleEdgeCasesBitForBitIdenticalToTheJdk() { + String[] edges = { + "0.0", + "-0.0", + "0.5", + "0.1", + "0.3", + "2.5", + "123.456", + "-123.456", + "1e22", + "1E22", + "1e-22", + "-1e22", + "1e23", + "1e-23", + "999999999999999.9", + "9007199254740992.0", + "9007199254740993.0", + "0.000001", + "0.000000000000001234", + "1234567890.12345", + "31.875", + "1e0", + "0.0e0", + "5e-1", + "1.7976931348623157e308", + "4.9e-324", + "2.2250738585072011e-308", + "2.2250738585072014e-308", + "1e-325", + "3.141592653589793", + "2.718281828459045", + "1.7976931348623157e307" + }; + + for (String raw : edges) { + assertParsesIdentically(raw); + } + } + + @Test + void readsRandomisedDoublesBitForBitIdenticalToTheJdk() { + Random random = new Random(42); + + // Doubles of varying magnitude, rendered by the JDK itself. + for (int i = 0; i < 20_000; i++) { + double scale = Math.pow(10, random.nextInt(40) - 20); + assertParsesIdentically(Double.toString(random.nextDouble() * scale)); + } + + // Adversarial decimal strings: random significand lengths, fraction points, and exponents. + for (int i = 0; i < 20_000; i++) { + StringBuilder raw = new StringBuilder(); + if (random.nextBoolean()) { + raw.append('-'); + } + int intDigits = random.nextInt(19) + 1; + raw.append(random.nextInt(9) + 1); + for (int d = 1; d < intDigits; d++) { + raw.append(random.nextInt(10)); + } + boolean hasFraction = random.nextBoolean(); + if (hasFraction) { + raw.append('.'); + int fractionDigits = random.nextInt(19) + 1; + for (int d = 0; d < fractionDigits; d++) { + raw.append(random.nextInt(10)); + } + } + if (!hasFraction || random.nextBoolean()) { + raw.append(random.nextBoolean() ? 'e' : 'E'); + if (random.nextBoolean()) { + raw.append(random.nextBoolean() ? '+' : '-'); + } + raw.append(random.nextInt(40)); + } + assertParsesIdentically(raw.toString()); + } + } + + private void assertParsesIdentically(String raw) { + Number parsed; + try (JsonInput input = new JsonInput(new StringReader(raw), new JsonTypeCoercer(), BY_NAME)) { + parsed = input.nextNumber(); + } + + assertThat(parsed).isInstanceOf(Double.class); + double expected = Double.parseDouble(raw); + assertThat(Double.doubleToLongBits((Double) parsed)) + .as("parsing %s (expected %s, got %s)", raw, expected, parsed) + .isEqualTo(Double.doubleToLongBits(expected)); + } +}