diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ComponentDBAdapter.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ComponentDBAdapter.java index 8a003aefd9..20d38a68be 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ComponentDBAdapter.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ComponentDBAdapter.java @@ -4,9 +4,9 @@ * Licensed 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. @@ -19,6 +19,7 @@ import java.io.IOException; import db.*; import ghidra.framework.data.OpenMode; +import ghidra.util.StringUtilities; import ghidra.util.exception.VersionException; /** @@ -60,7 +61,8 @@ abstract class ComponentDBAdapter { * @param length the total length of this component. * @param ordinal the component's ordinal. * @param offset the component's offset. - * @param name the component's name. + * @param name the component's name. This name will be subject to revision based upon + * {@link #cleanUpFieldName(String)} method use. * @param comment a comment about this component * @return the component data type record. * @throws IOException if there is a problem accessing the database. @@ -86,6 +88,10 @@ abstract class ComponentDBAdapter { /** * Updates the component data type table with the provided record. + *

+ * IMPORTANT: Any modification of field name should be subject to {@link #cleanUpFieldName(String)} + * use first. + * * @param record the new record * @throws IOException if there is a problem accessing the database. */ diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ComponentDBAdapterV0.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ComponentDBAdapterV0.java index 5dc9ebee2d..8fc34311c6 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ComponentDBAdapterV0.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ComponentDBAdapterV0.java @@ -18,7 +18,7 @@ package ghidra.program.database.data; import java.io.IOException; import db.*; -import ghidra.util.StringUtilities; +import ghidra.program.model.data.InternalDataTypeComponent; import ghidra.util.exception.VersionException; /** @@ -74,16 +74,14 @@ class ComponentDBAdapterV0 extends ComponentDBAdapter { @Override DBRecord createRecord(long dataTypeID, long parentID, int length, int ordinal, int offset, String name, String comment) throws IOException { - // Don't allow whitespace in field names. Until we change the API to throw an exception - // when a field name has whitespace, just silently replace whitespace with underscores. - String fieldName = StringUtilities.whitespaceToUnderscores(name); long key = DataTypeManagerDB.createKey(DataTypeManagerDB.COMPONENT, componentTable.getKey()); DBRecord record = ComponentDBAdapter.COMPONENT_SCHEMA.createRecord(key); record.setLongValue(ComponentDBAdapter.COMPONENT_PARENT_ID_COL, parentID); record.setLongValue(ComponentDBAdapter.COMPONENT_OFFSET_COL, offset); record.setLongValue(ComponentDBAdapter.COMPONENT_DT_ID_COL, dataTypeID); - record.setString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL, fieldName); + record.setString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL, + InternalDataTypeComponent.cleanupFieldName(name)); record.setString(ComponentDBAdapter.COMPONENT_COMMENT_COL, comment); record.setIntValue(ComponentDBAdapter.COMPONENT_SIZE_COL, length); record.setIntValue(ComponentDBAdapter.COMPONENT_ORDINAL_COL, ordinal); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeComponentDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeComponentDB.java index a3c9c6cbd5..89d3598083 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeComponentDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeComponentDB.java @@ -228,7 +228,9 @@ class DataTypeComponentDB implements InternalDataTypeComponent { @Override public String getFieldName() { if (record != null && !isZeroBitFieldComponent()) { - return record.getString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL); + String fieldName = record.getString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL); + // Blank check is required since we improperly allowed storage of blank names in the past + return StringUtils.isBlank(fieldName) ? null : fieldName; } return null; } @@ -236,7 +238,7 @@ class DataTypeComponentDB implements InternalDataTypeComponent { @Override public void setFieldName(String name) throws DuplicateNameException { if (record != null) { - String fieldName = cleanupFieldName(name); + String fieldName = InternalDataTypeComponent.cleanupFieldName(name); record.setString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL, fieldName); updateRecord(true); } @@ -415,7 +417,7 @@ class DataTypeComponentDB implements InternalDataTypeComponent { if (StringUtils.isBlank(comment)) { comment = null; } - String fieldName = cleanupFieldName(name); + String fieldName = InternalDataTypeComponent.cleanupFieldName(name); record.setString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL, fieldName); record.setLongValue(ComponentDBAdapter.COMPONENT_DT_ID_COL, dataMgr.getResolvedID(dt)); record.setString(ComponentDBAdapter.COMPONENT_COMMENT_COL, comment); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeComponentImpl.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeComponentImpl.java index 1493165b34..4519b49a1b 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeComponentImpl.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeComponentImpl.java @@ -58,7 +58,7 @@ public class DataTypeComponentImpl implements InternalDataTypeComponent, Seriali this.ordinal = ordinal; this.offset = offset; this.length = length; - this.fieldName = cleanupFieldName(fieldName); + this.fieldName = InternalDataTypeComponent.cleanupFieldName(fieldName); setDataType(dataType); setComment(comment); } @@ -130,7 +130,7 @@ public class DataTypeComponentImpl implements InternalDataTypeComponent, Seriali @Override public void setFieldName(String name) throws DuplicateNameException { - this.fieldName = cleanupFieldName(name); + this.fieldName = InternalDataTypeComponent.cleanupFieldName(name); } public static void checkDefaultFieldName(String fieldName) throws DuplicateNameException { @@ -171,7 +171,7 @@ public class DataTypeComponentImpl implements InternalDataTypeComponent, Seriali * @param newComment new comment */ void update(String name, DataType newDataType, String newComment) { - this.fieldName = cleanupFieldName(name); + this.fieldName = InternalDataTypeComponent.cleanupFieldName(name); this.dataType = newDataType; this.comment = StringUtils.isBlank(newComment) ? null : newComment; } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/InternalDataTypeComponent.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/InternalDataTypeComponent.java index f2bcc5eb5e..9f17790c50 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/InternalDataTypeComponent.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/InternalDataTypeComponent.java @@ -15,8 +15,6 @@ */ package ghidra.program.model.data; -import org.apache.commons.lang3.StringUtils; - import ghidra.util.StringUtilities; public interface InternalDataTypeComponent extends DataTypeComponent { @@ -56,16 +54,29 @@ public interface InternalDataTypeComponent extends DataTypeComponent { } /** - * Internal method for cleaning up field names. - * @param name the new field name - * @return the name with bad chars removed and also set back to null in the event - * the new name is the default name. + * Modify field name to transform whitespace chars to underscores after triming and checking + * for empty string. Empty string is returned as null for storage to indicate default name use. + * @param name original field name (may be null) + * @return revised field name (may be null) */ - public default String cleanupFieldName(String name) { - // For now, silently convert whitespace to underscores - String fieldName = StringUtilities.whitespaceToUnderscores(name); - if (StringUtils.isBlank(fieldName)) { - fieldName = null; + public static String cleanupFieldName(String name) { + String fieldName = name; + if (name != null) { + + // Trim field name and ensure empty string is stored as null to indicate default field name + fieldName = name.trim(); + + if (fieldName.length() == 0) { + fieldName = null; + } + else { + // NOTE: Should we be checking for default field name pattern and disallow. + // If so, additional parameters would be required (e.g., struct vs union, is packed struct) + + // Don't allow whitespace in field names. Until we change the API to throw an exception + // when a field name has whitespace, just silently replace whitespace with underscores. + fieldName = StringUtilities.whitespaceToUnderscores(fieldName); + } } return fieldName; } diff --git a/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/data/StructureDBTest.java b/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/data/StructureDBTest.java index ca0eb92a0e..4a5e80f535 100644 --- a/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/data/StructureDBTest.java +++ b/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/data/StructureDBTest.java @@ -2663,15 +2663,58 @@ public class StructureDBTest extends AbstractGenericTest { struct = (StructureDB) dataMgr.resolve(newStruct, null); component = struct.getComponent(0); - component.setFieldName("name in db with spaces"); + component.setFieldName(" name in db with spaces "); assertEquals("name_in_db_with_spaces", component.getFieldName()); - component = struct.add(new ByteDataType(), "another test", null); + component = struct.add(new ByteDataType(), " another test ", null); assertEquals("another_test", component.getFieldName()); - struct.insert(0, new ByteDataType(), 1, "insert test", ""); + struct.insert(0, new ByteDataType(), 1, " insert test ", ""); + component = struct.getComponent(0); + assertEquals("insert_test", component.getFieldName()); + + struct.replace(0, new ByteDataType(), 1, " insert test ", ""); component = struct.getComponent(0); assertEquals("insert_test", component.getFieldName()); } + @Test + public void testDefaultFieldNames() throws DuplicateNameException { + StructureDataType newStruct = new StructureDataType("Test", 0); + DataTypeComponent component = newStruct.add(new ByteDataType(), " ", null); + assertNull(component.getFieldName()); + + component = newStruct.add(new ByteDataType(), null, null); + assertNull(component.getFieldName()); + + struct = (StructureDB) dataMgr.resolve(newStruct, null); + component = struct.getComponent(0); + assertNull(component.getFieldName()); + + component.setFieldName(" "); + assertNull(component.getFieldName()); + + component = struct.add(new ByteDataType(), null, null); + assertNull(component.getFieldName()); + + component = struct.add(new ByteDataType(), " ", null); + assertNull(component.getFieldName()); + + struct.insert(0, new ByteDataType(), 1, null, ""); + component = struct.getComponent(0); + assertNull(component.getFieldName()); + + struct.insert(0, new ByteDataType(), 1, " ", ""); + component = struct.getComponent(0); + assertNull(component.getFieldName()); + + struct.replace(0, new ByteDataType(), 1, null, ""); + component = struct.getComponent(0); + assertNull(component.getFieldName()); + + struct.replace(0, new ByteDataType(), 1, " ", ""); + component = struct.getComponent(0); + assertNull(component.getFieldName()); + } + }