From 5a6daf43528bc1b6d63bff65e1220d4873990389 Mon Sep 17 00:00:00 2001 From: srikappa-amzn Date: Wed, 8 Sep 2021 02:02:24 -0700 Subject: [PATCH 1/3] Maximize read file size limit and set it to 1MiB for Atom use cases Signed-off-by: srikappa-amzn --- .../AzCore/AzCore/Serialization/Json/JsonUtils.cpp | 9 ++------- .../AzCore/AzCore/Serialization/Json/JsonUtils.h | 3 ++- Code/Framework/AzCore/AzCore/Utils/Utils.h | 7 ++----- .../Asset/Shader/Code/Source/Editor/AzslCompiler.cpp | 4 ++-- .../Shader/Code/Source/Editor/ShaderAssetBuilder.cpp | 3 ++- .../Shader/Code/Source/Editor/ShaderBuilderUtility.cpp | 9 +++++---- .../Code/Source/Editor/ShaderVariantAssetBuilder.cpp | 10 +++++----- .../RPI/Code/Include/Atom/RPI.Edit/Common/JsonUtils.h | 6 +++++- .../Source/RPI.Builders/Material/MaterialBuilder.cpp | 5 +++-- .../RPI.Edit/Material/MaterialSourceDataSerializer.cpp | 3 ++- .../Code/Source/RPI.Edit/Material/MaterialUtils.cpp | 3 ++- .../Editor/AssetCollectionAsyncLoaderTestComponent.cpp | 3 ++- 12 files changed, 34 insertions(+), 31 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonUtils.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonUtils.cpp index af35842afc..9498b67e79 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonUtils.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonUtils.cpp @@ -240,11 +240,6 @@ namespace AZ { IO::SizeType length = stream.GetLength(); - if (length > AZ::Utils::DefaultMaxFileSize) - { - return AZ::Failure(AZStd::string{ "Data is too large." }); - } - AZStd::vector memoryBuffer; memoryBuffer.resize_no_construct(static_cast::size_type>(static_cast::size_type>(length) + 1)); @@ -259,12 +254,12 @@ namespace AZ return ReadJsonString(AZStd::string_view{memoryBuffer.data(), memoryBuffer.size()}); } - AZ::Outcome ReadJsonFile(AZStd::string_view filePath) + AZ::Outcome ReadJsonFile(AZStd::string_view filePath, size_t maxFileSize) { // Read into memory first and then parse the json, rather than passing a file stream to rapidjson. // This should avoid creating a large number of micro-reads from the file. - auto readResult = AZ::Utils::ReadFile(filePath); + auto readResult = AZ::Utils::ReadFile(filePath, maxFileSize); if(!readResult.IsSuccess()) { return AZ::Failure(readResult.GetError()); diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonUtils.h b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonUtils.h index c7777feec0..c81497aa95 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonUtils.h +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonUtils.h @@ -71,7 +71,8 @@ namespace AZ AZ::Outcome ReadJsonString(AZStd::string_view jsonText); //! Parse a json file. Returns a failure with error message if the content is not valid JSON. - AZ::Outcome ReadJsonFile(AZStd::string_view filePath); + AZ::Outcome ReadJsonFile( + AZStd::string_view filePath, size_t maxFileSize = AZStd::numeric_limits::max()); //! Parse a json stream. Returns a failure with error message if the content is not valid JSON. AZ::Outcome ReadJsonStream(IO::GenericStream& stream); diff --git a/Code/Framework/AzCore/AzCore/Utils/Utils.h b/Code/Framework/AzCore/AzCore/Utils/Utils.h index e72a9b3f9c..1d3cbd777b 100644 --- a/Code/Framework/AzCore/AzCore/Utils/Utils.h +++ b/Code/Framework/AzCore/AzCore/Utils/Utils.h @@ -22,10 +22,6 @@ namespace AZ { namespace Utils { - //! Protects from allocating too much memory. The choice of a 1MB threshold is arbitrary. - //! If you need to work with larger files, please use AZ::IO directly instead of these utility functions. - inline constexpr size_t DefaultMaxFileSize = 1024 * 1024; - //! Terminates the application without going through the shutdown procedure. //! This is used when due to abnormal circumstances the application can no //! longer continue. On most platforms and in most configurations this will @@ -115,6 +111,7 @@ namespace AZ //! Read a file into a string. Returns a failure with error message if the content could not be loaded or if //! the file size is larger than the max file size provided. template - AZ::Outcome ReadFile(AZStd::string_view filePath, size_t maxFileSize = DefaultMaxFileSize); + AZ::Outcome ReadFile( + AZStd::string_view filePath, size_t maxFileSize); } } diff --git a/Gems/Atom/Asset/Shader/Code/Source/Editor/AzslCompiler.cpp b/Gems/Atom/Asset/Shader/Code/Source/Editor/AzslCompiler.cpp index 154bfdd607..8f761b621e 100644 --- a/Gems/Atom/Asset/Shader/Code/Source/Editor/AzslCompiler.cpp +++ b/Gems/Atom/Asset/Shader/Code/Source/Editor/AzslCompiler.cpp @@ -1149,7 +1149,7 @@ namespace AZ return BuildResult::CompilationFailed; } - auto readJsonResult = JsonSerializationUtils::ReadJsonFile(outputFile); + auto readJsonResult = JsonSerializationUtils::ReadJsonFile(outputFile, AZ::RPI::JsonUtils::AtomMaxFileSize); if (readJsonResult.IsSuccess()) { @@ -1170,7 +1170,7 @@ namespace AZ AZStd::string outputFile = m_inputFilePath; AzFramework::StringFunc::Path::ReplaceExtension(outputFile, outputExtension); - auto readJsonResult = JsonSerializationUtils::ReadJsonFile(outputFile); + auto readJsonResult = JsonSerializationUtils::ReadJsonFile(outputFile, AZ::RPI::JsonUtils::AtomMaxFileSize); if (readJsonResult.IsSuccess()) { diff --git a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder.cpp b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder.cpp index 9ee4ff2161..6b673759cf 100644 --- a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder.cpp +++ b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderAssetBuilder.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -555,7 +556,7 @@ namespace AZ shaderAssetCreator.SetRenderStates(renderStates); } - Outcome hlslSourceCodeOutcome = Utils::ReadFile(hlslFullPath); + Outcome hlslSourceCodeOutcome = Utils::ReadFile(hlslFullPath, AZ::RPI::JsonUtils::AtomMaxFileSize); if (!hlslSourceCodeOutcome.IsSuccess()) { AZ_Error( diff --git a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderBuilderUtility.cpp b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderBuilderUtility.cpp index 88abf92e54..928655111a 100644 --- a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderBuilderUtility.cpp +++ b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderBuilderUtility.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -51,7 +52,7 @@ namespace AZ { RPI::ShaderSourceData shaderSourceData; - auto document = JsonSerializationUtils::ReadJsonFile(fullPathToJsonFile); + auto document = JsonSerializationUtils::ReadJsonFile(fullPathToJsonFile, AZ::RPI::JsonUtils::AtomMaxFileSize); if (!document.IsSuccess()) { @@ -127,7 +128,7 @@ namespace AZ AZStd::unordered_map> outcomes; for (int i : indicesOfInterest) { - outcomes[i] = JsonSerializationUtils::ReadJsonFile(pathOfJsonFiles[i]); + outcomes[i] = JsonSerializationUtils::ReadJsonFile(pathOfJsonFiles[i], AZ::RPI::JsonUtils::AtomMaxFileSize); if (!outcomes[i].IsSuccess()) { AZ_Error(builderName, false, "%s", outcomes[i].GetError().c_str()); @@ -622,7 +623,7 @@ namespace AZ StructData inputStruct; inputStruct.m_id = ""; - auto jsonOutcome = JsonSerializationUtils::ReadJsonFile(pathToIaJson); + auto jsonOutcome = JsonSerializationUtils::ReadJsonFile(pathToIaJson, AZ::RPI::JsonUtils::AtomMaxFileSize); if (!jsonOutcome.IsSuccess()) { AZ_Error(ShaderBuilderUtilityName, false, "%s", jsonOutcome.GetError().c_str()); @@ -715,7 +716,7 @@ namespace AZ StructData outputStruct; outputStruct.m_id = ""; - auto jsonOutcome = JsonSerializationUtils::ReadJsonFile(pathToOmJson); + auto jsonOutcome = JsonSerializationUtils::ReadJsonFile(pathToOmJson, AZ::RPI::JsonUtils::AtomMaxFileSize); if (!jsonOutcome.IsSuccess()) { AZ_Error(ShaderBuilderUtilityName, false, "%s", jsonOutcome.GetError().c_str()); diff --git a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderVariantAssetBuilder.cpp b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderVariantAssetBuilder.cpp index 12e5382b63..5a552a2702 100644 --- a/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderVariantAssetBuilder.cpp +++ b/Gems/Atom/Asset/Shader/Code/Source/Editor/ShaderVariantAssetBuilder.cpp @@ -478,7 +478,7 @@ namespace AZ RPI::Ptr shaderOptionGroupLayout = RPI::ShaderOptionGroupLayout::Create(); // The shader options define what options are available, what are the allowed values/range // for each option and what is its default value. - auto jsonOutcome = JsonSerializationUtils::ReadJsonFile(optionsGroupJsonPath); + auto jsonOutcome = JsonSerializationUtils::ReadJsonFile(optionsGroupJsonPath, AZ::RPI::JsonUtils::AtomMaxFileSize); if (!jsonOutcome.IsSuccess()) { AZ_Error(ShaderVariantAssetBuilderName, false, "%s", jsonOutcome.GetError().c_str()); @@ -509,7 +509,7 @@ namespace AZ } auto functionsJsonPath = functionsJsonPathOutcome.TakeValue(); - auto jsonOutcome = JsonSerializationUtils::ReadJsonFile(functionsJsonPath); + auto jsonOutcome = JsonSerializationUtils::ReadJsonFile(functionsJsonPath, AZ::RPI::JsonUtils::AtomMaxFileSize); if (!jsonOutcome.IsSuccess()) { AZ_Error(ShaderVariantAssetBuilderName, false, "%s", jsonOutcome.GetError().c_str()); @@ -541,7 +541,7 @@ namespace AZ } auto srgJsonPath = srgJsonPathOutcome.TakeValue(); - auto jsonOutcome = JsonSerializationUtils::ReadJsonFile(srgJsonPath); + auto jsonOutcome = JsonSerializationUtils::ReadJsonFile(srgJsonPath, AZ::RPI::JsonUtils::AtomMaxFileSize); if (!jsonOutcome.IsSuccess()) { AZ_Error(ShaderVariantAssetBuilderName, false, "%s", jsonOutcome.GetError().c_str()); @@ -598,7 +598,7 @@ namespace AZ } auto bindingsJsonPath = bindingsJsonPathOutcome.TakeValue(); - auto jsonOutcome = JsonSerializationUtils::ReadJsonFile(bindingsJsonPath); + auto jsonOutcome = JsonSerializationUtils::ReadJsonFile(bindingsJsonPath, AZ::RPI::JsonUtils::AtomMaxFileSize); if (!jsonOutcome.IsSuccess()) { AZ_Error(ShaderVariantAssetBuilderName, false, "%s", jsonOutcome.GetError().c_str()); @@ -630,7 +630,7 @@ namespace AZ } hlslSourcePath = hlslSourcePathOutcome.TakeValue(); - Outcome hlslSourceOutcome = Utils::ReadFile(hlslSourcePath); + Outcome hlslSourceOutcome = Utils::ReadFile(hlslSourcePath, AZ::RPI::JsonUtils::AtomMaxFileSize); if (!hlslSourceOutcome.IsSuccess()) { AZ_Error( diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Common/JsonUtils.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Common/JsonUtils.h index 1a62e1753c..913a9702b1 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Common/JsonUtils.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Edit/Common/JsonUtils.h @@ -20,6 +20,10 @@ namespace AZ { namespace JsonUtils { + //! Protects from allocating too much memory. The choice of a 1MB threshold is arbitrary. + //! If you need to work with larger files, please use AZ::IO directly instead of these utility functions. + inline constexpr size_t AtomMaxFileSize = 1024 * 1024; + // Declarations... //! Loads serialized object data from a json file at the specified path @@ -39,7 +43,7 @@ namespace AZ { objectData = ObjectType(); - auto loadOutcome = AZ::JsonSerializationUtils::ReadJsonFile(path); + auto loadOutcome = AZ::JsonSerializationUtils::ReadJsonFile(path, AtomMaxFileSize); if (!loadOutcome.IsSuccess()) { AZ_Error("AZ::RPI::JsonUtils", false, "%s", loadOutcome.GetError().c_str()); diff --git a/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp index 09f6610150..d2ccad1997 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Builders/Material/MaterialBuilder.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -151,7 +152,7 @@ namespace AZ AZStd::string fullSourcePath; AzFramework::StringFunc::Path::ConstructFull(request.m_watchFolder.data(), request.m_sourceFile.data(), fullSourcePath, true); - auto loadOutcome = JsonSerializationUtils::ReadJsonFile(fullSourcePath); + auto loadOutcome = JsonSerializationUtils::ReadJsonFile(fullSourcePath, AZ::RPI::JsonUtils::AtomMaxFileSize); if (!loadOutcome.IsSuccess()) { AZ_Error(MaterialBuilderName, false, "%s", loadOutcome.GetError().c_str()); @@ -298,7 +299,7 @@ namespace AZ AZStd::string fullSourcePath; AzFramework::StringFunc::Path::ConstructFull(request.m_watchFolder.data(), request.m_sourceFile.data(), fullSourcePath, true); - auto loadOutcome = JsonSerializationUtils::ReadJsonFile(fullSourcePath); + auto loadOutcome = JsonSerializationUtils::ReadJsonFile(fullSourcePath, AZ::RPI::JsonUtils::AtomMaxFileSize); if (!loadOutcome.IsSuccess()) { AZ_Error(MaterialBuilderName, false, "Failed to load material file: %s", loadOutcome.GetError().c_str()); diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp index 307d2bb80f..6f68115ccb 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialSourceDataSerializer.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -67,7 +68,7 @@ namespace AZ { AZStd::string materialTypePath = AssetUtils::ResolvePathReference(jsonFileLoadContext->GetFilePath(), materialSourceData->m_materialType); - auto materialTypeJson = JsonSerializationUtils::ReadJsonFile(materialTypePath); + auto materialTypeJson = JsonSerializationUtils::ReadJsonFile(materialTypePath, AZ::RPI::JsonUtils::AtomMaxFileSize); if (!materialTypeJson.IsSuccess()) { AZStd::string failureMessage; diff --git a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp index 0c2e9dca1f..80b292ea9b 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Edit/Material/MaterialUtils.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -64,7 +65,7 @@ namespace AZ AZ::Outcome loadOutcome; if (document == nullptr) { - loadOutcome = AZ::JsonSerializationUtils::ReadJsonFile(filePath); + loadOutcome = AZ::JsonSerializationUtils::ReadJsonFile(filePath, AZ::RPI::JsonUtils::AtomMaxFileSize); if (!loadOutcome.IsSuccess()) { AZ_Error("AZ::RPI::JsonUtils", false, "%s", loadOutcome.GetError().c_str()); diff --git a/Gems/AtomLyIntegration/AtomBridge/Code/Source/Editor/AssetCollectionAsyncLoaderTestComponent.cpp b/Gems/AtomLyIntegration/AtomBridge/Code/Source/Editor/AssetCollectionAsyncLoaderTestComponent.cpp index 2a72641050..ef902fec3e 100644 --- a/Gems/AtomLyIntegration/AtomBridge/Code/Source/Editor/AssetCollectionAsyncLoaderTestComponent.cpp +++ b/Gems/AtomLyIntegration/AtomBridge/Code/Source/Editor/AssetCollectionAsyncLoaderTestComponent.cpp @@ -16,6 +16,7 @@ #include // Included so we can deduce the asset type from asset paths. +#include #include #include #include @@ -114,7 +115,7 @@ namespace AZ { rapidjson::Document jsonDoc; - auto readJsonResult = JsonSerializationUtils::ReadJsonFile(pathToAssetListJson); + auto readJsonResult = JsonSerializationUtils::ReadJsonFile(pathToAssetListJson, AZ::RPI::JsonUtils::AtomMaxFileSize); if (!readJsonResult.IsSuccess()) { From 7bf58945134f83a1b8433f53b345d1d7634fe733 Mon Sep 17 00:00:00 2001 From: srikappa-amzn Date: Wed, 8 Sep 2021 14:29:08 -0700 Subject: [PATCH 2/3] Set max file size limit as default parameter for ReadFile Signed-off-by: srikappa-amzn --- Code/Framework/AzCore/AzCore/Serialization/Json/JsonUtils.h | 3 ++- Code/Framework/AzCore/AzCore/Utils/Utils.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonUtils.h b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonUtils.h index c81497aa95..c684d0397a 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/JsonUtils.h +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/JsonUtils.h @@ -70,7 +70,8 @@ namespace AZ //! Parse json text. Returns a failure with error message if the content is not valid JSON. AZ::Outcome ReadJsonString(AZStd::string_view jsonText); - //! Parse a json file. Returns a failure with error message if the content is not valid JSON. + //! Parse a json file. Returns a failure with error message if the content is not valid JSON or if + //! the file size is larger than the max file size provided. AZ::Outcome ReadJsonFile( AZStd::string_view filePath, size_t maxFileSize = AZStd::numeric_limits::max()); diff --git a/Code/Framework/AzCore/AzCore/Utils/Utils.h b/Code/Framework/AzCore/AzCore/Utils/Utils.h index 1d3cbd777b..d8d4290f7f 100644 --- a/Code/Framework/AzCore/AzCore/Utils/Utils.h +++ b/Code/Framework/AzCore/AzCore/Utils/Utils.h @@ -112,6 +112,6 @@ namespace AZ //! the file size is larger than the max file size provided. template AZ::Outcome ReadFile( - AZStd::string_view filePath, size_t maxFileSize); + AZStd::string_view filePath, size_t maxFileSize = AZStd::numeric_limits::max()); } } From a80314f84a7d6867ff5db187830c320ba0eda20d Mon Sep 17 00:00:00 2001 From: srikappa-amzn Date: Wed, 8 Sep 2021 23:25:24 -0700 Subject: [PATCH 3/3] Added a missing include file Signed-off-by: srikappa-amzn --- Gems/Atom/Asset/Shader/Code/Source/Editor/AzslCompiler.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Gems/Atom/Asset/Shader/Code/Source/Editor/AzslCompiler.cpp b/Gems/Atom/Asset/Shader/Code/Source/Editor/AzslCompiler.cpp index 8f761b621e..a82c731d7b 100644 --- a/Gems/Atom/Asset/Shader/Code/Source/Editor/AzslCompiler.cpp +++ b/Gems/Atom/Asset/Shader/Code/Source/Editor/AzslCompiler.cpp @@ -17,6 +17,7 @@ #include #include +#include #include #include