Skip to content
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

Avoid ClassCastException on IllegalArgumentException when invoking sync get cache method #25110

Closed
wants to merge 1 commit into from

Conversation

WEIZIBIN
Copy link

@WEIZIBIN WEIZIBIN commented May 20, 2020

Avoid some exception cause ClassCastException

Let's take a look at ConcurrentMapCache.java


	@Override
	@Nullable
	public <T> T get(Object key, Callable<T> valueLoader) {
		return (T) fromStoreValue(this.store.computeIfAbsent(key, k -> {
			try {
				return toStoreValue(valueLoader.call());
			}
			catch (Throwable ex) {
				throw new ValueRetrievalException(key, valueLoader, ex);
			}
		}));
	}

	@Override
	protected Object fromStoreValue(@Nullable Object storeValue) {
		if (storeValue != null && this.serialization != null) {
			try {
				return super.fromStoreValue(deserializeValue(this.serialization, storeValue));
			}
			catch (Throwable ex) {
				throw new IllegalArgumentException("Failed to deserialize cache value '" + storeValue + "'", ex);
			}
		}
		else {
			return super.fromStoreValue(storeValue);
		}

	}

It will throw IllegalArgumentException when there is a problem with deserialization, but not ThrowableWrapper class type, when cast to ThrowableWrapper it will cause another exception (java.lang.ClassCastException: java.lang.IllegalArgumentException cannot be cast to org.springframework.cache.interceptor.CacheOperationInvoker$ThrowableWrapper)

So we need a wrap code on this.

… cache method

Avoid some exception cause ClassCastException
@pivotal-issuemaster
Copy link

@WEIZIBIN Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 20, 2020
@pivotal-issuemaster
Copy link

@WEIZIBIN Thank you for signing the Contributor License Agreement!

@jhoeller jhoeller self-assigned this May 20, 2020
@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 20, 2020
@jhoeller jhoeller added this to the 5.2.7 milestone May 20, 2020
@jhoeller jhoeller changed the title wrap ValueRetrievalException in ThrowableWrapper when invoke sync get cache method Avoid ClassCastException on IllegalArgumentException when invoking sync get cache method May 22, 2020
@jhoeller
Copy link
Contributor

While it is not typically expected for such internal deserialization to fail (since the serialized representation can only have been stored from a valid object in the same JVM run), it is a valid point that we should not attempt a hard cast of the exception there. However, the ThrowableWrapper comment refers to what's typically propagated from the invoker; there is no need to rewrap other kinds of exceptions the same way, just to rethrow them as-is. I've repurposed the PR for that slightly different resolution. Thanks for raising this, in any case!

@jhoeller jhoeller added the for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x label May 22, 2020
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x labels May 22, 2020
@jhoeller jhoeller closed this in 807ded2 May 22, 2020
FelixFly pushed a commit to FelixFly/spring-framework that referenced this pull request Aug 16, 2020
zx20110729 pushed a commit to zx20110729/spring-framework that referenced this pull request Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants