From ef00779b4e28ac932b0390b23db6de1e5312a696 Mon Sep 17 00:00:00 2001 From: Kim Rader Date: Wed, 15 Nov 2023 17:13:05 -0800 Subject: [PATCH 1/4] Implement pureChecks for file handler classes Signed-off-by: Kim Rader --- .../file/impl/handlers/FileAppendHandler.java | 18 ++++++++++++++++-- .../file/impl/handlers/FileCreateHandler.java | 19 +++++++++++++++---- .../file/impl/handlers/FileDeleteHandler.java | 17 ++++++++++++++++- .../handlers/FileSystemDeleteHandler.java | 16 +++++++++++++++- .../handlers/FileSystemUndeleteHandler.java | 16 +++++++++++++++- .../file/impl/handlers/FileUpdateHandler.java | 16 +++++++++++++++- .../file/impl/utils/FileServiceUtils.java | 13 +++++-------- 7 files changed, 97 insertions(+), 18 deletions(-) diff --git a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileAppendHandler.java b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileAppendHandler.java index c053739ca54d..b6cbfabf7b2f 100644 --- a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileAppendHandler.java +++ b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileAppendHandler.java @@ -32,6 +32,7 @@ import com.hedera.hapi.node.base.SubType; import com.hedera.hapi.node.file.FileAppendTransactionBody; import com.hedera.hapi.node.state.file.File; +import com.hedera.hapi.node.transaction.TransactionBody; import com.hedera.node.app.service.file.ReadableFileStore; import com.hedera.node.app.service.file.impl.WritableFileStore; import com.hedera.node.app.service.file.impl.WritableUpgradeFileStore; @@ -65,6 +66,19 @@ public FileAppendHandler() { // Exists for injection } + /** + * Performs checks independent of state or context + * @param txn the transaction to check + */ + @Override + public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckException { + final FileAppendTransactionBody transactionBody = txn.fileAppendOrThrow(); + + if (transactionBody.fileID() == null) { + throw new PreCheckException(INVALID_FILE_ID); + } + } + /** * This method is called during the pre-handle workflow. * @@ -79,8 +93,8 @@ public void preHandle(@NonNull final PreHandleContext context) throws PreCheckEx final var transactionBody = context.body().fileAppendOrThrow(); final var fileStore = context.createStore(ReadableFileStore.class); - final var transactionFileId = requireNonNull(transactionBody.fileID()); - preValidate(transactionFileId, fileStore, context, false); + final var transactionFileId = transactionBody.fileIDOrThrow(); + preValidate(transactionFileId, fileStore, context); var file = fileStore.getFileLeaf(transactionFileId); validateAndAddRequiredKeys(file, null, context); diff --git a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileCreateHandler.java b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileCreateHandler.java index eeecec898dbc..25a1ad3a6f83 100644 --- a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileCreateHandler.java +++ b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileCreateHandler.java @@ -29,7 +29,9 @@ import com.hedera.hapi.node.base.HederaFunctionality; import com.hedera.hapi.node.base.KeyList; import com.hedera.hapi.node.base.SubType; +import com.hedera.hapi.node.file.FileCreateTransactionBody; import com.hedera.hapi.node.state.file.File; +import com.hedera.hapi.node.transaction.TransactionBody; import com.hedera.node.app.hapi.fees.usage.file.FileOpsUsage; import com.hedera.node.app.service.file.impl.WritableFileStore; import com.hedera.node.app.service.file.impl.records.CreateFileRecordBuilder; @@ -60,6 +62,19 @@ public FileCreateHandler(final FileOpsUsage fileOpsUsage) { this.fileOpsUsage = fileOpsUsage; } + /** + * Performs checks independent of state or context + * @param txn the transaction to check + */ + @Override + public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckException { + final FileCreateTransactionBody transactionBody = txn.fileCreateOrThrow(); + + if (!transactionBody.hasExpirationTime()) { + throw new PreCheckException(INVALID_EXPIRATION_TIME); + } + } + /** * This method is called during the pre-handle workflow. * @@ -75,10 +90,6 @@ public void preHandle(@NonNull final PreHandleContext context) throws PreCheckEx final var transactionBody = context.body().fileCreateOrThrow(); validateAndAddRequiredKeys(null, transactionBody.keys(), context); - - if (!transactionBody.hasExpirationTime()) { - throw new PreCheckException(INVALID_EXPIRATION_TIME); - } } @Override diff --git a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileDeleteHandler.java b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileDeleteHandler.java index c17ea7735cd4..dd2b7f97d333 100644 --- a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileDeleteHandler.java +++ b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileDeleteHandler.java @@ -27,7 +27,9 @@ import com.hedera.hapi.node.base.HederaFunctionality; import com.hedera.hapi.node.base.SubType; +import com.hedera.hapi.node.file.FileDeleteTransactionBody; import com.hedera.hapi.node.state.file.File; +import com.hedera.hapi.node.transaction.TransactionBody; import com.hedera.node.app.hapi.utils.fee.FileFeeBuilder; import com.hedera.node.app.service.file.ReadableFileStore; import com.hedera.node.app.service.file.impl.WritableFileStore; @@ -58,6 +60,19 @@ public FileDeleteHandler(final FileFeeBuilder usageEstimator) { this.usageEstimator = usageEstimator; } + /** + * Performs checks independent of state or context + * @param txn the transaction to check + */ + @Override + public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckException { + final FileDeleteTransactionBody transactionBody = txn.fileDeleteOrThrow(); + + if (transactionBody.fileID() == null) { + throw new PreCheckException(INVALID_FILE_ID); + } + } + /** * This method is called during the pre-handle workflow. * @@ -74,7 +89,7 @@ public void preHandle(@NonNull final PreHandleContext context) throws PreCheckEx final var transactionBody = context.body().fileDeleteOrThrow(); final var fileStore = context.createStore(ReadableFileStore.class); final var transactionFileId = requireNonNull(transactionBody.fileID()); - preValidate(transactionFileId, fileStore, context, true); + preValidate(transactionFileId, fileStore, context); var file = fileStore.getFileLeaf(transactionFileId); validateAndAddRequiredKeysForDelete(file, context); diff --git a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileSystemDeleteHandler.java b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileSystemDeleteHandler.java index 0327133e89f5..1644c205e61e 100644 --- a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileSystemDeleteHandler.java +++ b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileSystemDeleteHandler.java @@ -26,6 +26,7 @@ import com.hedera.hapi.node.base.SubType; import com.hedera.hapi.node.base.TimestampSeconds; import com.hedera.hapi.node.state.file.File; +import com.hedera.hapi.node.transaction.TransactionBody; import com.hedera.node.app.hapi.utils.fee.FileFeeBuilder; import com.hedera.node.app.service.file.ReadableFileStore; import com.hedera.node.app.service.file.impl.WritableFileStore; @@ -55,6 +56,19 @@ public FileSystemDeleteHandler(final FileFeeBuilder usageEstimator) { this.usageEstimator = usageEstimator; } + /** + * Performs checks independent of state or context + * @param txn the transaction to check + */ + @Override + public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckException { + final var transactionBody = txn.systemDeleteOrThrow(); + + if (transactionBody.fileID() == null) { + throw new PreCheckException(INVALID_FILE_ID); + } + } + /** * This method is called during the pre-handle workflow. * @@ -71,7 +85,7 @@ public void preHandle(@NonNull final PreHandleContext context) throws PreCheckEx final var transactionBody = context.body().systemDeleteOrThrow(); final var fileStore = context.createStore(ReadableFileStore.class); final var transactionFileId = requireNonNull(transactionBody.fileID()); - preValidate(transactionFileId, fileStore, context, true); + preValidate(transactionFileId, fileStore, context); } @Override diff --git a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileSystemUndeleteHandler.java b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileSystemUndeleteHandler.java index 17ddfb36db30..36c12203a4c9 100644 --- a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileSystemUndeleteHandler.java +++ b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileSystemUndeleteHandler.java @@ -24,6 +24,7 @@ import com.hedera.hapi.node.base.HederaFunctionality; import com.hedera.hapi.node.base.SubType; import com.hedera.hapi.node.state.file.File; +import com.hedera.hapi.node.transaction.TransactionBody; import com.hedera.node.app.hapi.utils.fee.FileFeeBuilder; import com.hedera.node.app.service.file.ReadableFileStore; import com.hedera.node.app.service.file.impl.WritableFileStore; @@ -54,6 +55,19 @@ public FileSystemUndeleteHandler(final FileFeeBuilder usageEstimator) { this.usageEstimator = usageEstimator; } + /** + * Performs checks independent of state or context + * @param txn the transaction to check + */ + @Override + public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckException { + final var transactionBody = txn.systemUndeleteOrThrow(); + + if (transactionBody.fileID() == null) { + throw new PreCheckException(INVALID_FILE_ID); + } + } + /** * This method is called during the pre-handle workflow. * @@ -70,7 +84,7 @@ public void preHandle(@NonNull final PreHandleContext context) throws PreCheckEx final var transactionBody = context.body().systemUndeleteOrThrow(); final var fileStore = context.createStore(ReadableFileStore.class); final var transactionFileId = requireNonNull(transactionBody.fileID()); - preValidate(transactionFileId, fileStore, context, true); + preValidate(transactionFileId, fileStore, context); } @Override diff --git a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileUpdateHandler.java b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileUpdateHandler.java index e7ac3978395c..fc9c9f3b7b22 100644 --- a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileUpdateHandler.java +++ b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileUpdateHandler.java @@ -35,6 +35,7 @@ import com.hedera.hapi.node.base.Timestamp; import com.hedera.hapi.node.file.FileUpdateTransactionBody; import com.hedera.hapi.node.state.file.File; +import com.hedera.hapi.node.transaction.TransactionBody; import com.hedera.node.app.hapi.fees.usage.file.FileOpsUsage; import com.hedera.node.app.service.file.ReadableFileStore; import com.hedera.node.app.service.file.impl.WritableFileStore; @@ -69,6 +70,19 @@ public FileUpdateHandler(final FileOpsUsage fileOpsUsage) { this.fileOpsUsage = fileOpsUsage; } + /** + * Performs checks independent of state or context + * @param txn the transaction to check + */ + @Override + public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckException { + final var transactionBody = txn.fileUpdateOrThrow(); + + if (transactionBody.fileID() == null) { + throw new PreCheckException(INVALID_FILE_ID); + } + } + /** * This method is called during the pre-handle workflow. * @@ -84,7 +98,7 @@ public void preHandle(@NonNull final PreHandleContext context) throws PreCheckEx final var transactionBody = context.body().fileUpdateOrThrow(); final var fileStore = context.createStore(ReadableFileStore.class); final var transactionFileId = requireNonNull(transactionBody.fileID()); - preValidate(transactionFileId, fileStore, context, false); + preValidate(transactionFileId, fileStore, context); var file = fileStore.getFileLeaf(transactionFileId); if (wantsToMutateNonExpiryField(transactionBody)) { diff --git a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/utils/FileServiceUtils.java b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/utils/FileServiceUtils.java index ecccf0d7aeb0..67c86f4da834 100644 --- a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/utils/FileServiceUtils.java +++ b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/utils/FileServiceUtils.java @@ -63,23 +63,20 @@ public static void validateContent(@NonNull byte[] content, @NonNull FilesConfig } /** - * The function validates that the fileId is non-null, not a reserved system Id, and matches a file in the store. + * The function validates that the fileId is not a reserved system Id and that it matches a file in the store. * * @param fileId the file id to validate and to fetch the metadata * @param fileStore the file store to fetch the metadata of specified file id + * @param context the prehandle context for the transaction * @throws PreCheckException if the file id is invalid or the file does not exist */ public static void preValidate( - @Nullable final FileID fileId, + @NonNull final FileID fileId, @NonNull final ReadableFileStore fileStore, - @NonNull final PreHandleContext context, - boolean isDelete) + @NonNull final PreHandleContext context) throws PreCheckException { requireNonNull(context); - - if (fileId == null) { - throw new PreCheckException(INVALID_FILE_ID); - } + requireNonNull(fileId); final var fileConfig = context.configuration().getConfigData(FilesConfig.class); From 4742555d5bb66072bbe56c88bb5e09e1be411fe0 Mon Sep 17 00:00:00 2001 From: Kim Rader Date: Wed, 15 Nov 2023 19:06:13 -0800 Subject: [PATCH 2/4] Fix unit test Signed-off-by: Kim Rader --- .../service/file/impl/test/handlers/FileCreateTest.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hedera-node/hedera-file-service-impl/src/test/java/com/hedera/node/app/service/file/impl/test/handlers/FileCreateTest.java b/hedera-node/hedera-file-service-impl/src/test/java/com/hedera/node/app/service/file/impl/test/handlers/FileCreateTest.java index e7fb5ab4a092..d30f37a37337 100644 --- a/hedera-node/hedera-file-service-impl/src/test/java/com/hedera/node/app/service/file/impl/test/handlers/FileCreateTest.java +++ b/hedera-node/hedera-file-service-impl/src/test/java/com/hedera/node/app/service/file/impl/test/handlers/FileCreateTest.java @@ -165,18 +165,19 @@ void createWithEmptyKeys() throws PreCheckException { } @Test - @DisplayName("no expatriation time is added") + @DisplayName("no expiration time is added") void createAddsDifferentSubmitKey() throws PreCheckException { // given: final var payerKey = mockPayerLookup(); final var keys = anotherKeys; // when: - final var context = new FakePreHandleContext(accountStore, newCreateTxn(keys, 0)); + final var txn = newCreateTxn(keys, 0); + final var context = new FakePreHandleContext(accountStore, txn); // then: assertThat(context.payerKey()).isEqualTo(payerKey); - assertThrowsPreCheck(() -> subject.preHandle(context), INVALID_EXPIRATION_TIME); + assertThrowsPreCheck(() -> subject.pureChecks(txn), INVALID_EXPIRATION_TIME); } @Test From 5994d96252ba03d18698e9804a29e85a0e7abe8d Mon Sep 17 00:00:00 2001 From: Kim Rader Date: Thu, 16 Nov 2023 10:58:53 -0800 Subject: [PATCH 3/4] Added one more check in FileAppend pureChecks Signed-off-by: Kim Rader --- .../app/service/file/impl/handlers/FileAppendHandler.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileAppendHandler.java b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileAppendHandler.java index b6cbfabf7b2f..c1d37c57de9e 100644 --- a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileAppendHandler.java +++ b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileAppendHandler.java @@ -77,6 +77,9 @@ public void pureChecks(@NonNull final TransactionBody txn) throws PreCheckExcept if (transactionBody.fileID() == null) { throw new PreCheckException(INVALID_FILE_ID); } + if (transactionBody.contents() == null || transactionBody.contents().length() == 0) { + throw new PreCheckException(FILE_CONTENT_EMPTY); + } } /** @@ -108,11 +111,11 @@ public void handle(@NonNull final HandleContext handleContext) throws HandleExce final var target = fileAppend.fileID(); final var data = fileAppend.contents(); final var fileServiceConfig = handleContext.configuration().getConfigData(FilesConfig.class); - if (data == null || data.length() <= 0) { + if (data == null || data.length() <= 0) { // should never happen, this is checked in pureChecks logger.debug("FileAppend: No data to append"); } - if (target == null) { + if (target == null) { // should never happen, this is checked in pureChecks throw new HandleException(INVALID_FILE_ID); } From 059e8b73562976d0ffadf773f170982729f8624d Mon Sep 17 00:00:00 2001 From: Kim Rader Date: Thu, 16 Nov 2023 12:17:59 -0800 Subject: [PATCH 4/4] Spotless --- .../app/service/file/impl/handlers/FileAppendHandler.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileAppendHandler.java b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileAppendHandler.java index c1d37c57de9e..11f45b73a6e8 100644 --- a/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileAppendHandler.java +++ b/hedera-node/hedera-file-service-impl/src/main/java/com/hedera/node/app/service/file/impl/handlers/FileAppendHandler.java @@ -111,11 +111,11 @@ public void handle(@NonNull final HandleContext handleContext) throws HandleExce final var target = fileAppend.fileID(); final var data = fileAppend.contents(); final var fileServiceConfig = handleContext.configuration().getConfigData(FilesConfig.class); - if (data == null || data.length() <= 0) { // should never happen, this is checked in pureChecks + if (data == null || data.length() <= 0) { // should never happen, this is checked in pureChecks logger.debug("FileAppend: No data to append"); } - if (target == null) { // should never happen, this is checked in pureChecks + if (target == null) { // should never happen, this is checked in pureChecks throw new HandleException(INVALID_FILE_ID); }