From ea8ac339019ee3c965adcb975a0c9e84e597ed1f Mon Sep 17 00:00:00 2001 From: Moritz Halbritter Date: Tue, 8 Feb 2022 14:09:37 +0100 Subject: [PATCH 1/3] Ignore invalid stream types when reading log update events See gh-29307 --- .../buildpack/platform/docker/LogUpdateEvent.java | 12 ++++++++++-- .../platform/docker/LogUpdateEventTests.java | 8 +++++++- .../log-update-event-invalid-stream-type.stream | Bin 0 -> 91 bytes 3 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/resources/org/springframework/boot/buildpack/platform/docker/log-update-event-invalid-stream-type.stream diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEvent.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEvent.java index c7a3cffe8925..c9d2cc99e55e 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEvent.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEvent.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -90,7 +90,15 @@ private static LogUpdateEvent read(InputStream inputStream) throws IOException { if (header == null) { return null; } - StreamType streamType = StreamType.values()[header[0]]; + + // First byte denotes stream type. 0 = stdin, 1 = stdout, 2 = stderr + byte streamTypeId = header[0]; + if (streamTypeId < 0 || streamTypeId >= StreamType.values().length) { + // Stream type is out of bounds, ignore this event. See gh-29307 + return null; + } + + StreamType streamType = StreamType.values()[streamTypeId]; long size = 0; for (int i = 0; i < 4; i++) { size = (size << 8) + (header[i + 4] & 0xff); diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEventTests.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEventTests.java index d794c307b8e4..8315d75e2617 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEventTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEventTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -53,6 +53,12 @@ void readAllWhenAnsiStreamReturnsEvents() throws Exception { assertThat(events.get(2).toString()).isEqualTo(" OpenJDK JRE 11.0.5: Reusing cached layer"); } + @Test + void readSucceedsWhenStreamTypeIsInvalid() throws IOException { + List events = readAll("log-update-event-invalid-stream-type.stream"); + assertThat(events).isEmpty(); + } + private List readAll(String name) throws IOException { List events = new ArrayList<>(); try (InputStream inputStream = getClass().getResourceAsStream(name)) { diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/resources/org/springframework/boot/buildpack/platform/docker/log-update-event-invalid-stream-type.stream b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/resources/org/springframework/boot/buildpack/platform/docker/log-update-event-invalid-stream-type.stream new file mode 100644 index 0000000000000000000000000000000000000000..f9ddbfa14e6ab781d438bc9a5fb27385a1900589 GIT binary patch literal 91 zcmW;3yA6Oa3)832k{Ou5c_a}M(8Kgd+>OI;TW>p`_qh9fT-NLCgfd7N k0o5#tP)Q)RwuETD)tF2n;0#ty8gnS@GyE=EXc06weLcVz+5i9m literal 0 HcmV?d00001 From dfcc76d4d857c3758a234e6e5c88631afc6acf7f Mon Sep 17 00:00:00 2001 From: Moritz Halbritter Date: Wed, 9 Feb 2022 10:30:40 +0100 Subject: [PATCH 2/3] Convert illegal stream types into stderr LogUpdateEvents See gh-29307 --- .../platform/docker/LogUpdateEvent.java | 31 ++++++++++++++++--- .../platform/docker/LogUpdateEventTests.java | 3 +- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEvent.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEvent.java index c9d2cc99e55e..0b7b8f094a90 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEvent.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEvent.java @@ -76,9 +76,20 @@ public String toString() { static void readAll(InputStream inputStream, Consumer consumer) throws IOException { try { LogUpdateEvent event; - while ((event = LogUpdateEvent.read(inputStream)) != null) { - consumer.accept(event); + do { + try { + event = LogUpdateEvent.read(inputStream); + if (event != null) { + consumer.accept(event); + } + } + catch (IllegalStateException e) { + // If parsing the event failed, convert exception into a stdErr event + event = new LogUpdateEvent(StreamType.STD_ERR, e.getMessage().getBytes(StandardCharsets.UTF_8)); + consumer.accept(event); + } } + while (event != null); } finally { inputStream.close(); @@ -93,17 +104,27 @@ private static LogUpdateEvent read(InputStream inputStream) throws IOException { // First byte denotes stream type. 0 = stdin, 1 = stdout, 2 = stderr byte streamTypeId = header[0]; + boolean illegalStreamType = false; if (streamTypeId < 0 || streamTypeId >= StreamType.values().length) { - // Stream type is out of bounds, ignore this event. See gh-29307 - return null; + // Don't throw exception right here, otherwise the stream is not advanced + // correctly, and subsequent reads start from a wrong position + illegalStreamType = true; } - StreamType streamType = StreamType.values()[streamTypeId]; long size = 0; for (int i = 0; i < 4; i++) { size = (size << 8) + (header[i + 4] & 0xff); } byte[] payload = read(inputStream, size); + + if (illegalStreamType) { + // At this point the stream has been advanved correctly and it is safe to + // throw exceptions + throw new IllegalStateException("Stream type is out of bounds. Must be >= 0 and < " + + StreamType.values().length + ", but was " + streamTypeId); + } + + StreamType streamType = StreamType.values()[streamTypeId]; return new LogUpdateEvent(streamType, payload); } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEventTests.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEventTests.java index 8315d75e2617..e8a6fc429793 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEventTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEventTests.java @@ -56,7 +56,8 @@ void readAllWhenAnsiStreamReturnsEvents() throws Exception { @Test void readSucceedsWhenStreamTypeIsInvalid() throws IOException { List events = readAll("log-update-event-invalid-stream-type.stream"); - assertThat(events).isEmpty(); + assertThat(events).hasSize(1); + assertThat(events.get(0).toString()).isEqualTo("Stream type is out of bounds. Must be >= 0 and < 3, but was 3"); } private List readAll(String name) throws IOException { From 5d048bf5f01896308655184ed15bc0c56a1c9361 Mon Sep 17 00:00:00 2001 From: Moritz Halbritter Date: Wed, 9 Feb 2022 12:03:37 +0100 Subject: [PATCH 3/3] Abort parsing LogUpdateEvents when encountering invalid stream type See gh-29307 --- .../platform/docker/LogUpdateEvent.java | 40 ++++++++----------- .../platform/docker/LogUpdateEventTests.java | 3 +- 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEvent.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEvent.java index 0b7b8f094a90..9ed0483139ed 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEvent.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEvent.java @@ -22,6 +22,8 @@ import java.util.function.Consumer; import java.util.regex.Pattern; +import org.springframework.util.StreamUtils; + /** * An update event used to provide log updates. * @@ -76,20 +78,19 @@ public String toString() { static void readAll(InputStream inputStream, Consumer consumer) throws IOException { try { LogUpdateEvent event; - do { - try { - event = LogUpdateEvent.read(inputStream); - if (event != null) { - consumer.accept(event); - } - } - catch (IllegalStateException e) { - // If parsing the event failed, convert exception into a stdErr event - event = new LogUpdateEvent(StreamType.STD_ERR, e.getMessage().getBytes(StandardCharsets.UTF_8)); - consumer.accept(event); - } + while ((event = LogUpdateEvent.read(inputStream)) != null) { + consumer.accept(event); } - while (event != null); + } + catch (IllegalStateException ex) { + // Parsing has failed, abort further parsing + LogUpdateEvent abortedEvent = new LogUpdateEvent(StreamType.STD_ERR, + ex.getMessage().getBytes(StandardCharsets.UTF_8)); + consumer.accept(abortedEvent); + + // At this point, the inputStream is burned, consume it fully to prevent + // further processing + StreamUtils.drain(inputStream); } finally { inputStream.close(); @@ -104,11 +105,9 @@ private static LogUpdateEvent read(InputStream inputStream) throws IOException { // First byte denotes stream type. 0 = stdin, 1 = stdout, 2 = stderr byte streamTypeId = header[0]; - boolean illegalStreamType = false; if (streamTypeId < 0 || streamTypeId >= StreamType.values().length) { - // Don't throw exception right here, otherwise the stream is not advanced - // correctly, and subsequent reads start from a wrong position - illegalStreamType = true; + throw new IllegalStateException("Stream type is out of bounds. Must be >= 0 and < " + + StreamType.values().length + ", but was " + streamTypeId + ". Will abort parsing."); } long size = 0; @@ -117,13 +116,6 @@ private static LogUpdateEvent read(InputStream inputStream) throws IOException { } byte[] payload = read(inputStream, size); - if (illegalStreamType) { - // At this point the stream has been advanved correctly and it is safe to - // throw exceptions - throw new IllegalStateException("Stream type is out of bounds. Must be >= 0 and < " - + StreamType.values().length + ", but was " + streamTypeId); - } - StreamType streamType = StreamType.values()[streamTypeId]; return new LogUpdateEvent(streamType, payload); } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEventTests.java b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEventTests.java index e8a6fc429793..577eb012dc40 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEventTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/test/java/org/springframework/boot/buildpack/platform/docker/LogUpdateEventTests.java @@ -57,7 +57,8 @@ void readAllWhenAnsiStreamReturnsEvents() throws Exception { void readSucceedsWhenStreamTypeIsInvalid() throws IOException { List events = readAll("log-update-event-invalid-stream-type.stream"); assertThat(events).hasSize(1); - assertThat(events.get(0).toString()).isEqualTo("Stream type is out of bounds. Must be >= 0 and < 3, but was 3"); + assertThat(events.get(0).toString()) + .isEqualTo("Stream type is out of bounds. Must be >= 0 and < 3, but was 3. Will abort parsing."); } private List readAll(String name) throws IOException {