search

line / armeria

Your go-to microservice framework for any situation, from the creator of Netty et al. You can build any type of microservice leveraging your favorite technologies, including gRPC, Thrift, Kotlin, Retrofit, Reactive Streams, Spring Boot and Dropwizard.
https://armeria.dev
Apache License 2.0
4.83k stars 922 forks source link

Fix a bug where a protocol violation is not handled by `ServerErrorHandler` #5811

Closed ikhoon closed 4 months ago

ikhoon commented 4 months ago

Motivation:

CorsServerErrorHandler was added as the first error handler by #5632 CorsServerErrorHandler does not handle an exception but delegates it to the default one or user-defined handler and injects CORS headers.

CorsServerErrorHandler overrode onServiceException to delegate but omitted to override onProtocolViolation. As a result, a user-defined onProtocolViolation wasn't invoked.

Modifications:

Result:

A protocol violation is now correctly handled by ServerErrorHandler.onProtocolViolation()

github-actions[bot] commented 4 months ago

🔍 Build Scan® (commit: ff2278eb82af69914967116d3b6b70854b73a20d)

Job name Status Build Scan®
build-windows-latest-jdk-21 ❌ (failure) https://ge.armeria.dev/s/t43pqp6pvck3m
build-self-hosted-unsafe-jdk-8 ✅ https://ge.armeria.dev/s/jmvy5vny3pxs4
build-self-hosted-unsafe-jdk-21-snapshot-blockhound ✅ https://ge.armeria.dev/s/4zz2rarkesxms
build-self-hosted-unsafe-jdk-17-min-java-17-coverage ✅ https://ge.armeria.dev/s/xfrppyzuxvzrs
build-self-hosted-unsafe-jdk-17-min-java-11 ✅ https://ge.armeria.dev/s/y2qbpw4hkrvk2
build-self-hosted-unsafe-jdk-17-leak ✅ https://ge.armeria.dev/s/5atluh4yso6jc
build-self-hosted-unsafe-jdk-11 ✅ https://ge.armeria.dev/s/qxguaj33hph44
build-macos-12-jdk-21 ✅ https://ge.armeria.dev/s/bcn3vgs64ypba
ikhoon commented 4 months ago

I prefer keeping it default because most users may not be interested in protocol-level errors.