From ba8182b135534ed46caa815007a5d1dba14feda5 Mon Sep 17 00:00:00 2001 From: Santiago Pericasgeertsen Date: Tue, 9 Jan 2024 15:13:07 -0500 Subject: [PATCH] Check result of Integer.parseUnsignedInt() to be non-negative. Signed-off-by: Santiago Pericasgeertsen --- .../java/io/helidon/common/IntegerParser.java | 46 +++++++++++++++++++ .../io/helidon/common/IntegerParserTest.java | 42 +++++++++++++++++ .../webclient/http1/Http1CallChainBase.java | 8 +++- .../webserver/http1/Http1Connection.java | 11 ++++- 4 files changed, 103 insertions(+), 4 deletions(-) create mode 100644 common/common/src/main/java/io/helidon/common/IntegerParser.java create mode 100644 common/common/src/test/java/io/helidon/common/IntegerParserTest.java diff --git a/common/common/src/main/java/io/helidon/common/IntegerParser.java b/common/common/src/main/java/io/helidon/common/IntegerParser.java new file mode 100644 index 00000000000..cfe05aa1a00 --- /dev/null +++ b/common/common/src/main/java/io/helidon/common/IntegerParser.java @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2024 Oracle and/or its affiliates. + * + * 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 io.helidon.common; + +import java.util.function.Supplier; + +/** + * Utility class to properly report integer parsing errors. + */ +public class IntegerParser { + + private IntegerParser() { + } + + /** + * Throws exception if value returned is negative. Method {@link Integer#parseUnsignedInt(String, int)} + * can return negative numbers. + * + * @param s string to parse + * @param radix the number radix + * @param supplier throwable value + * @return the number + * @param type of throwable + * @throws T throwable if number is negative + */ + public static int parseNonNegative(String s, int radix, Supplier supplier) throws T { + int v = Integer.parseUnsignedInt(s, radix); + if (v < 0) { + throw supplier.get(); + } + return v; + } +} diff --git a/common/common/src/test/java/io/helidon/common/IntegerParserTest.java b/common/common/src/test/java/io/helidon/common/IntegerParserTest.java new file mode 100644 index 00000000000..dabc94522dc --- /dev/null +++ b/common/common/src/test/java/io/helidon/common/IntegerParserTest.java @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2024 Oracle and/or its affiliates. + * + * 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 io.helidon.common; + +import org.junit.jupiter.api.Test; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class IntegerParserTest { + + @Test + void testNormalParse() { + int v = IntegerParser.parseNonNegative( + "7FFFFFFF", + 16, + () -> new RuntimeException("Oops")); + assertThat(v, is(Integer.MAX_VALUE)); + } + + @Test + void testNegativeParse() { + assertThrows(RuntimeException.class, () -> + IntegerParser.parseNonNegative("FFFFFFFF", + 16, + () -> new RuntimeException("Value cannot be negative"))); + } +} diff --git a/webclient/http1/src/main/java/io/helidon/webclient/http1/Http1CallChainBase.java b/webclient/http1/src/main/java/io/helidon/webclient/http1/Http1CallChainBase.java index 41c076d0ee3..f4af4f762c9 100644 --- a/webclient/http1/src/main/java/io/helidon/webclient/http1/Http1CallChainBase.java +++ b/webclient/http1/src/main/java/io/helidon/webclient/http1/Http1CallChainBase.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 Oracle and/or its affiliates. + * Copyright (c) 2023, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,7 +23,9 @@ import java.time.Duration; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; +import io.helidon.common.IntegerParser; import io.helidon.common.buffers.BufferData; import io.helidon.common.buffers.Bytes; import io.helidon.common.buffers.DataReader; @@ -55,6 +57,8 @@ abstract class Http1CallChainBase implements WebClientService.Chain { private static final System.Logger LOGGER = System.getLogger(Http1CallChainBase.class.getName()); + private static final Supplier NEGATIVE_SIZE_SUPPLIER = + () -> new IllegalArgumentException("Chunk size is invalid"); private final BufferData writeBuffer = BufferData.growing(128); private final HttpClientConfig clientConfig; @@ -507,7 +511,7 @@ private void ensureBuffer() { reader.skip(2); // CRLF int length; try { - length = Integer.parseUnsignedInt(hex, 16); + length = IntegerParser.parseNonNegative(hex, 16, NEGATIVE_SIZE_SUPPLIER); } catch (NumberFormatException e) { throw new IllegalArgumentException("Chunk size is not a number:\n" + BufferData.create(hex.getBytes(US_ASCII)).debugDataHex()); diff --git a/webserver/webserver/src/main/java/io/helidon/webserver/http1/Http1Connection.java b/webserver/webserver/src/main/java/io/helidon/webserver/http1/Http1Connection.java index cb8e5dc572e..9fa7095c2d8 100644 --- a/webserver/webserver/src/main/java/io/helidon/webserver/http1/Http1Connection.java +++ b/webserver/webserver/src/main/java/io/helidon/webserver/http1/Http1Connection.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2023 Oracle and/or its affiliates. + * Copyright (c) 2022, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,7 +23,9 @@ import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Semaphore; +import java.util.function.Supplier; +import io.helidon.common.IntegerParser; import io.helidon.common.buffers.BufferData; import io.helidon.common.buffers.DataReader; import io.helidon.common.buffers.DataWriter; @@ -64,6 +66,11 @@ */ public class Http1Connection implements ServerConnection, InterruptableTask { private static final System.Logger LOGGER = System.getLogger(Http1Connection.class.getName()); + private static final Supplier NEGATIVE_SIZE_SUPPLIER = + () -> RequestException.builder() + .type(EventType.BAD_REQUEST) + .message("Chunk size is invalid") + .build(); static final byte[] CONTINUE_100 = "HTTP/1.1 100 Continue\r\n\r\n".getBytes(StandardCharsets.UTF_8); @@ -264,7 +271,7 @@ private BufferData readEntityFromPipeline(HttpPrologue prologue, WritableHeaders private BufferData readNextChunk(HttpPrologue prologue, WritableHeaders headers) { // chunk length processing String hex = reader.readLine(); - int chunkLength = Integer.parseUnsignedInt(hex, 16); + int chunkLength = IntegerParser.parseNonNegative(hex, 16, NEGATIVE_SIZE_SUPPLIER); currentEntitySizeRead += chunkLength; if (maxPayloadSize != -1 && currentEntitySizeRead > maxPayloadSize) {