diff --git a/core/src/main/java/com/predic8/membrane/core/exchange/AbstractExchange.java b/core/src/main/java/com/predic8/membrane/core/exchange/AbstractExchange.java index 309af637ce..1f835a0aa9 100644 --- a/core/src/main/java/com/predic8/membrane/core/exchange/AbstractExchange.java +++ b/core/src/main/java/com/predic8/membrane/core/exchange/AbstractExchange.java @@ -329,11 +329,7 @@ public long getResponseContentLength() { return length; if (getResponse().getBody().isRead()) { - try { - return getResponse().getBody().getLength(); - } catch (IOException e) { - log.error("", e); - } + return getResponse().getBody().getLength(); } return -1; diff --git a/core/src/main/java/com/predic8/membrane/core/exchangestore/FileExchangeStore.java b/core/src/main/java/com/predic8/membrane/core/exchangestore/FileExchangeStore.java index 4d65f16cc7..df3f413468 100644 --- a/core/src/main/java/com/predic8/membrane/core/exchangestore/FileExchangeStore.java +++ b/core/src/main/java/com/predic8/membrane/core/exchangestore/FileExchangeStore.java @@ -323,11 +323,7 @@ public void bodyRequested(AbstractBody body) { } public void bodyComplete(AbstractBody body) { - try { - snapInternal(exc, flow, getBody(body)); - } catch (IOException e) { - throw new RuntimeException(e); - } + snapInternal(exc, flow, getBody(body)); } } } diff --git a/core/src/main/java/com/predic8/membrane/core/http/AbstractBody.java b/core/src/main/java/com/predic8/membrane/core/http/AbstractBody.java index f4697b4619..e3cd248651 100644 --- a/core/src/main/java/com/predic8/membrane/core/http/AbstractBody.java +++ b/core/src/main/java/com/predic8/membrane/core/http/AbstractBody.java @@ -42,6 +42,10 @@ * Streams do not have to be read completely. Accessing the body from multiple * threads is illegal. Using a Body Stream after the Body as been accessed by * someone else (using streams or not) is illegal. + *

+ * Public instance methods must not throw {@link IOException}s. Throw an + * unchecked {@link ReadingBodyException} or {@link WritingBodyException} instead. + * (This is enforced by the BodyDoesntThrowIOExceptionTest .) */ public abstract class AbstractBody { private static final Logger log = LoggerFactory.getLogger(AbstractBody.class.getName()); @@ -53,7 +57,7 @@ public abstract class AbstractBody { protected final List observers = new ArrayList<>(1); private boolean wasStreamed = false; - public void read() throws IOException { + public void read() { if (read) return; @@ -64,11 +68,15 @@ public void read() throws IOException { observer.bodyRequested(this); chunks.clear(); - readLocal(); + try { + readLocal(); + } catch (IOException e) { + throw new ReadingBodyException(e); + } markAsRead(); } - public void discard() throws IOException { + public void discard() { read(); } @@ -105,7 +113,7 @@ protected void markAsRead() { * {@link #getContent()}. If you do not need the body as one single byte[], * you should therefore use {@link #getContentAsStream()} instead. */ - public byte[] getContent() throws IOException { + public byte[] getContent() { if (wasStreamed) throw new IllegalStateException("Cannot read body after it was streamed."); read(); @@ -117,25 +125,29 @@ public byte[] getContent() throws IOException { return content; } - public InputStream getContentAsStream() throws IOException { + public InputStream getContentAsStream() { if (wasStreamed) throw new IllegalStateException("Cannot read body after it was streamed."); read(); return new BodyInputStream(chunks); } - public void write(AbstractBodyTransferrer out, boolean retainCopy) throws IOException { - if (!read && !retainCopy) { - if (wasStreamed) - log.warn("Streaming the body twice will not work."); - for (MessageObserver observer : observers) - observer.bodyRequested(this); - wasStreamed = true; - writeStreamed(out); - return; + public void write(AbstractBodyTransferrer out, boolean retainCopy) { + try { + if (!read && !retainCopy) { + if (wasStreamed) + log.warn("Streaming the body twice will not work."); + for (MessageObserver observer : observers) + observer.bodyRequested(this); + wasStreamed = true; + writeStreamed(out); + return; + } + + writeAlreadyRead(out); + } catch (IOException e) { + throw new WritingBodyException(e); } - - writeAlreadyRead(out); } protected abstract void writeAlreadyRead(AbstractBodyTransferrer out) throws IOException; @@ -145,7 +157,7 @@ public void write(AbstractBodyTransferrer out, boolean retainCopy) throws IOExce /** * Is called when there are no observers that need to read the body. Streams the body without reading it */ - protected abstract void writeStreamed(AbstractBodyTransferrer out) throws IOException; + protected abstract void writeStreamed(AbstractBodyTransferrer out); /** * Warning: Calling this method will trigger reading the body from the client, disabling "streaming". @@ -153,7 +165,7 @@ public void write(AbstractBodyTransferrer out, boolean retainCopy) throws IOExce * * @return the length of the return value of {@link #getContent()} */ - public int getLength() throws IOException { + public int getLength() { read(); int length = 0; @@ -182,10 +194,14 @@ public int getLength() throws IOException { * 0 * */ - public byte[] getRaw() throws IOException { + public byte[] getRaw() { read(); - return getRawLocal(); - } + try { + return getRawLocal(); + } catch (IOException e) { + throw new ReadingBodyException(e); + } + } protected abstract byte[] getRawLocal() throws IOException; @@ -204,9 +220,9 @@ public String toString() { } try { return new String(getRaw(), UTF_8); - } catch (IOException e) { - log.error("", e); - return "Error in body: " + e; + } catch (ReadingBodyException e) { + log.error(e.getMessage()); + return "Error in body: " + e.getMessage(); } } diff --git a/core/src/main/java/com/predic8/membrane/core/http/Body.java b/core/src/main/java/com/predic8/membrane/core/http/Body.java index 18d5acce6b..5a53a7d7b5 100644 --- a/core/src/main/java/com/predic8/membrane/core/http/Body.java +++ b/core/src/main/java/com/predic8/membrane/core/http/Body.java @@ -81,7 +81,7 @@ protected void readLocal() throws IOException { } } - public void discard() throws IOException { + public void discard() { if (read) return; if (wasStreamed()) @@ -90,8 +90,12 @@ public void discard() throws IOException { for (MessageObserver observer : observers) observer.bodyRequested(this); - skipBodyContent(); - } + try { + skipBodyContent(); + } catch (IOException e) { + throw new ReadingBodyException(e); + } + } private void skipBodyContent() throws IOException { byte[] buffer = null; @@ -148,25 +152,39 @@ protected void writeNotRead(AbstractBodyTransferrer out) throws IOException { } @Override - protected void writeStreamed(AbstractBodyTransferrer out) throws IOException { + protected void writeStreamed(AbstractBodyTransferrer out) { byte[] buffer = new byte[BUFFER_SIZE]; long totalLength = 0; int length; chunks.clear(); - while ((this.length > totalLength || this.length == -1) && (length = inputStream.read(buffer)) > 0) { - totalLength += length; + while (true) { + try { + if (!((this.length > totalLength || this.length == -1) && (length = inputStream.read(buffer)) > 0)) + break; + } catch (IOException e) { + throw new ReadingBodyException(e); + } + totalLength += length; streamedLength += length; - out.write(buffer, 0, length); + try { + out.write(buffer, 0, length); + } catch (IOException e) { + throw new WritingBodyException(e); + } for (MessageObserver observer : observers) observer.bodyChunk(buffer, 0, length); } - out.finish(null); - markAsRead(); + try { + out.finish(null); + } catch (IOException e) { + throw new WritingBodyException(e); + } + markAsRead(); } @Override - public int getLength() throws IOException { + public int getLength() { if (wasStreamed()) return (int)streamedLength; return super.getLength(); diff --git a/core/src/main/java/com/predic8/membrane/core/http/BodyCollectingMessageObserver.java b/core/src/main/java/com/predic8/membrane/core/http/BodyCollectingMessageObserver.java index 26c39e66ea..3afe75e4e6 100644 --- a/core/src/main/java/com/predic8/membrane/core/http/BodyCollectingMessageObserver.java +++ b/core/src/main/java/com/predic8/membrane/core/http/BodyCollectingMessageObserver.java @@ -63,7 +63,7 @@ public void bodyChunk(Chunk chunk) { storedSize += chunk.getLength(); } - public AbstractBody getBody(AbstractBody body) throws IOException { + public AbstractBody getBody(AbstractBody body) { if (!body.wasStreamed()) { return body; } diff --git a/core/src/main/java/com/predic8/membrane/core/http/ChunkedBody.java b/core/src/main/java/com/predic8/membrane/core/http/ChunkedBody.java index ef28d87edd..bc61ed3c85 100644 --- a/core/src/main/java/com/predic8/membrane/core/http/ChunkedBody.java +++ b/core/src/main/java/com/predic8/membrane/core/http/ChunkedBody.java @@ -109,17 +109,25 @@ public static int readChunkSize(InputStream in) throws IOException { } @Override - public void read() throws IOException { - if (bodyObserved && !bodyComplete) - ByteUtil.readStream(getContentAsStream()); - bodyObserved = true; - super.read(); + public void read() { + try { + if (bodyObserved && !bodyComplete) + ByteUtil.readStream(getContentAsStream()); + bodyObserved = true; + super.read(); + } catch (IOException e) { + throw new ReadingBodyException(e); + } } @Override - public void write(AbstractBodyTransferrer out, boolean retainCopy) throws IOException { - if (bodyObserved && !bodyComplete) - ByteUtil.readStream(getContentAsStream()); + public void write(AbstractBodyTransferrer out, boolean retainCopy) { + try { + if (bodyObserved && !bodyComplete) + ByteUtil.readStream(getContentAsStream()); + } catch (IOException e) { + throw new ReadingBodyException(e); + } super.write(out, retainCopy); } @@ -140,7 +148,7 @@ protected void readLocal() throws IOException { } @Override - public void discard() throws IOException { + public void discard() { if (read) return; if (wasStreamed()) @@ -149,8 +157,12 @@ public void discard() throws IOException { for (MessageObserver observer : observers) observer.bodyRequested(this); - readChunksAndDrop(inputStream); - trailer = readTrailer(inputStream); + try { + readChunksAndDrop(inputStream); + trailer = readTrailer(inputStream); + } catch (IOException e) { + throw new ReadingBodyException(e); + } markAsRead(); } @@ -212,22 +224,34 @@ protected void writeNotRead(AbstractBodyTransferrer out) throws IOException { } @Override - protected void writeStreamed(AbstractBodyTransferrer out) throws IOException { + protected void writeStreamed(AbstractBodyTransferrer out) { log.debug("writeStreamed"); int chunkSize; - while ((chunkSize = readChunkSize(inputStream)) > 0) { - Chunk chunk = new Chunk(readByteArray(inputStream, chunkSize)); - out.write(chunk); - for (MessageObserver observer : observers) - observer.bodyChunk(chunk); - //noinspection ResultOfMethodCallIgnored - inputStream.read(); // CR - //noinspection ResultOfMethodCallIgnored - inputStream.read(); // LF - lengthStreamed += chunkSize; + try { + while ((chunkSize = readChunkSize(inputStream)) > 0) { + Chunk chunk = new Chunk(readByteArray(inputStream, chunkSize)); + try { + out.write(chunk); + } catch (IOException e) { + throw new WritingBodyException(e); + } + for (MessageObserver observer : observers) + observer.bodyChunk(chunk); + //noinspection ResultOfMethodCallIgnored + inputStream.read(); // CR + //noinspection ResultOfMethodCallIgnored + inputStream.read(); // LF + lengthStreamed += chunkSize; + } + trailer = readTrailer(inputStream); + } catch (IOException e) { // note that we only want to catch the IOExceptions associated to *reading* the body + throw new ReadingBodyException(e); + } + try { + out.finish(trailer); + } catch (IOException e) { + throw new WritingBodyException(e); } - trailer = readTrailer(inputStream); - out.finish(trailer); markAsRead(); } @@ -281,7 +305,7 @@ protected void writeAlreadyRead(AbstractBodyTransferrer out) throws IOException } @Override - public int getLength() throws IOException { + public int getLength() { if (wasStreamed()) return (int) lengthStreamed; return super.getLength(); diff --git a/core/src/main/java/com/predic8/membrane/core/http/EmptyBody.java b/core/src/main/java/com/predic8/membrane/core/http/EmptyBody.java index ae4ab83f9f..bb74e1acc7 100644 --- a/core/src/main/java/com/predic8/membrane/core/http/EmptyBody.java +++ b/core/src/main/java/com/predic8/membrane/core/http/EmptyBody.java @@ -44,7 +44,7 @@ protected void writeNotRead(AbstractBodyTransferrer out) throws IOException { } @Override - protected void writeStreamed(AbstractBodyTransferrer out) throws IOException { + protected void writeStreamed(AbstractBodyTransferrer out) { //ignore } @@ -54,8 +54,12 @@ protected byte[] getRawLocal() throws IOException { } @Override - public void write(AbstractBodyTransferrer out, boolean retainCopy) throws IOException { - out.finish(null); + public void write(AbstractBodyTransferrer out, boolean retainCopy) { + try { + out.finish(null); + } catch (IOException e) { + throw new WritingBodyException(e); + } markAsRead(); } diff --git a/core/src/main/java/com/predic8/membrane/core/http/Message.java b/core/src/main/java/com/predic8/membrane/core/http/Message.java index c3a0391a67..e394841bbd 100644 --- a/core/src/main/java/com/predic8/membrane/core/http/Message.java +++ b/core/src/main/java/com/predic8/membrane/core/http/Message.java @@ -74,8 +74,8 @@ public void readBody() throws IOException { public void discardBody() { try { body.discard(); - } catch (IOException e) { - log.debug("Error discarding body. Can be ignored.", e); + } catch (ReadingBodyException e) { + log.debug("Error discarding body ({}). Can be ignored.", e.getMessage()); } } @@ -94,13 +94,8 @@ public AbstractBody getBody() { * * @see AbstractBody#getContentAsStream() */ - public InputStream getBodyAsStream() { - try { - return body.getContentAsStream(); - } catch (IOException e) { - log.error("Could not get body as stream", e); - throw new ReadingBodyException(e); - } + public InputStream getBodyAsStream() throws ReadingBodyException { + return body.getContentAsStream(); } /** @@ -110,7 +105,7 @@ public InputStream getBodyAsStream() { * *

Supports streaming: The HTTP message does not have to be completely received yet for this method to return.

*/ - public InputStream getBodyAsStreamDecoded() { + public InputStream getBodyAsStreamDecoded() throws ReadingBodyException { // TODO: this logic should be split up into configurable decoding modules // TODO: decoding result should be cached try { @@ -118,6 +113,8 @@ public InputStream getBodyAsStreamDecoded() { if (m != null) return m.getBodyAsStream(); // we know decoding is not necessary any more return MessageUtil.getContentAsStream(this); + } catch (ReadingBodyException e) { + throw e; } catch (Exception e) { log.error("Could not decode body stream", e); throw new RuntimeException("Could not decode body stream", e); @@ -136,7 +133,7 @@ public InputStream getBodyAsStreamDecoded() { * * @return the message's body as a Java String. */ - public String getBodyAsStringDecoded() { + public String getBodyAsStringDecoded() throws ReadingBodyException { try { return new String(MessageUtil.getContent(this), getCharsetOrDefault()); } catch (Exception e) { @@ -148,7 +145,7 @@ public String getBodyAsStringDecoded() { * Sets the body. * Does NOT adjust the header fields (Content-Length etc.): Use {@link #setBodyContent(byte[])} instead. */ - public void setBody(AbstractBody b) { + public void setBody(AbstractBody b) throws ReadingBodyException { discardBody(); // Make sure remaining bytes are read from original body's input stream body = b; } @@ -156,7 +153,7 @@ public void setBody(AbstractBody b) { /** * Sets the body. Also adjusts the header fields (Content-Length, Content-Encoding, Transfer-Encoding). */ - public void setBodyContent(byte[] content) { + public void setBodyContent(byte[] content) throws ReadingBodyException { discardBody(); // Make sure remaining bytes are read from original body's input stream body = new Body(content); header.removeFields(CONTENT_ENCODING); @@ -295,12 +292,13 @@ public String getName() { return "message"; } - public boolean isBodyEmpty() throws IOException { + public boolean isBodyEmpty() throws ReadingBodyException { if (header.hasContentLength()) return header.getContentLength() == 0; - if (getBody().read) + if (getBody().read) { return getBody().getLength() == 0; + } return getBody() instanceof EmptyBody; } @@ -351,14 +349,10 @@ public void addObserver(MessageObserver observer) { } public int estimateHeapSize() { - try { - return 100 + - (header != null ? header.estimateHeapSize() : 0) + - (body != null ? body.isRead() ? body.getLength() : 0 : 0) + - (errorMessage != null ? 2*errorMessage.length() : 0); - } catch (IOException e) { - throw new RuntimeException(e); - } + return 100 + + (header != null ? header.estimateHeapSize() : 0) + + (body != null ? body.isRead() ? body.getLength() : 0 : 0) + + (errorMessage != null ? 2*errorMessage.length() : 0); } public abstract T createSnapshot(Runnable bodyUpdatedCallback, BodyCollectingMessageObserver.Strategy strategy, long limit) throws Exception; @@ -404,7 +398,7 @@ public void bodyRequested(AbstractBody body) { public void bodyComplete(AbstractBody body2) { try { result.setBody(getBody(body2)); - } catch (IOException e) { + } catch (ReadingBodyException e) { throw new RuntimeException(e); } try { diff --git a/core/src/main/java/com/predic8/membrane/core/http/ReadingBodyException.java b/core/src/main/java/com/predic8/membrane/core/http/ReadingBodyException.java index afba45c5c9..f4f20bfefb 100644 --- a/core/src/main/java/com/predic8/membrane/core/http/ReadingBodyException.java +++ b/core/src/main/java/com/predic8/membrane/core/http/ReadingBodyException.java @@ -14,6 +14,13 @@ package com.predic8.membrane.core.http; +/** + * Indicates that an error occurred while reading the body of a message. + * + * The 'message' should already be enough to indicate the error. + * + * (No need to use {@link com.predic8.membrane.core.util.ExceptionUtil#concatMessageAndCauseMessages(Throwable)}.) + */ public class ReadingBodyException extends RuntimeException { public ReadingBodyException(Exception e) { super(e); diff --git a/core/src/main/java/com/predic8/membrane/core/http/WritingBodyException.java b/core/src/main/java/com/predic8/membrane/core/http/WritingBodyException.java new file mode 100644 index 0000000000..615809c41a --- /dev/null +++ b/core/src/main/java/com/predic8/membrane/core/http/WritingBodyException.java @@ -0,0 +1,28 @@ +/* Copyright 2025 predic8 GmbH, www.predic8.com + + Licensed 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 com.predic8.membrane.core.http; + +/** + * Indicates that an error occurred while writing the body of a message. + * + * The 'message' should already be enough to indicate the error. + * + * (No need to use {@link com.predic8.membrane.core.util.ExceptionUtil#concatMessageAndCauseMessages(Throwable)}.) + */ +public class WritingBodyException extends RuntimeException { + public WritingBodyException(Exception e) { + super(e); + } +} diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/balancer/BalancerHealthMonitor.java b/core/src/main/java/com/predic8/membrane/core/interceptor/balancer/BalancerHealthMonitor.java index d024574b6d..d5702bcdd3 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/balancer/BalancerHealthMonitor.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/balancer/BalancerHealthMonitor.java @@ -17,6 +17,7 @@ import com.predic8.membrane.annot.*; import com.predic8.membrane.core.*; import com.predic8.membrane.core.exchange.*; +import com.predic8.membrane.core.http.ReadingBodyException; import com.predic8.membrane.core.interceptor.balancer.Node.*; import com.predic8.membrane.core.transport.http.*; import com.predic8.membrane.core.transport.http.client.*; @@ -134,8 +135,8 @@ private static Status getStatus(Node node, Exchange exc) { return DOWN; try { exc.getResponse().getBody().read(); - } catch (IOException e) { - log.debug("Calling health endpoint failed: {} {}", exc, concatMessageAndCauseMessages(e)); + } catch (ReadingBodyException e) { + log.debug("Calling health endpoint failed: {} {}", exc, e.getMessage()); return DOWN; } int status = exc.getResponse().getStatusCode(); diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/beautifier/BeautifierInterceptor.java b/core/src/main/java/com/predic8/membrane/core/interceptor/beautifier/BeautifierInterceptor.java index 914bda8de6..26f443f903 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/beautifier/BeautifierInterceptor.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/beautifier/BeautifierInterceptor.java @@ -54,8 +54,8 @@ private Outcome handleInternal(Message msg) { try { if (msg.isBodyEmpty()) return CONTINUE; - } catch (IOException e) { - log.error("", e); + } catch (ReadingBodyException e) { + log.error("Could not beautify body ({}), but continuing flow.", e.getMessage()); return CONTINUE; } diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/flow/CallInterceptor.java b/core/src/main/java/com/predic8/membrane/core/interceptor/flow/CallInterceptor.java index 36a6aac2b7..5e8b9e1844 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/flow/CallInterceptor.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/flow/CallInterceptor.java @@ -124,11 +124,7 @@ private void setRequestBody(Request.Builder builder, Exchange exchange) { if (!methodShouldHaveBody(method)) { return; } - try { - builder.body(exchange.getRequest().getBody().getContent()); - } catch (IOException e) { - throw new RuntimeException("Error setting request body", e); - } + builder.body(exchange.getRequest().getBody().getContent()); } private static boolean methodShouldHaveBody(String method) { diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/log/LogInterceptor.java b/core/src/main/java/com/predic8/membrane/core/interceptor/log/LogInterceptor.java index 7a6711ad5a..076712eb80 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/log/LogInterceptor.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/log/LogInterceptor.java @@ -107,7 +107,7 @@ private void logMessage(Exchange exc, Flow flow) { try { if (!body || msg.isBodyEmpty()) return; - } catch (IOException e) { + } catch (ReadingBodyException e) { writeLog("Error accessing body: " + e.getMessage()); return; } diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/log/access/AccessLogInterceptorService.java b/core/src/main/java/com/predic8/membrane/core/interceptor/log/access/AccessLogInterceptorService.java index 110e0a62e3..720314cb3a 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/log/access/AccessLogInterceptorService.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/log/access/AccessLogInterceptorService.java @@ -17,6 +17,7 @@ import com.predic8.membrane.core.exchange.Exchange; import com.predic8.membrane.core.http.Message; +import com.predic8.membrane.core.http.ReadingBodyException; import com.predic8.membrane.core.interceptor.log.AdditionalVariable; import com.predic8.membrane.core.lang.spel.SpELExchangeEvaluationContext; import org.slf4j.Logger; @@ -134,7 +135,7 @@ private Supplier getPayLoadSize(Message msg) { return () -> { try { return msg.getBody().getLength(); - } catch (IOException e) { + } catch (ReadingBodyException e) { return defaultValue; } }; diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/ntlm/NtlmInterceptor.java b/core/src/main/java/com/predic8/membrane/core/interceptor/ntlm/NtlmInterceptor.java index 1fe530cd0f..b6a6e43614 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/ntlm/NtlmInterceptor.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/ntlm/NtlmInterceptor.java @@ -138,11 +138,7 @@ private String buildRequestUrl(Exchange exc) { } private void prepareStreamByEmptyingIt(Exchange exc) { - try { - exc.getResponse().getBody().getContent(); - } catch (IOException e) { - LOG.warn("",e); - } + exc.getResponse().getBody().getContent(); } @MCChildElement(order = 1) diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/rest/XML2HTTP.java b/core/src/main/java/com/predic8/membrane/core/interceptor/rest/XML2HTTP.java index 2b8820cf78..c907f434ab 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/rest/XML2HTTP.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/rest/XML2HTTP.java @@ -130,7 +130,7 @@ public static void unwrapMessageIfNecessary(Message message) { message.getHeader().clear(); if (!foundBody) message.setBodyContent(new byte[0]); - } catch (XMLStreamException | IOException | XML2HTTPException e) { + } catch (XMLStreamException | XML2HTTPException e) { log.error("Failed to unwrap XML-HTTP document for message.", e); } } diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/schemavalidation/ValidatorInterceptor.java b/core/src/main/java/com/predic8/membrane/core/interceptor/schemavalidation/ValidatorInterceptor.java index 748f1b45d0..342cd2814d 100644 --- a/core/src/main/java/com/predic8/membrane/core/interceptor/schemavalidation/ValidatorInterceptor.java +++ b/core/src/main/java/com/predic8/membrane/core/interceptor/schemavalidation/ValidatorInterceptor.java @@ -16,6 +16,7 @@ import com.predic8.membrane.annot.*; import com.predic8.membrane.core.exchange.*; +import com.predic8.membrane.core.http.ReadingBodyException; import com.predic8.membrane.core.interceptor.*; import com.predic8.membrane.core.interceptor.schemavalidation.json.*; import com.predic8.membrane.core.proxies.*; @@ -151,8 +152,8 @@ private Outcome handleInternal(Exchange exc, Flow flow) { try { if (exc.getMessage(flow).isBodyEmpty()) return CONTINUE; - } catch (IOException e) { - log.error("", e); + } catch (ReadingBodyException e) { + log.error("Validation failed: {}", e.getMessage()); internal(router.isProduction(),getDisplayName()) .addSubSee("io") .detail("Could not read message body") diff --git a/core/src/main/java/com/predic8/membrane/core/lang/jsonpath/JsonpathExchangeExpression.java b/core/src/main/java/com/predic8/membrane/core/lang/jsonpath/JsonpathExchangeExpression.java index a3b82117c3..5d0041d16c 100644 --- a/core/src/main/java/com/predic8/membrane/core/lang/jsonpath/JsonpathExchangeExpression.java +++ b/core/src/main/java/com/predic8/membrane/core/lang/jsonpath/JsonpathExchangeExpression.java @@ -18,6 +18,7 @@ import com.fasterxml.jackson.databind.exc.*; import com.jayway.jsonpath.*; import com.predic8.membrane.core.exchange.*; +import com.predic8.membrane.core.http.ReadingBodyException; import com.predic8.membrane.core.interceptor.Interceptor.*; import com.predic8.membrane.core.lang.*; import com.predic8.membrane.core.util.*; @@ -69,8 +70,8 @@ public T evaluate(Exchange exchange, Flow flow, Class type) { log.debug("Body is empty or Content-Type not JSON. Nothing to evaluate for expression: {}", expression); // Normal return resultForNoEvaluation(type); } - } catch (IOException e) { - log.error("Error checking if body is empty", e); + } catch (ReadingBodyException e) { + log.error("Error checking if body is empty: {}", e.getMessage()); return resultForNoEvaluation(type); } diff --git a/core/src/main/java/com/predic8/membrane/core/lang/spel/typeconverters/SpELBodyToStringTypeConverter.java b/core/src/main/java/com/predic8/membrane/core/lang/spel/typeconverters/SpELBodyToStringTypeConverter.java index 419bf38a4f..e6cf392557 100644 --- a/core/src/main/java/com/predic8/membrane/core/lang/spel/typeconverters/SpELBodyToStringTypeConverter.java +++ b/core/src/main/java/com/predic8/membrane/core/lang/spel/typeconverters/SpELBodyToStringTypeConverter.java @@ -25,7 +25,7 @@ public class SpELBodyToStringTypeConverter implements Converter Request getOpenapiValidatorRequest(Exchange exc) throws IOException, ParseException { + public static Request getOpenapiValidatorRequest(Exchange exc) throws ParseException { Request request = new Request<>(exc.getRequest().getMethod(), exc.getRequestURI()); for (HeaderField header : exc.getRequest().getHeader().getAllHeaderFields()) { request.getHeaders().put(header.getHeaderName().toString(), header.getValue()); @@ -244,7 +244,7 @@ public static Request getOpenapiValidatorRequest(Exchange ex return request; } - public static Response getOpenapiValidatorResponse(Exchange exc) throws ParseException, IOException { + public static Response getOpenapiValidatorResponse(Exchange exc) throws ParseException { Response response = Response.statusCode(exc.getResponse().getStatusCode()); for (HeaderField header : exc.getResponse().getHeader().getAllHeaderFields()) { diff --git a/core/src/main/java/com/predic8/membrane/core/proxies/StatisticCollector.java b/core/src/main/java/com/predic8/membrane/core/proxies/StatisticCollector.java index 73e2826fcf..d2a13e0d34 100644 --- a/core/src/main/java/com/predic8/membrane/core/proxies/StatisticCollector.java +++ b/core/src/main/java/com/predic8/membrane/core/proxies/StatisticCollector.java @@ -90,8 +90,8 @@ public void collectFrom(AbstractExchange exc) { totalBytesSent += requestBody.isRead() ? requestBody.getLength() : 0; AbstractBody responseBody = exc.getResponse().getBody(); totalBytesReceived += responseBody.isRead() ? responseBody.getLength() : 0; - } catch (IOException e) { - log.warn("", e); + } catch (ReadingBodyException e) { + log.warn("Could not count statistic: {}", e.getMessage()); } } diff --git a/core/src/main/java/com/predic8/membrane/core/transport/http/HttpServerHandler.java b/core/src/main/java/com/predic8/membrane/core/transport/http/HttpServerHandler.java index 9ebd047e2a..855e665956 100644 --- a/core/src/main/java/com/predic8/membrane/core/transport/http/HttpServerHandler.java +++ b/core/src/main/java/com/predic8/membrane/core/transport/http/HttpServerHandler.java @@ -315,9 +315,13 @@ private String getRemoteAddr(DNSCache dnsCache, InetAddress remoteAddr, String i * client, nothing will be done. (Allowing the HTTP connection state to skip * over the body transmission.) */ - private void removeBodyFromBuffer() throws IOException { - if (!exchange.getRequest().getHeader().is100ContinueExpected() || srcIn.available() > 0) { - exchange.getRequest().discardBody(); + private void removeBodyFromBuffer() throws ReadingBodyException { + try { + if (!exchange.getRequest().getHeader().is100ContinueExpected() || srcIn.available() > 0) { + exchange.getRequest().discardBody(); + } + } catch (IOException e) { + throw new ReadingBodyException(e); } } diff --git a/core/src/main/java/com/predic8/membrane/core/transport/http2/StreamInfo.java b/core/src/main/java/com/predic8/membrane/core/transport/http2/StreamInfo.java index 4466e5210d..fec2e5ae74 100644 --- a/core/src/main/java/com/predic8/membrane/core/transport/http2/StreamInfo.java +++ b/core/src/main/java/com/predic8/membrane/core/transport/http2/StreamInfo.java @@ -223,22 +223,35 @@ protected void writeNotRead(AbstractBodyTransferrer out) throws IOException { } @Override - protected void writeStreamed(AbstractBodyTransferrer out) throws IOException { + protected void writeStreamed(AbstractBodyTransferrer out) { chunks.clear(); while (true) { - DataFrame df = removeDataFrame(); + DataFrame df = null; + try { + df = removeDataFrame(); + } catch (IOException e) { + throw new ReadingBodyException(e); + } if (df == null) continue; int len = df.getDataLength(); if (len > 0) { - out.write(df.getContent(), df.getDataStartIndex(), len); + try { + out.write(df.getContent(), df.getDataStartIndex(), len); + } catch (IOException e) { + throw new WritingBodyException(e); + } streamedLength += len; } if (df.isEndStream()) break; } - out.finish(trailer); + try { + out.finish(trailer); + } catch (IOException e) { + throw new WritingBodyException(e); + } markAsRead(); } @@ -254,7 +267,7 @@ protected byte[] getRawLocal() throws IOException { } @Override - public int getLength() throws IOException { + public int getLength() { if (wasStreamed()) return streamedLength; return super.getLength(); diff --git a/core/src/main/java/com/predic8/membrane/core/util/ExceptionUtil.java b/core/src/main/java/com/predic8/membrane/core/util/ExceptionUtil.java index aa1bcd3a72..8173879c93 100644 --- a/core/src/main/java/com/predic8/membrane/core/util/ExceptionUtil.java +++ b/core/src/main/java/com/predic8/membrane/core/util/ExceptionUtil.java @@ -15,13 +15,29 @@ public class ExceptionUtil { + /** + * Concatenates the messages of all nested exceptions. + * + * This could be improved to make the resulting String more dense in case of repeated information parts. + * + * @param throwable the exception + * @return a String containing all messages of nested exceptions + */ public static String concatMessageAndCauseMessages(Throwable throwable) { StringBuilder sb = new StringBuilder(); + boolean causedBy = false; do { - sb.append(throwable.getMessage()); + boolean skip = throwable.getMessage() == null || sb.toString().contains(throwable.getMessage()); + if (!skip) { + if (causedBy) { + sb.append(" caused by: "); + causedBy = false; + } + sb.append(throwable.getMessage()); + } throwable = throwable.getCause(); - if (throwable != null) { - sb.append(" caused by: "); + if (throwable != null && !skip) { + causedBy = true; } } while (throwable != null); return sb.toString(); diff --git a/core/src/main/java/com/predic8/membrane/core/util/MessageUtil.java b/core/src/main/java/com/predic8/membrane/core/util/MessageUtil.java index 56926ffca4..ff5950ab0f 100644 --- a/core/src/main/java/com/predic8/membrane/core/util/MessageUtil.java +++ b/core/src/main/java/com/predic8/membrane/core/util/MessageUtil.java @@ -36,36 +36,44 @@ public class MessageUtil { saxParserFactory.setValidating(false); } - public static InputStream getContentAsStream(Message msg) throws IOException { - if (msg.isGzip()) { - return new GZIPInputStream(msg.getBodyAsStream()); - } - if (msg.isDeflate()) { - return new ByteArrayInputStream(getDecompressedData(msg.getBody().getContent())); - } - if (msg.isBrotli()) { - return new BrotliInputStream(msg.getBodyAsStream()); + public static InputStream getContentAsStream(Message msg) { + try { + if (msg.isGzip()) { + return new GZIPInputStream(msg.getBodyAsStream()); + } + if (msg.isDeflate()) { + return new ByteArrayInputStream(getDecompressedData(msg.getBody().getContent())); + } + if (msg.isBrotli()) { + return new BrotliInputStream(msg.getBodyAsStream()); + } + return msg.getBodyAsStream(); + } catch (IOException e) { + throw new ReadingBodyException(e); } - return msg.getBodyAsStream(); } - public static byte[] getContent(Message msg) throws Exception { - if (msg.isGzip()) { - try (InputStream lInputStream = msg.getBodyAsStream(); - GZIPInputStream lGZIPInputStream = new GZIPInputStream(lInputStream)) { - return lGZIPInputStream.readAllBytes(); - } - } - if (msg.isDeflate()) { - return getDecompressedData(msg.getBody().getContent()); - } - if (msg.isBrotli()) { - try (InputStream lInputStream = msg.getBodyAsStream(); - BrotliInputStream lBrotliInputStream = new BrotliInputStream(lInputStream)) { - return lBrotliInputStream.readAllBytes(); + public static byte[] getContent(Message msg) { + try { + if (msg.isGzip()) { + try (InputStream lInputStream = msg.getBodyAsStream(); + GZIPInputStream lGZIPInputStream = new GZIPInputStream(lInputStream)) { + return lGZIPInputStream.readAllBytes(); + } + } + if (msg.isDeflate()) { + return getDecompressedData(msg.getBody().getContent()); + } + if (msg.isBrotli()) { + try (InputStream lInputStream = msg.getBodyAsStream(); + BrotliInputStream lBrotliInputStream = new BrotliInputStream(lInputStream)) { + return lBrotliInputStream.readAllBytes(); + } } + return msg.getBody().getContent(); + } catch (IOException e) { + throw new ReadingBodyException(e); } - return msg.getBody().getContent(); } public static Source getSOAPBody(InputStream stream) { diff --git a/core/src/main/java/com/predic8/membrane/core/util/SOAPUtil.java b/core/src/main/java/com/predic8/membrane/core/util/SOAPUtil.java index 09a33dc352..53e5c2839e 100644 --- a/core/src/main/java/com/predic8/membrane/core/util/SOAPUtil.java +++ b/core/src/main/java/com/predic8/membrane/core/util/SOAPUtil.java @@ -50,9 +50,11 @@ public static boolean isSOAP( XOPReconstitutor xopr, Message msg) { QName name = ((StartElement) event).getName(); return (isSOAP11Element(name) || isSOAP12Element(name)) && - "Envelope".equals(name.getLocalPart()); + "Envelope".equals(name.getLocalPart()); } } + } catch (ReadingBodyException e) { + throw e; } catch (Exception e) { log.warn("Ignoring exception: ", e); } diff --git a/core/src/test/java/com/predic8/membrane/core/interceptor/BodyDoesntThrowIOExceptionTest.java b/core/src/test/java/com/predic8/membrane/core/interceptor/BodyDoesntThrowIOExceptionTest.java new file mode 100644 index 0000000000..6d0a1bee04 --- /dev/null +++ b/core/src/test/java/com/predic8/membrane/core/interceptor/BodyDoesntThrowIOExceptionTest.java @@ -0,0 +1,29 @@ +package com.predic8.membrane.core.interceptor; + +import com.predic8.membrane.core.http.AbstractBody; +import com.tngtech.archunit.junit.AnalyzeClasses; +import com.tngtech.archunit.junit.ArchTest; +import com.tngtech.archunit.lang.ArchRule; +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition; + +import java.io.IOException; + +@AnalyzeClasses(packages = "com.predic8.membrane.core") +public class BodyDoesntThrowIOExceptionTest { + @ArchTest + static final ArchRule bodyMethodsShouldNotThrowIOException = + ArchRuleDefinition.methods().that() + .areDeclaredInClassesThat().areAssignableTo(AbstractBody.class).and() + .arePublic().and() + .areNotStatic() + .should().notDeclareThrowableOfType(IOException.class) + .as("Public instance methods of Body and its subclasses should not throw IOException.") + .because(""" + a) We need to distinguish between HTTP server and client. + + b) The body should handle errors itself, so that resources can be freed correctly (HTTP stream handling). + + c) Throw ReadingBodyException or WritingBodyException instead. + """); + +} diff --git a/core/src/test/java/com/predic8/membrane/core/interceptor/flow/invocation/testinterceptors/AbortFlowTestInterceptor.java b/core/src/test/java/com/predic8/membrane/core/interceptor/flow/invocation/testinterceptors/AbortFlowTestInterceptor.java index 185b9a5b9b..e306a8f6d1 100644 --- a/core/src/test/java/com/predic8/membrane/core/interceptor/flow/invocation/testinterceptors/AbortFlowTestInterceptor.java +++ b/core/src/test/java/com/predic8/membrane/core/interceptor/flow/invocation/testinterceptors/AbortFlowTestInterceptor.java @@ -28,11 +28,7 @@ public class AbortFlowTestInterceptor extends AbstractInterceptor { @Override public Outcome handleRequest(Exchange exc) { - try { - exc.setResponse(Response.ok().body(exc.getRequest().getBody().getContent()).build()); - } catch (IOException e) { - throw new RuntimeException(e); - } + exc.setResponse(Response.ok().body(exc.getRequest().getBody().getContent()).build()); return ABORT; } diff --git a/core/src/test/java/com/predic8/membrane/core/interceptor/flow/invocation/testinterceptors/ExceptionTestInterceptor.java b/core/src/test/java/com/predic8/membrane/core/interceptor/flow/invocation/testinterceptors/ExceptionTestInterceptor.java index f9714870bb..7e9c0a7d56 100644 --- a/core/src/test/java/com/predic8/membrane/core/interceptor/flow/invocation/testinterceptors/ExceptionTestInterceptor.java +++ b/core/src/test/java/com/predic8/membrane/core/interceptor/flow/invocation/testinterceptors/ExceptionTestInterceptor.java @@ -27,11 +27,7 @@ public class ExceptionTestInterceptor extends AbstractInterceptor { @Override public Outcome handleRequest(Exchange exc) { - try { - exc.setResponse(ok().body(exc.getRequest().getBody().getContent()).build()); - } catch (IOException e) { - throw new RuntimeException(e); - } + exc.setResponse(ok().body(exc.getRequest().getBody().getContent()).build()); throw new RuntimeException(); } diff --git a/core/src/test/java/com/predic8/membrane/core/transport/ssl/SessionResumptionTest.java b/core/src/test/java/com/predic8/membrane/core/transport/ssl/SessionResumptionTest.java index b7ab5e7bc2..88635efaca 100644 --- a/core/src/test/java/com/predic8/membrane/core/transport/ssl/SessionResumptionTest.java +++ b/core/src/test/java/com/predic8/membrane/core/transport/ssl/SessionResumptionTest.java @@ -100,13 +100,7 @@ private static Router createTLSServer(int port) { public Outcome handleRequest(Exchange exc) { // Inlined from Exchange. Maybe use EchoIntercepor Response.ResponseBuilder builder = Response.ok(); - byte[] content; - try { - content = exc.getRequest().getBody().getContent(); - } catch (IOException e) { - throw new RuntimeException(e); - } - builder.body(content); + builder.body(exc.getRequest().getBody().getContent()); String contentType = exc.getRequest().getHeader().getContentType(); if (contentType != null) builder.header(CONTENT_TYPE, contentType);