Skip to content

Commit

Permalink
SearchSlowLog uses a non thread-safe object to escape json (#48363)
Browse files Browse the repository at this point in the history
This commit fixes the usage of JsonStringEncoder#quoteAsUTF8 in the SearchSlowLog.
JsonStringEncoder#getInstance should always be called to get a thread local object
but this assumption was broken by #44642. This means that any slow log can throw
an AIOOBE since it uses the same byte array concurrently.

Closes #48358
  • Loading branch information
jimczi authored Oct 23, 2019
1 parent b1cfb4b commit cf13259
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@
*/
package org.elasticsearch.common.logging;

import com.fasterxml.jackson.core.io.JsonStringEncoder;
import org.apache.logging.log4j.message.MapMessage;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Chars;
import org.apache.logging.log4j.util.StringBuilders;

import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedHashMap;
Expand All @@ -37,8 +35,6 @@
* A base class for custom log4j logger messages. Carries additional fields which will populate JSON fields in logs.
*/
public class ESLogMessage extends MapMessage<ESLogMessage, Object> {
private static final JsonStringEncoder JSON_STRING_ENCODER = JsonStringEncoder.getInstance();

private final String messagePattern;
private final List<Object> arguments = new ArrayList<>();

Expand Down Expand Up @@ -106,9 +102,4 @@ public static String asJsonArray(Stream<String> stream) {
.map(ESLogMessage::inQuotes)
.collect(Collectors.joining(", ")) + "]";
}

public static String escapeJson(String text) {
byte[] sourceEscaped = JSON_STRING_ENCODER.quoteAsUTF8(text);
return new String(sourceEscaped, Charset.defaultCharset());
}
}
13 changes: 11 additions & 2 deletions server/src/main/java/org/elasticsearch/index/SearchSlowLog.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.index;

import com.fasterxml.jackson.core.io.JsonStringEncoder;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.common.Strings;
Expand All @@ -32,13 +33,16 @@
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.tasks.Task;

import java.nio.charset.Charset;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.stream.Stream;

public final class SearchSlowLog implements SearchOperationListener {
private static final Charset UTF_8 = Charset.forName("UTF-8");

private long queryWarnThreshold;
private long queryInfoThreshold;
private long queryDebugThreshold;
Expand Down Expand Up @@ -169,13 +173,13 @@ private static Map<String, Object> prepareMap(SearchContext context, long tookIn
} else {
messageFields.put("total_hits", "-1");
}
messageFields.put("stats", ESLogMessage.escapeJson(ESLogMessage.asJsonArray(
messageFields.put("stats", escapeJson(ESLogMessage.asJsonArray(
context.groupStats() != null ? context.groupStats().stream() : Stream.empty())));
messageFields.put("search_type", context.searchType());
messageFields.put("total_shards", context.numberOfShards());

if (context.request().source() != null) {
String source = ESLogMessage.escapeJson(context.request().source().toString(FORMAT_PARAMS));
String source = escapeJson(context.request().source().toString(FORMAT_PARAMS));

messageFields.put("source", source);
} else {
Expand Down Expand Up @@ -221,6 +225,11 @@ private static String message(SearchContext context, long tookInNanos) {
}
return sb.toString();
}

private static String escapeJson(String text) {
byte[] sourceEscaped = JsonStringEncoder.getInstance().quoteAsUTF8(text);
return new String(sourceEscaped, UTF_8);
}
}

private void setQueryWarnThreshold(TimeValue warnThreshold) {
Expand Down

0 comments on commit cf13259

Please sign in to comment.