Skip to content

Failing to handle deserialization exceptions in batch listener #3114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ikercrg opened this issue Mar 11, 2024 · 1 comment · Fixed by #3115
Closed

Failing to handle deserialization exceptions in batch listener #3114

ikercrg opened this issue Mar 11, 2024 · 1 comment · Fixed by #3115

Comments

@ikercrg
Copy link

ikercrg commented Mar 11, 2024

I am migrating Spring boot from version 2 to version 3.

In my former implementation, I was based on v2.8.4 documentation and all was working fine.

Currently, ListenerUtils.byteArrayToDeserializationException is removed and I have to use SerializationUtils.byteArrayToDeserializationException.

For this, I am basing on v3.1.x documentation, but I have problems.

The example given in the documentation, precisely this part, does not compile:

DeserializationException deserEx = SerializationUtils.byteArrayToDeserializationException(this.logger,
                    headers.get(i).get(SerializationUtils.VALUE_DESERIALIZER_EXCEPTION_HEADER));

Theoretically what is passed is a byte[], but the method only admits a Header. So we would need a cast for that. With ListenerUtils I was casting it to byte[]. But now, I cast to Header or RecordHeader, I get the following error -> Foreign deserialization exception header ignored; possible attack? Something like the following:

DeserializationException exception = SerializationUtils.byteArrayToDeserializationException(
            new LogAccessor(IndexingService.class),
            new RecordHeader(SerializationUtils.KEY_DESERIALIZER_EXCEPTION_HEADER,
                (byte[]) headersOfNullItem.get(SerializationUtils.VALUE_DESERIALIZER_EXCEPTION_HEADER)));

I have tried to create a DeserializationExceptionHeader, but it is package-protected.

Using

List<ConsumerRecord<String, my_object>> my_method()

instead of what is said in the documentation

List<Thing> in, @Header(KafkaHeaders.BATCH_CONVERTED_HEADERS) List<Map<String, Object>> headers

works fine.

Issue opened as petition from @artembilan in Stackoverflow:

See https://stackoverflow.com/questions/78122348/failing-to-handle-deserialization-exceptions-in-batch-listener

@sobychacko
Copy link
Contributor

@ikercrg, You are right in that when you consume as ConsumerRecord, you can now use the SerializationUtils#getExceptionHeader method.

	/**
	 * Extract a {@link DeserializationException} from the supplied header name, if
	 * present.
	 * @param record the consumer record.
	 * @param headerName the header name.
	 * @param logger the logger for logging errors.
	 * @return the exception or null.
	 * @since 2.9.11
	 */
	@Nullable
	public static DeserializationException getExceptionFromHeader(final ConsumerRecord<?, ?> record,
			String headerName, LogAccessor logger) {

However, it appears that there is a bug that might be preventing us from using the method byteArrayToDeserializationException. It expects the header to be of type DeserializationExceptionHeader, while users cannot create that because of the fact that it is package-protected.

public static DeserializationException byteArrayToDeserializationException(LogAccessor logger, Header header) {

		if (header != null && !(header instanceof DeserializationExceptionHeader)) {
			throw new IllegalStateException("Foreign deserialization exception header ignored; possible attack?");
		}
		try {
...

If you create a standard RecordHeader, that will certainly fail at that check ^^. We might have to delay that check or come up with another way to tackle this. We will come up with a plan soon. Thanks for raising the issue.

sobychacko added a commit to sobychacko/spring-kafka that referenced this issue Mar 12, 2024
Fixes: spring-projects#3114

* Since DeserializationExceptionHeader is currently porpagated as a `byte[]`,
  it encounters some issues when processing the header especially in batch
  listeners. Fixing this by providing the deserialization header without `byte[]` conversion
* Adding test to verify
* Refactoring in SerializationTestUtils
artembilan pushed a commit that referenced this issue Mar 14, 2024
Fixes: #3114

* Since DeserializationExceptionHeader is currently porpagated as a `byte[]`,
  it encounters some issues when processing the header especially in batch
  listeners. Fixing this by providing the deserialization header without `byte[]` conversion
* Adding test to verify
* Refactoring in SerializationTestUtils

**Auto-cherry-pick to `3.1.x` & `3.0.x`**
spring-builds pushed a commit that referenced this issue Mar 14, 2024
Fixes: #3114

* Since DeserializationExceptionHeader is currently porpagated as a `byte[]`,
  it encounters some issues when processing the header especially in batch
  listeners. Fixing this by providing the deserialization header without `byte[]` conversion
* Adding test to verify
* Refactoring in SerializationTestUtils

(cherry picked from commit 7cc7fc8)
artembilan pushed a commit that referenced this issue Mar 14, 2024
Fixes: #3114

* Since DeserializationExceptionHeader is currently porpagated as a `byte[]`,
  it encounters some issues when processing the header especially in batch
  listeners. Fixing this by providing the deserialization header without `byte[]` conversion
* Adding test to verify
* Refactoring in SerializationTestUtils

(cherry picked from commit 7cc7fc8)

# Conflicts:
#	spring-kafka/src/main/java/org/springframework/kafka/support/KafkaUtils.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment