RuntimeException thrown instead of IllegalArgumentException when failing to decode URL

[luisRubiera]

Describe the bug

When making a request with an invalid parameter in the body—specifically, a non-decodable parameter—the exception thrown is a RuntimeException. However, according to the documentation here, the expected behavior is to throw an IllegalArgumentException.

Expected behavior

According to the code in URLUtils.decode, an IllegalArgumentException should be thrown when decoding an invalid parameter.

Actual behavior

When an invalid parameter is provided, the code calls FailedToDecodeURL, which throws a RuntimeException, leading to a 500 Internal Server Error instead of a 400 Bad Request.

How to Reproduce?

Short Version:
Run a Quarkus-based web server and send a request with a malformed, non-decodable parameter in the request body.

Long Version (Reproducing with Keycloak):

1. Run the following docker-compose file to start Keycloak (>= 23, as Keycloak migrated to Quarkus after this version):

version: '3.6'

services:
  keycloak_web:
    image: quay.io/keycloak/keycloak:23.0
    platform: linux/arm64
    container_name: keycloak_web
    environment:
      KC_DB: postgres
      KC_DB_URL: jdbc:postgresql://keycloakdb:5432/keycloak
      KC_DB_USERNAME: keycloak
      KC_DB_PASSWORD: password

      KC_HOSTNAME: localhost
      KC_HOSTNAME_PORT: 8080
      KC_HOSTNAME_STRICT: false
      KC_HOSTNAME_STRICT_HTTPS: false

      KC_LOG_LEVEL: info
      KC_METRICS_ENABLED: true
      KC_HEALTH_ENABLED: true
      KEYCLOAK_ADMIN: admin
      KEYCLOAK_ADMIN_PASSWORD: admin
    command: start-dev
    depends_on:
      - keycloakdb
    ports:
      - 8080:8080

  keycloakdb:
    image: postgres:15
    volumes:
      - postgres_data:/var/lib/postgresql/data
    environment:
      POSTGRES_DB: keycloak
      POSTGRES_USER: keycloak
      POSTGRES_PASSWORD: password

volumes:
  postgres_data:

2. Execute the following curl command:

curl -X POST "http://localhost:8080/realms/master/protocol/openid-connect/token" \
     -H "Content-Type: application/x-www-form-urlencoded" \
     -d "anyparam=fake%XXuser"

3. The response will trigger an error in the logs:

keycloak_web  | 2025-02-11 13:26:22,334 ERROR [org.keycloak.services.error.KeycloakErrorHandler] (executor-thread-1) Uncaught server error: java.lang.RuntimeException: Failed to decode URL fake%XXuser to UTF-8
keycloak_web  |         at org.jboss.resteasy.reactive.common.util.URLUtils.failedToDecodeURL(URLUtils.java:227)
keycloak_web  |         at org.jboss.resteasy.reactive.common.util.URLUtils.decode(URLUtils.java:161)
keycloak_web  |         at org.jboss.resteasy.reactive.common.util.URLUtils.decode(URLUtils.java:87)
keycloak_web  |         at org.jboss.resteasy.reactive.server.core.multipart.FormEncodedDataDefinition$FormEncodedDataParser.decodeParameterValue(FormEncodedDataDefinition.java:208)
keycloak_web  |         at org.jboss.resteasy.reactive.server.core.multipart.FormEncodedDataDefinition$FormEncodedDataParser.inputDone(FormEncodedDataDefinition.java:264)
keycloak_web  |         at org.jboss.resteasy.reactive.server.core.multipart.FormEncodedDataDefinition$FormEncodedDataParser.parseBlocking(FormEncodedDataDefinition.java:239)
keycloak_web  |         at org.jboss.resteasy.reactive.server.handlers.FormBodyHandler.handle(FormBodyHandler.java:94)
keycloak_web  |         at io.quarkus.resteasy.reactive.server.runtime.QuarkusResteasyReactiveRequestContext.invokeHandler(QuarkusResteasyReactiveRequestContext.java:150)
keycloak_web  |         at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:145)
keycloak_web  |         at io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:576)
keycloak_web  |         at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513)
keycloak_web  |         at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538)
keycloak_web  |         at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29)
keycloak_web  |         at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29)
keycloak_web  |         at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
keycloak_web  |         at java.base/java.lang.Thread.run(Thread.java:840)

Output of uname -a or ver

Linux fe9e37cde0ad 6.10.14-linuxkit #1 SMP Thu Oct 24 19:28:55 UTC 2024 aarch64 aarch64 aarch64 GNU/Linux

Output of java -version

openjdk version "17.0.10" 2024-01-16 LTS OpenJDK Runtime Environment (Red_Hat-17.0.10.0.7-1) (build 17.0.10+7-LTS) OpenJDK 64-Bit Server VM (Red_Hat-17.0.10.0.7-1) (build 17.0.10+7-LTS, mixed mode)

Quarkus version or git rev

tested with 3.2.7.Final, also 3.8.5 and the code has not changed

Build tool (ie. output of mvnw --version or gradlew --version)

built with docker so not needed

Additional information

• The issue arises because the failedToDecodeURL method throws a RuntimeException, which results in a 500 Internal Server Error.
• Instead, it should throw an IllegalArgumentException, which would result in a 400 Bad Request.
• Simply updating the documentation to match the RuntimeException behavior is not recommended, as it allows attackers to cause unnecessary 500 errors instead of being properly handled as bad input.

Why This Matters:

• A RuntimeException (500 Internal Server Error) implies an issue with the backend, whereas an IllegalArgumentException (400 Bad Request) correctly indicates a client-side issue.
• In cases of malformed input (like an attacker sending invalid encoded parameters), the server should not respond with a 500 but rather return a 400, signaling that the request is invalid.

Suggested Fix

Modify URLUtils.failedToDecodeURL to throw an IllegalArgumentException instead of a RuntimeException.

Current Code:

    private static RuntimeException failedToDecodeURL(String s, Charset enc, Throwable o) {
        return new RuntimeException("Failed to decode URL " + s + " to " + enc, o);
    }

Proposed Change:

    private static IllegalArgumentException failedToDecodeURL(String s, Charset enc, Throwable o) {
        return new IllegalArgumentException("Invalid encoding URL of " + s + " to " + enc, o);
    }

[quarkus-bot]

/cc @FroMage (rest)

[geoand]

Makes sense! Would you like to propose a PR?

[luisRubiera]

Sure! , i'll submit the PR , it would be my first so i'll probably take long to follow the contributing guidelines 🙏

[geoand]

It shouldn't be too long :)

[rodcheater]

I came across this because I have the same issue. I’m not sure how changing the exception type to IllegalArgumentException fixes the issue that such requests respond with a 500. I’m guessing the OP has an exception mapper to map IllegalArgumentException to a 400 response, but I’m reluctant to do that because an IllegalArgumentException could come from anywhere and such a mapping might mask genuine server errors that need our attention.

Should it be throwing an instance of jakarta.ws.rs.BadRequestException instead?

(I also have the same issue with decoding forms in Undertow servlets but I don’t think there’s an easy fix there because the exception flows through user code.)

[geoand]

Should it be throwing an instance of jakarta.ws.rs.BadRequestException instead?

That does sounds reasonable, WDYT @FroMage ?