Skip to content

Commit f0b979b

Browse files
rjernstmridula-s109
authored andcommitted
Add http-only headers to ElasticsearchException (elastic#130348)
When return an error from Elasticsearch exceptions may contain values written as http response headers. ElasticsearchException contains a map of headers that are added to the response. But these values are also written to a special "header" section of the response body. This commit renames the existing "headers" in ElasticsearchException to "body headers", which are both http headers and written to the response body. A new "http headers" is added for headers that should only be written as response headers.
1 parent 3c8c526 commit f0b979b

File tree

34 files changed

+235
-168
lines changed

34 files changed

+235
-168
lines changed

modules/reindex/src/test/java/org/elasticsearch/reindex/ReindexFromRemoteWithAuthTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ public <Request extends ActionRequest, Response extends ActionResponse> void app
209209
String auth = context.getHeader(AUTHORIZATION_HEADER);
210210
if (auth == null) {
211211
ElasticsearchSecurityException e = new ElasticsearchSecurityException("Authentication required", RestStatus.UNAUTHORIZED);
212-
e.addHeader("WWW-Authenticate", "Basic realm=auth-realm");
212+
e.addBodyHeader("WWW-Authenticate", "Basic realm=auth-realm");
213213
throw e;
214214
}
215215
if (false == REQUIRED_AUTH.equals(auth)) {

server/src/internalClusterTest/java/org/elasticsearch/ingest/IngestClientIT.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -322,8 +322,8 @@ public void testPipelineOriginHeader() throws Exception {
322322
client().index(indexRequest).get();
323323
});
324324
IngestProcessorException ingestException = (IngestProcessorException) e.getCause();
325-
assertThat(ingestException.getHeader("processor_type"), equalTo(List.of("fail")));
326-
assertThat(ingestException.getHeader("pipeline_origin"), equalTo(List.of("3", "2", "1")));
325+
assertThat(ingestException.getBodyHeader("processor_type"), equalTo(List.of("fail")));
326+
assertThat(ingestException.getBodyHeader("pipeline_origin"), equalTo(List.of("3", "2", "1")));
327327
}
328328

329329
public void testPipelineProcessorOnFailure() throws Exception {

server/src/main/java/org/elasticsearch/ElasticsearchException.java

Lines changed: 56 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,8 @@ public class ElasticsearchException extends RuntimeException implements ToXConte
122122
private static final Map<Integer, CheckedFunction<StreamInput, ? extends ElasticsearchException, IOException>> ID_TO_SUPPLIER;
123123
private static final Map<Class<? extends ElasticsearchException>, ElasticsearchExceptionHandle> CLASS_TO_ELASTICSEARCH_EXCEPTION_HANDLE;
124124
private final Map<String, List<String>> metadata = new HashMap<>();
125-
private final Map<String, List<String>> headers = new HashMap<>();
125+
private final Map<String, List<String>> bodyHeaders = new HashMap<>();
126+
private final Map<String, List<String>> httpHeaders = new HashMap<>();
126127

127128
/**
128129
* Construct a <code>ElasticsearchException</code> with the specified cause exception.
@@ -169,14 +170,14 @@ public ElasticsearchException(String msg, Throwable cause, Object... args) {
169170
public ElasticsearchException(StreamInput in) throws IOException {
170171
super(in.readOptionalString(), in.readException());
171172
readStackTrace(this, in);
172-
headers.putAll(in.readMapOfLists(StreamInput::readString));
173+
bodyHeaders.putAll(in.readMapOfLists(StreamInput::readString));
173174
metadata.putAll(in.readMapOfLists(StreamInput::readString));
174175
}
175176

176177
private void maybePutTimeoutHeader() {
177178
if (isTimeout()) {
178179
// see https://www.rfc-editor.org/rfc/rfc8941.html#section-4.1.9 for booleans in structured headers
179-
headers.put(TIMED_OUT_HEADER, List.of("?1"));
180+
bodyHeaders.put(TIMED_OUT_HEADER, List.of("?1"));
180181
}
181182
}
182183

@@ -220,42 +221,77 @@ protected Map<String, List<String>> getMetadata() {
220221
}
221222

222223
/**
223-
* Adds a new header with the given key.
224+
* Adds a new header with the given key that is part of the response body and http headers.
224225
* This method will replace existing header if a header with the same key already exists
225226
*/
226-
public void addHeader(String key, List<String> value) {
227+
public void addBodyHeader(String key, List<String> value) {
227228
// we need to enforce this otherwise bw comp doesn't work properly, as "es." was the previous criteria to split headers in two sets
228229
if (key.startsWith("es.")) {
229230
throw new IllegalArgumentException("exception headers must not start with [es.], found [" + key + "] instead");
230231
}
231-
this.headers.put(key, value);
232+
this.bodyHeaders.put(key, value);
232233
}
233234

234235
/**
235-
* Adds a new header with the given key.
236+
* Adds a new header with the given key that is part of the response body and http headers.
236237
* This method will replace existing header if a header with the same key already exists
237238
*/
238-
public void addHeader(String key, String... value) {
239-
addHeader(key, Arrays.asList(value));
239+
public void addBodyHeader(String key, String... value) {
240+
addBodyHeader(key, Arrays.asList(value));
240241
}
241242

242243
/**
243-
* Returns a set of all header keys on this exception
244+
* Returns a set of all body header keys on this exception
244245
*/
245-
public Set<String> getHeaderKeys() {
246-
return headers.keySet();
246+
public Set<String> getBodyHeaderKeys() {
247+
return bodyHeaders.keySet();
247248
}
248249

249250
/**
250-
* Returns the list of header values for the given key or {@code null} if no header for the
251+
* Returns the list of body header values for the given key or {@code null} if no header for the
251252
* given key exists.
252253
*/
253-
public List<String> getHeader(String key) {
254-
return headers.get(key);
254+
public List<String> getBodyHeader(String key) {
255+
return bodyHeaders.get(key);
255256
}
256257

257-
protected Map<String, List<String>> getHeaders() {
258-
return headers;
258+
protected Map<String, List<String>> getBodyHeaders() {
259+
return bodyHeaders;
260+
}
261+
262+
/**
263+
* Adds a new http header with the given key.
264+
* This method will replace existing http header if a header with the same key already exists
265+
*/
266+
public void addHttpHeader(String key, List<String> value) {
267+
this.httpHeaders.put(key, value);
268+
}
269+
270+
/**
271+
* Adds a new http header with the given key.
272+
* This method will replace existing http header if a header with the same key already exists
273+
*/
274+
public void addHttpHeader(String key, String... value) {
275+
this.httpHeaders.put(key, List.of(value));
276+
}
277+
278+
/**
279+
* Returns a set of all body header keys on this exception
280+
*/
281+
public Set<String> getHttpHeaderKeys() {
282+
return httpHeaders.keySet();
283+
}
284+
285+
/**
286+
* Returns the list of http header values for the given key or {@code null} if no header for the
287+
* given key exists.
288+
*/
289+
public List<String> getHttpHeader(String key) {
290+
return httpHeaders.get(key);
291+
}
292+
293+
protected Map<String, List<String>> getHttpHeaders() {
294+
return httpHeaders;
259295
}
260296

261297
/**
@@ -335,7 +371,7 @@ private static Writer<Throwable> createNestingFunction(int thisLevel, Runnable n
335371
protected void writeTo(StreamOutput out, Writer<Throwable> nestedExceptionsWriter) throws IOException {
336372
out.writeOptionalString(this.getMessage());
337373
nestedExceptionsWriter.write(out, this);
338-
out.writeMap(headers, StreamOutput::writeStringCollection);
374+
out.writeMap(bodyHeaders, StreamOutput::writeStringCollection);
339375
out.writeMap(metadata, StreamOutput::writeStringCollection);
340376
}
341377

@@ -384,7 +420,7 @@ protected XContentBuilder toXContent(XContentBuilder builder, Params params, int
384420
if (ex != this) {
385421
generateThrowableXContent(builder, params, this, nestedLevel);
386422
} else {
387-
innerToXContent(builder, params, this, headers, metadata, getCause(), nestedLevel);
423+
innerToXContent(builder, params, this, bodyHeaders, metadata, getCause(), nestedLevel);
388424
}
389425
return builder;
390426
}
@@ -581,7 +617,7 @@ public static ElasticsearchException innerFromXContent(XContentParser parser, bo
581617
e.addMetadata("es." + entry.getKey(), entry.getValue());
582618
}
583619
for (Map.Entry<String, List<String>> header : headers.entrySet()) {
584-
e.addHeader(header.getKey(), header.getValue());
620+
e.addBodyHeader(header.getKey(), header.getValue());
585621
}
586622

587623
// Adds root causes as suppressed exception. This way they are not lost

server/src/main/java/org/elasticsearch/action/bulk/FailureStoreDocumentConverter.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -115,22 +115,23 @@ private static XContentBuilder createSource(IndexRequest source, Exception excep
115115
// we can't instantiate it in tests, so we'll have to check for the headers directly.
116116
var ingestException = ExceptionsHelper.<ElasticsearchException>unwrapCausesAndSuppressed(
117117
exception,
118-
t -> t instanceof ElasticsearchException e && Sets.haveNonEmptyIntersection(e.getHeaderKeys(), INGEST_EXCEPTION_HEADERS)
118+
t -> t instanceof ElasticsearchException e
119+
&& Sets.haveNonEmptyIntersection(e.getBodyHeaderKeys(), INGEST_EXCEPTION_HEADERS)
119120
).orElse(null);
120121
if (ingestException != null) {
121-
if (ingestException.getHeaderKeys().contains(PIPELINE_ORIGIN_EXCEPTION_HEADER)) {
122-
List<String> pipelineOrigin = ingestException.getHeader(PIPELINE_ORIGIN_EXCEPTION_HEADER);
122+
if (ingestException.getBodyHeaderKeys().contains(PIPELINE_ORIGIN_EXCEPTION_HEADER)) {
123+
List<String> pipelineOrigin = ingestException.getBodyHeader(PIPELINE_ORIGIN_EXCEPTION_HEADER);
123124
Collections.reverse(pipelineOrigin);
124125
if (pipelineOrigin.isEmpty() == false) {
125126
builder.field("pipeline_trace", pipelineOrigin);
126127
builder.field("pipeline", pipelineOrigin.get(pipelineOrigin.size() - 1));
127128
}
128129
}
129-
if (ingestException.getHeaderKeys().contains(PROCESSOR_TAG_EXCEPTION_HEADER)) {
130-
builder.field("processor_tag", ingestException.getHeader(PROCESSOR_TAG_EXCEPTION_HEADER).get(0));
130+
if (ingestException.getBodyHeaderKeys().contains(PROCESSOR_TAG_EXCEPTION_HEADER)) {
131+
builder.field("processor_tag", ingestException.getBodyHeader(PROCESSOR_TAG_EXCEPTION_HEADER).get(0));
131132
}
132-
if (ingestException.getHeaderKeys().contains(PROCESSOR_TYPE_EXCEPTION_HEADER)) {
133-
builder.field("processor_type", ingestException.getHeader(PROCESSOR_TYPE_EXCEPTION_HEADER).get(0));
133+
if (ingestException.getBodyHeaderKeys().contains(PROCESSOR_TYPE_EXCEPTION_HEADER)) {
134+
builder.field("processor_type", ingestException.getBodyHeader(PROCESSOR_TYPE_EXCEPTION_HEADER).get(0));
134135
}
135136
}
136137
}

server/src/main/java/org/elasticsearch/action/search/SearchPhaseExecutionException.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ protected XContentBuilder toXContent(XContentBuilder builder, Params params, int
128128
// We don't have a cause when all shards failed, but we do have shards failures so we can "guess" a cause
129129
// (see {@link #getCause()}). Here, we use super.getCause() because we don't want the guessed exception to
130130
// be rendered twice (one in the "cause" field, one in "failed_shards")
131-
innerToXContent(builder, params, this, getHeaders(), getMetadata(), super.getCause(), nestedLevel);
131+
innerToXContent(builder, params, this, getBodyHeaders(), getMetadata(), super.getCause(), nestedLevel);
132132
}
133133
return builder;
134134
}

server/src/main/java/org/elasticsearch/common/io/stream/NotSerializableExceptionWrapper.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ public NotSerializableExceptionWrapper(Throwable other) {
3636
addSuppressed(otherSuppressed);
3737
}
3838
if (other instanceof ElasticsearchException ex) {
39-
for (String key : ex.getHeaderKeys()) {
40-
this.addHeader(key, ex.getHeader(key));
39+
for (String key : ex.getBodyHeaderKeys()) {
40+
this.addBodyHeader(key, ex.getBodyHeader(key));
4141
}
4242
for (String key : ex.getMetadataKeys()) {
4343
this.addMetadata(key, ex.getMetadata(key));

server/src/main/java/org/elasticsearch/ingest/CompoundProcessor.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -292,9 +292,9 @@ void executeOnFailure(
292292
}
293293

294294
private static void putFailureMetadata(IngestDocument ingestDocument, ElasticsearchException cause) {
295-
List<String> processorTypeHeader = cause.getHeader(PROCESSOR_TYPE_EXCEPTION_HEADER);
296-
List<String> processorTagHeader = cause.getHeader(PROCESSOR_TAG_EXCEPTION_HEADER);
297-
List<String> processorOriginHeader = cause.getHeader(PIPELINE_ORIGIN_EXCEPTION_HEADER);
295+
List<String> processorTypeHeader = cause.getBodyHeader(PROCESSOR_TYPE_EXCEPTION_HEADER);
296+
List<String> processorTagHeader = cause.getBodyHeader(PROCESSOR_TAG_EXCEPTION_HEADER);
297+
List<String> processorOriginHeader = cause.getBodyHeader(PIPELINE_ORIGIN_EXCEPTION_HEADER);
298298
String failedProcessorType = (processorTypeHeader != null) ? processorTypeHeader.get(0) : null;
299299
String failedProcessorTag = (processorTagHeader != null) ? processorTagHeader.get(0) : null;
300300
String failedPipelineId = (processorOriginHeader != null) ? processorOriginHeader.get(0) : null;
@@ -316,24 +316,24 @@ private static void removeFailureMetadata(IngestDocument ingestDocument) {
316316
}
317317

318318
static IngestProcessorException newCompoundProcessorException(Exception e, Processor processor, IngestDocument document) {
319-
if (e instanceof IngestProcessorException ipe && ipe.getHeader(PROCESSOR_TYPE_EXCEPTION_HEADER) != null) {
319+
if (e instanceof IngestProcessorException ipe && ipe.getBodyHeader(PROCESSOR_TYPE_EXCEPTION_HEADER) != null) {
320320
return ipe;
321321
}
322322

323323
IngestProcessorException exception = new IngestProcessorException(e);
324324

325325
String processorType = processor.getType();
326326
if (processorType != null) {
327-
exception.addHeader(PROCESSOR_TYPE_EXCEPTION_HEADER, processorType);
327+
exception.addBodyHeader(PROCESSOR_TYPE_EXCEPTION_HEADER, processorType);
328328
}
329329
String processorTag = processor.getTag();
330330
if (processorTag != null) {
331-
exception.addHeader(PROCESSOR_TAG_EXCEPTION_HEADER, processorTag);
331+
exception.addBodyHeader(PROCESSOR_TAG_EXCEPTION_HEADER, processorTag);
332332
}
333333
if (document != null) {
334334
List<String> pipelineStack = document.getPipelineStack();
335335
if (pipelineStack.isEmpty() == false) {
336-
exception.addHeader(PIPELINE_ORIGIN_EXCEPTION_HEADER, pipelineStack);
336+
exception.addBodyHeader(PIPELINE_ORIGIN_EXCEPTION_HEADER, pipelineStack);
337337
}
338338
}
339339

server/src/main/java/org/elasticsearch/ingest/IngestPipelineException.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public class IngestPipelineException extends ElasticsearchException implements E
2828

2929
IngestPipelineException(final String pipeline, final Exception cause) {
3030
super(cause);
31-
this.addHeader(PIPELINE_ORIGIN_EXCEPTION_HEADER, List.of(pipeline));
31+
this.addBodyHeader(PIPELINE_ORIGIN_EXCEPTION_HEADER, List.of(pipeline));
3232
}
3333

3434
public IngestPipelineException(final StreamInput in) throws IOException {

server/src/main/java/org/elasticsearch/ingest/IngestService.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1604,8 +1604,8 @@ public synchronized void reloadPipeline(ProjectId projectId, String id) throws E
16041604
}
16051605

16061606
private static Pipeline substitutePipeline(String id, ElasticsearchParseException e) {
1607-
String tag = e.getHeaderKeys().contains("processor_tag") ? e.getHeader("processor_tag").get(0) : null;
1608-
String type = e.getHeaderKeys().contains("processor_type") ? e.getHeader("processor_type").get(0) : "unknown";
1607+
String tag = e.getBodyHeaderKeys().contains("processor_tag") ? e.getBodyHeader("processor_tag").get(0) : null;
1608+
String type = e.getBodyHeaderKeys().contains("processor_type") ? e.getBodyHeader("processor_type").get(0) : "unknown";
16091609
String errorMessage = "pipeline with id [" + id + "] could not be loaded, caused by [" + e.getDetailedMessage() + "]";
16101610
Processor failureProcessor = new AbstractProcessor(tag, "this is a placeholder processor") {
16111611
@Override

server/src/main/java/org/elasticsearch/rest/RestResponse.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,12 +212,16 @@ static RestResponse createSimpleErrorResponse(RestChannel channel, RestStatus st
212212
}
213213

214214
public void copyHeaders(ElasticsearchException ex) {
215-
Set<String> headerKeySet = ex.getHeaderKeys();
215+
Set<String> bodyHeaderKeySet = ex.getBodyHeaderKeys();
216+
Set<String> httpHeaderKeySet = ex.getHttpHeaderKeys();
216217
if (customHeaders == null) {
217-
customHeaders = Maps.newMapWithExpectedSize(headerKeySet.size());
218+
customHeaders = Maps.newMapWithExpectedSize(bodyHeaderKeySet.size() + httpHeaderKeySet.size());
218219
}
219-
for (String key : headerKeySet) {
220-
customHeaders.computeIfAbsent(key, k -> new ArrayList<>()).addAll(ex.getHeader(key));
220+
for (String key : bodyHeaderKeySet) {
221+
customHeaders.computeIfAbsent(key, k -> new ArrayList<>()).addAll(ex.getBodyHeader(key));
222+
}
223+
for (String key : httpHeaderKeySet) {
224+
customHeaders.computeIfAbsent(key, k -> new ArrayList<>()).addAll(ex.getHttpHeader(key));
221225
}
222226
}
223227

0 commit comments

Comments
 (0)