Skip to content

Commit 04f7655

Browse files
jhoellerjhoeller
committed
Fine-tuned method/field access checks
1 parent a28fc76 commit 04f7655

File tree

3 files changed

+69
-72
lines changed

3 files changed

+69
-72
lines changed

‎spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2015 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -248,13 +248,15 @@ else if ("C".equals(this.exitTypeDescriptor)) {
248248
cf.exitCompilationScope();
249249
mv.visitInsn(insn);
250250
}
251+
251252
else if (this.indexedType == IndexedType.LIST) {
252253
mv.visitTypeInsn(CHECKCAST, "java/util/List");
253254
cf.enterCompilationScope();
254255
this.children[0].generateCode(mv, cf);
255256
cf.exitCompilationScope();
256257
mv.visitMethodInsn(INVOKEINTERFACE, "java/util/List", "get", "(I)Ljava/lang/Object;", true);
257258
}
259+
258260
else if (this.indexedType == IndexedType.MAP) {
259261
mv.visitTypeInsn(CHECKCAST, "java/util/Map");
260262
// Special case when the key is an unquoted string literal that will be parsed as
@@ -271,27 +273,30 @@ else if (this.indexedType == IndexedType.MAP) {
271273
}
272274
mv.visitMethodInsn(INVOKEINTERFACE, "java/util/Map", "get", "(Ljava/lang/Object;)Ljava/lang/Object;", true);
273275
}
276+
274277
else if (this.indexedType == IndexedType.OBJECT) {
275278
ReflectivePropertyAccessor.OptimalPropertyAccessor accessor =
276279
(ReflectivePropertyAccessor.OptimalPropertyAccessor) this.cachedReadAccessor;
277280
Member member = accessor.member;
278281
boolean isStatic = Modifier.isStatic(member.getModifiers());
279-
String memberDeclaringClassSlashedDescriptor = member.getDeclaringClass().getName().replace('.', '/');
282+
String classDesc = member.getDeclaringClass().getName().replace('.', '/');
283+
280284
if (!isStatic) {
281285
if (descriptor == null) {
282286
cf.loadTarget(mv);
283287
}
284-
if (descriptor == null || !memberDeclaringClassSlashedDescriptor.equals(descriptor.substring(1))) {
285-
mv.visitTypeInsn(CHECKCAST, memberDeclaringClassSlashedDescriptor);
288+
if (descriptor == null || !classDesc.equals(descriptor.substring(1))) {
289+
mv.visitTypeInsn(CHECKCAST, classDesc);
286290
}
287291
}
288-
if (member instanceof Field) {
289-
mv.visitFieldInsn(isStatic ? GETSTATIC : GETFIELD, memberDeclaringClassSlashedDescriptor,
290-
member.getName(), CodeFlow.toJvmDescriptor(((Field) member).getType()));
292+
293+
if (member instanceof Method) {
294+
mv.visitMethodInsn((isStatic? INVOKESTATIC : INVOKEVIRTUAL), classDesc, member.getName(),
295+
CodeFlow.createSignatureDescriptor((Method) member), false);
291296
}
292297
else {
293-
mv.visitMethodInsn(isStatic? INVOKESTATIC : INVOKEVIRTUAL, memberDeclaringClassSlashedDescriptor,
294-
member.getName(), CodeFlow.createSignatureDescriptor((Method) member), false);
298+
mv.visitFieldInsn((isStatic ? GETSTATIC : GETFIELD), classDesc, member.getName(),
299+
CodeFlow.toJvmDescriptor(((Field) member).getType()));
295300
}
296301
}
297302

@@ -555,12 +560,8 @@ public TypedValue getValue() {
555560
ReflectivePropertyAccessor.OptimalPropertyAccessor optimalAccessor =
556561
(ReflectivePropertyAccessor.OptimalPropertyAccessor) accessor;
557562
Member member = optimalAccessor.member;
558-
if (member instanceof Field) {
559-
Indexer.this.exitTypeDescriptor = CodeFlow.toDescriptor(((Field)member).getType());
560-
}
561-
else {
562-
Indexer.this.exitTypeDescriptor = CodeFlow.toDescriptor(((Method)member).getReturnType());
563-
}
563+
Indexer.this.exitTypeDescriptor = CodeFlow.toDescriptor(member instanceof Method ?
564+
((Method) member).getReturnType() : ((Field) member).getType());
564565
}
565566
return accessor.read(this.evaluationContext, this.targetObject, this.name);
566567
}

‎spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ public void generateCode(MethodVisitor mv, CodeFlow cf) {
287287
throw new IllegalStateException("No applicable cached executor found: " + executorToCheck);
288288
}
289289

290-
ReflectiveMethodExecutor methodExecutor = (ReflectiveMethodExecutor)executorToCheck.get();
290+
ReflectiveMethodExecutor methodExecutor = (ReflectiveMethodExecutor) executorToCheck.get();
291291
Method method = methodExecutor.getMethod();
292292
boolean isStaticMethod = Modifier.isStatic(method.getModifiers());
293293
String descriptor = cf.lastDescriptor();
@@ -297,7 +297,8 @@ public void generateCode(MethodVisitor mv, CodeFlow cf) {
297297
// Nothing on the stack but something is needed
298298
cf.loadTarget(mv);
299299
}
300-
} else {
300+
}
301+
else {
301302
if (isStaticMethod) {
302303
// Something on the stack when nothing is needed
303304
mv.visitInsn(POP);
@@ -308,22 +309,18 @@ public void generateCode(MethodVisitor mv, CodeFlow cf) {
308309
CodeFlow.insertBoxIfNecessary(mv, descriptor.charAt(0));
309310
}
310311

311-
boolean itf = method.getDeclaringClass().isInterface();
312-
String methodDeclaringClassSlashedDescriptor = null;
313-
if (Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
314-
methodDeclaringClassSlashedDescriptor = method.getDeclaringClass().getName().replace('.', '/');
315-
}
316-
else {
317-
methodDeclaringClassSlashedDescriptor = methodExecutor.getPublicDeclaringClass().getName().replace('.', '/');
318-
}
312+
String classDesc = (Modifier.isPublic(method.getDeclaringClass().getModifiers()) ?
313+
method.getDeclaringClass().getName().replace('.', '/') :
314+
methodExecutor.getPublicDeclaringClass().getName().replace('.', '/'));
319315
if (!isStaticMethod) {
320-
if (descriptor == null || !descriptor.substring(1).equals(methodDeclaringClassSlashedDescriptor)) {
321-
CodeFlow.insertCheckCast(mv, "L"+ methodDeclaringClassSlashedDescriptor);
316+
if (descriptor == null || !descriptor.substring(1).equals(classDesc)) {
317+
CodeFlow.insertCheckCast(mv, "L" + classDesc);
322318
}
323319
}
324-
generateCodeForArguments(mv, cf, method, children);
325-
mv.visitMethodInsn(isStaticMethod ? INVOKESTATIC : INVOKEVIRTUAL,
326-
methodDeclaringClassSlashedDescriptor, method.getName(), CodeFlow.createSignatureDescriptor(method), itf);
320+
321+
generateCodeForArguments(mv, cf, method, this.children);
322+
mv.visitMethodInsn((isStaticMethod ? INVOKESTATIC : INVOKEVIRTUAL), classDesc, method.getName(),
323+
CodeFlow.createSignatureDescriptor(method), method.getDeclaringClass().isInterface());
327324
cf.pushDescriptor(this.exitTypeDescriptor);
328325
}
329326

‎spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java

Lines changed: 41 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2015 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -165,7 +165,7 @@ public TypedValue read(EvaluationContext context, Object target, String name) th
165165
return new TypedValue(value, invoker.typeDescriptor.narrow(value));
166166
}
167167
catch (Exception ex) {
168-
throw new AccessException("Unable to access property '" + name + "' through getter", ex);
168+
throw new AccessException("Unable to access property '" + name + "' through getter method", ex);
169169
}
170170
}
171171
}
@@ -187,12 +187,12 @@ public TypedValue read(EvaluationContext context, Object target, String name) th
187187
return new TypedValue(value, invoker.typeDescriptor.narrow(value));
188188
}
189189
catch (Exception ex) {
190-
throw new AccessException("Unable to access field: " + name, ex);
190+
throw new AccessException("Unable to access field '" + name + "'", ex);
191191
}
192192
}
193193
}
194194

195-
throw new AccessException("Neither getter nor field found for property '" + name + "'");
195+
throw new AccessException("Neither getter method nor field found for property '" + name + "'");
196196
}
197197

198198
@Override
@@ -240,7 +240,7 @@ public void write(EvaluationContext context, Object target, String name, Object
240240
newValue, TypeDescriptor.forObject(newValue), typeDescriptor);
241241
}
242242
catch (EvaluationException evaluationException) {
243-
throw new AccessException("Type conversion failure",evaluationException);
243+
throw new AccessException("Type conversion failure", evaluationException);
244244
}
245245
}
246246
CacheKey cacheKey = new CacheKey(type, name, target instanceof Class);
@@ -262,7 +262,7 @@ public void write(EvaluationContext context, Object target, String name, Object
262262
return;
263263
}
264264
catch (Exception ex) {
265-
throw new AccessException("Unable to access property '" + name + "' through setter", ex);
265+
throw new AccessException("Unable to access property '" + name + "' through setter method", ex);
266266
}
267267
}
268268
}
@@ -283,12 +283,12 @@ public void write(EvaluationContext context, Object target, String name, Object
283283
return;
284284
}
285285
catch (Exception ex) {
286-
throw new AccessException("Unable to access field: " + name, ex);
286+
throw new AccessException("Unable to access field '" + name + "'", ex);
287287
}
288288
}
289289
}
290290

291-
throw new AccessException("Neither setter nor field found for property '" + name + "'");
291+
throw new AccessException("Neither setter method nor field found for property '" + name + "'");
292292
}
293293

294294
private TypeDescriptor getTypeDescriptor(EvaluationContext context, Object target, String name) {
@@ -469,11 +469,11 @@ public PropertyAccessor createOptimalAccessor(EvaluationContext evalContext, Obj
469469
InvokerPair invocationTarget = this.readerCache.get(cacheKey);
470470

471471
if (invocationTarget == null || invocationTarget.member instanceof Method) {
472-
Method method = (Method) (invocationTarget==null?null:invocationTarget.member);
472+
Method method = (Method) (invocationTarget != null ? invocationTarget.member : null);
473473
if (method == null) {
474474
method = findGetterForProperty(name, type, target);
475475
if (method != null) {
476-
invocationTarget = new InvokerPair(method,new TypeDescriptor(new MethodParameter(method,-1)));
476+
invocationTarget = new InvokerPair(method, new TypeDescriptor(new MethodParameter(method, -1)));
477477
ReflectionUtils.makeAccessible(method);
478478
this.readerCache.put(cacheKey, invocationTarget);
479479
}
@@ -497,6 +497,7 @@ public PropertyAccessor createOptimalAccessor(EvaluationContext evalContext, Obj
497497
return new OptimalPropertyAccessor(invocationTarget);
498498
}
499499
}
500+
500501
return this;
501502
}
502503

@@ -577,16 +578,8 @@ public static class OptimalPropertyAccessor implements CompilablePropertyAccesso
577578
OptimalPropertyAccessor(InvokerPair target) {
578579
this.member = target.member;
579580
this.typeDescriptor = target.typeDescriptor;
580-
if (this.member instanceof Field) {
581-
Field field = (Field) this.member;
582-
this.needsToBeMadeAccessible = (!Modifier.isPublic(field.getModifiers()) ||
583-
!Modifier.isPublic(field.getDeclaringClass().getModifiers())) && !field.isAccessible();
584-
}
585-
else {
586-
Method method = (Method) this.member;
587-
this.needsToBeMadeAccessible = ((!Modifier.isPublic(method.getModifiers()) ||
588-
!Modifier.isPublic(method.getDeclaringClass().getModifiers())) && !method.isAccessible());
589-
}
581+
this.needsToBeMadeAccessible = (!Modifier.isPublic(this.member.getModifiers()) ||
582+
!Modifier.isPublic(this.member.getDeclaringClass().getModifiers()));
590583
}
591584

592585
@Override
@@ -599,10 +592,12 @@ public boolean canRead(EvaluationContext context, Object target, String name) th
599592
if (target == null) {
600593
return false;
601594
}
595+
602596
Class<?> type = (target instanceof Class ? (Class<?>) target : target.getClass());
603597
if (type.isArray()) {
604598
return false;
605599
}
600+
606601
if (this.member instanceof Method) {
607602
Method method = (Method) this.member;
608603
String getterName = "get" + StringUtils.capitalize(name);
@@ -621,30 +616,31 @@ public boolean canRead(EvaluationContext context, Object target, String name) th
621616
@Override
622617
public TypedValue read(EvaluationContext context, Object target, String name) throws AccessException {
623618
if (this.member instanceof Method) {
619+
Method method = (Method) this.member;
624620
try {
625-
if (this.needsToBeMadeAccessible) {
626-
ReflectionUtils.makeAccessible((Method) this.member);
621+
if (this.needsToBeMadeAccessible && !method.isAccessible()) {
622+
method.setAccessible(true);
627623
}
628-
Object value = ((Method) this.member).invoke(target);
624+
Object value = method.invoke(target);
629625
return new TypedValue(value, this.typeDescriptor.narrow(value));
630626
}
631627
catch (Exception ex) {
632-
throw new AccessException("Unable to access property '" + name + "' through getter", ex);
628+
throw new AccessException("Unable to access property '" + name + "' through getter method", ex);
633629
}
634630
}
635-
if (this.member instanceof Field) {
631+
else {
632+
Field field = (Field) this.member;
636633
try {
637-
if (this.needsToBeMadeAccessible) {
638-
ReflectionUtils.makeAccessible((Field) this.member);
634+
if (this.needsToBeMadeAccessible && !field.isAccessible()) {
635+
field.setAccessible(true);
639636
}
640-
Object value = ((Field) this.member).get(target);
637+
Object value = field.get(target);
641638
return new TypedValue(value, this.typeDescriptor.narrow(value));
642639
}
643640
catch (Exception ex) {
644-
throw new AccessException("Unable to access field: " + name, ex);
641+
throw new AccessException("Unable to access field '" + name + "'", ex);
645642
}
646643
}
647-
throw new AccessException("Neither getter nor field found for property '" + name + "'");
648644
}
649645

650646
@Override
@@ -665,40 +661,43 @@ public boolean isCompilable() {
665661

666662
@Override
667663
public Class<?> getPropertyType() {
668-
if (this.member instanceof Field) {
669-
return ((Field) this.member).getType();
664+
if (this.member instanceof Method) {
665+
return ((Method) this.member).getReturnType();
670666
}
671667
else {
672-
return ((Method) this.member).getReturnType();
668+
return ((Field) this.member).getType();
673669
}
674670
}
675671

676672
@Override
677673
public void generateCode(String propertyName, MethodVisitor mv, CodeFlow cf) {
678674
boolean isStatic = Modifier.isStatic(this.member.getModifiers());
679675
String descriptor = cf.lastDescriptor();
680-
String memberDeclaringClassSlashedDescriptor = this.member.getDeclaringClass().getName().replace('.', '/');
676+
String classDesc = this.member.getDeclaringClass().getName().replace('.', '/');
677+
681678
if (!isStatic) {
682679
if (descriptor == null) {
683680
cf.loadTarget(mv);
684681
}
685-
if (descriptor == null || !memberDeclaringClassSlashedDescriptor.equals(descriptor.substring(1))) {
686-
mv.visitTypeInsn(CHECKCAST, memberDeclaringClassSlashedDescriptor);
682+
if (descriptor == null || !classDesc.equals(descriptor.substring(1))) {
683+
mv.visitTypeInsn(CHECKCAST, classDesc);
687684
}
688-
} else {
685+
}
686+
else {
689687
if (descriptor != null) {
690688
// A static field/method call will not consume what is on the stack,
691689
// it needs to be popped off.
692690
mv.visitInsn(POP);
693691
}
694692
}
695-
if (this.member instanceof Field) {
696-
mv.visitFieldInsn(isStatic ? GETSTATIC : GETFIELD, memberDeclaringClassSlashedDescriptor,
697-
this.member.getName(), CodeFlow.toJvmDescriptor(((Field) this.member).getType()));
693+
694+
if (this.member instanceof Method) {
695+
mv.visitMethodInsn((isStatic ? INVOKESTATIC : INVOKEVIRTUAL), classDesc, this.member.getName(),
696+
CodeFlow.createSignatureDescriptor((Method) this.member), false);
698697
}
699698
else {
700-
mv.visitMethodInsn(isStatic ? INVOKESTATIC : INVOKEVIRTUAL, memberDeclaringClassSlashedDescriptor,
701-
this.member.getName(), CodeFlow.createSignatureDescriptor((Method) this.member),false);
699+
mv.visitFieldInsn((isStatic ? GETSTATIC : GETFIELD), classDesc, this.member.getName(),
700+
CodeFlow.toJvmDescriptor(((Field) this.member).getType()));
702701
}
703702
}
704703
}

0 commit comments

Comments
 (0)