Skip to content

Commit

Permalink
OPENJPA-2917 fix off-by-1 issue on java stack
Browse files Browse the repository at this point in the history
long parameters take up 2 byte on the call stack
  • Loading branch information
struberg committed Oct 17, 2023
1 parent 33372a7 commit 815ef23
Show file tree
Hide file tree
Showing 7 changed files with 196 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@
*/
public class PCDataGenerator extends DynamicStorageGenerator {

private static final Localizer _loc = Localizer.forPackage
(PCDataGenerator.class);
private static final Localizer _loc = Localizer.forPackage(PCDataGenerator.class);

protected static final String POSTFIX = "$openjpapcdata";

Expand All @@ -89,8 +88,10 @@ public OpenJPAConfiguration getConfiguration() {
* Return a {@link PCData} instance for the given oid and metadata.
*/
public PCData generatePCData(Object oid, ClassMetaData meta) {
if (meta == null)
if (meta == null) {
return null;
}

Class<?> type = meta.getDescribedType();
DynamicStorage storage = _generated.get(type);
if (storage == null) {
Expand All @@ -110,13 +111,15 @@ public PCData generatePCData(Object oid, ClassMetaData meta) {
* Actually generate the factory instance.
*/
private DynamicStorage generateStorage(ClassMetaData meta) {
if (_log.isTraceEnabled())
if (_log.isTraceEnabled()) {
_log.trace(_loc.get("pcdata-generate", meta));
}

FieldMetaData[] fields = meta.getFields();
int[] types = new int[fields.length];
for (int i = 0; i < types.length; i++)
for (int i = 0; i < types.length; i++) {
types[i] = replaceType(fields[i]);
}
return generateStorage(types, meta);
}

Expand All @@ -128,8 +131,10 @@ protected void finish(DynamicPCData data, ClassMetaData meta) {

@Override
protected int getCreateFieldMethods(int typeCode) {
if (typeCode >= JavaTypes.OBJECT)
if (typeCode >= JavaTypes.OBJECT) {
return POLICY_SILENT;
}

// don't bother creating set/get<Primitive> methods
return POLICY_EMPTY;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1215,6 +1215,7 @@ private AbstractInsnNode addNotifyMutation(ClassNode classNode, MethodNode metho
addGetManagedValueCode(classNode, insns, fmd, true);
}
else {
// the method parameter
insns.add(new VarInsnNode(AsmHelper.getLoadInsn(type), param + 1));
}
insns.add(new MethodInsnNode(Opcodes.INVOKESTATIC,
Expand Down Expand Up @@ -4195,7 +4196,7 @@ private void addSubclassSetMethod(ClassNode classNode, FieldMetaData fmd) throws
null, null);
classNode.methods.add(newMethod);
final InsnList instructions = newMethod.instructions;
int nextFreeVarPos = 2;
int nextFreeVarPos = 1 + Type.getType(propType).getSize();

setVisibilityToSuperMethod(newMethod);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@
import org.apache.openjpa.conf.OpenJPAConfiguration;
import org.apache.openjpa.conf.OpenJPAConfigurationImpl;
import org.apache.openjpa.lib.conf.Configurations;
import org.apache.openjpa.util.asm.AsmHelper;
import org.apache.openjpa.util.asm.BytecodeWriter;
import org.apache.openjpa.lib.util.J2DoPrivHelper;
import org.apache.openjpa.lib.util.Options;
import org.apache.openjpa.meta.MetaDataRepository;
import org.apache.openjpa.persistence.test.AbstractCachedEMFTestCase;
import org.apache.openjpa.util.asm.ClassNodeTracker;
import org.apache.openjpa.util.asm.EnhancementClassLoader;
import org.apache.openjpa.util.asm.EnhancementProject;
import org.apache.xbean.asm9.Type;

Expand All @@ -52,10 +54,23 @@ public void testExplicitEnhancementWithClassNotInFirstPU() throws ClassNotFoundE
ClassNodeTracker bc = assertNotPC(loader, project, className);

PCEnhancer enhancer = new PCEnhancer(conf, bc, repos, loader);
enhancer.setCreateSubclass(true);

assertEquals(PCEnhancer.ENHANCE_PC, enhancer.run());

assertTrue(enhancer.getPCBytecode().getClassNode().interfaces.contains(Type.getInternalName(PersistenceCapable.class)));

// load the Class<?> for real.
EnhancementProject finalProject = new EnhancementProject();
EnhancementClassLoader finalLoader = new EnhancementClassLoader(finalProject, this.getClass().getClassLoader());
final byte[] classBytes2 = AsmHelper.toByteArray(enhancer.getPCBytecode());

// this is just to make the ClassLoader aware of the bytecode for the enhanced class
finalProject.loadClass(classBytes2, finalLoader);

String pcClassName = enhancer.getPCBytecode().getClassNode().name.replace("/", ".");
final Class<?> implClass = Class.forName(pcClassName, true, finalLoader);
assertNotNull(implClass);
}

private ClassNodeTracker assertNotPC(ClassLoader loader, EnhancementProject project, String className) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,48 @@

import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import jakarta.persistence.IdClass;

@Entity
@IdClass(UnenhancedBootstrapInstanceId.class)
public class UnenhancedBootstrapInstance {
private long billNumber;
private long billVersion;
private long billRevision;
private String billDescription;

@Id
public long getBillNumber() {
return billNumber;
}

public void setBillNumber(final long number) {
billNumber = number;
}

@Id
public long getBillVersion() {
return billVersion;
}

public void setBillVersion(final long version) {
billVersion = version;
}

@Id
private int id;
public long getBillRevision() {
return billRevision;
}

public void setBillRevision(final long revision) {
billRevision = revision;
}

public String getBillDescription() {
return billDescription;
}

public void setBillDescription(final String description) {
billDescription = description;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.openjpa.enhance;

import java.io.Serializable;

/**
*
*/
public class UnenhancedBootstrapInstanceId implements Serializable {
private long billNumber;
private long billVersion;
private long billRevision;

public UnenhancedBootstrapInstanceId() {

}

public UnenhancedBootstrapInstanceId(final long number, final long version, final long revision) {
this.billNumber = number;
this.billVersion = version;
this.billRevision = revision;
}

public long getBillNumber() {
return this.billNumber;
}

public void setBillNumber(final long number) {
this.billNumber = number;
}

public long getBillVersion() {
return this.billVersion;
}

public void setBillVersion(final long version) {
this.billVersion = version;
}

public long getBillRevision() {
return this.billRevision;
}

public void setBillRevision(final long revision) {
this.billRevision = revision;
}

public boolean equals(final Object obj) {
if (obj == this)
return true;

if (!(obj instanceof UnenhancedBootstrapInstanceId))
return false;

final UnenhancedBootstrapInstanceId pk = (UnenhancedBootstrapInstanceId) obj;

if (billNumber != pk.billNumber)
return false;

if (billVersion != pk.billVersion)
return false;

if (billRevision != pk.billRevision)
return false;

return true;
}

public int hashCode() {
return (billNumber + "." + billVersion + "." + billRevision).hashCode();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ public class UnenhancedFieldAccess
@Version public int version;
protected String stringField = "foo";

private long longVal;

@Basic(fetch = FetchType.LAZY)
private String lazyField = "lazy";

Expand All @@ -65,6 +67,15 @@ public String getLazyField() {
return lazyField;
}


public long getLongVal() {
return longVal;
}

public void setLongVal(long longVal) {
this.longVal = longVal;
}

@Override
public boolean equals(Object o) {
if (o == this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,36 +65,36 @@ public void setUp() throws Exception {
}

public void testSimpleCompoundIdTestEntity() throws Exception {
EntityManager em = emf.createEntityManager();
String jpql = "SELECT o FROM SimpleCompoundIdTestEntity o ORDER BY o.secondId";
List<SimpleCompoundIdTestEntity> list = em.createQuery(jpql,SimpleCompoundIdTestEntity.class)
.getResultList();
assertEquals(2, list.size());
assertEquals(Long.valueOf(123), list.get(0).getSecondId());

SimpleCompoundIdTestEntity secondResult = list.get(1);
assertNotNull("BUG (JIRA-1397)! Result list contains null in second element", secondResult);
assertNull(secondResult.getSecondId());
em.close();
}


public void testComplexCompoundIdTestEntity() throws Exception {
EntityManager em = emf.createEntityManager();
String jpql = "SELECT o FROM ComplexCompoundIdTestEntity o ORDER BY o.type";
List<ComplexCompoundIdTestEntity> list = em.createQuery(jpql,ComplexCompoundIdTestEntity.class)
.getResultList();
assertEquals(2, list.size());
ComplexCompoundIdTestEntity secondResult = list.get(1);
assertNotNull("Result list contains null in second element", secondResult);
assertNull("Result list's second record secondId field was not null", secondResult.getType());
em.close();
}

/**
* Create tables with logical compound keys without non-null constraint.
* Populate them with null values in some of the columns.
*/
EntityManager em = emf.createEntityManager();
String jpql = "SELECT o FROM SimpleCompoundIdTestEntity o ORDER BY o.secondId";
List<SimpleCompoundIdTestEntity> list = em.createQuery(jpql,SimpleCompoundIdTestEntity.class)
.getResultList();
assertEquals(2, list.size());
assertEquals(Long.valueOf(123), list.get(0).getSecondId());

SimpleCompoundIdTestEntity secondResult = list.get(1);
assertNotNull("BUG (JIRA-1397)! Result list contains null in second element", secondResult);
assertNull(secondResult.getSecondId());
em.close();
}


public void testComplexCompoundIdTestEntity() throws Exception {
EntityManager em = emf.createEntityManager();
String jpql = "SELECT o FROM ComplexCompoundIdTestEntity o ORDER BY o.type";
List<ComplexCompoundIdTestEntity> list = em.createQuery(jpql,ComplexCompoundIdTestEntity.class)
.getResultList();
assertEquals(2, list.size());
ComplexCompoundIdTestEntity secondResult = list.get(1);
assertNotNull("Result list contains null in second element", secondResult);
assertNull("Result list's second record secondId field was not null", secondResult.getType());
em.close();
}

/**
* Create tables with logical compound keys without non-null constraint.
* Populate them with null values in some of the columns.
*/
private void createTables(EntityManager em) throws Exception {
em.getTransaction().begin();
OpenJPAEntityManager kem = OpenJPAPersistence.cast(em);
Expand Down

0 comments on commit 815ef23

Please sign in to comment.