The method createNewFile converts all the inputstream to memory. This…#980
Conversation
… could cause out of memory errors. Now the process use a buffer to avoid this. Issue: 205339
Cherry pick to beta success |
… could cause out of memory errors. Now the process use a buffer to avoid this. Issue: 205339
Cherry pick to beta success |
…rage. Previously, if the file extension was .tmp, it was always set to image/jpeg, which caused issues—such as when uploading a PDF: upon downloading it, the file couldn't be opened properly.
Cherry pick to beta failed, 1 conflicted file in commit e131aea
|
ladrians
left a comment
There was a problem hiding this comment.
this time I checked the usage of detectedContentType for the assignment of "image/jpeg", looks good
Manual cherry pick to beta success |
Cherry pick to beta failed, 2 conflicted files in commit e131aea
|
There was a problem hiding this comment.
Pull Request Overview
This PR addresses memory issues in file upload methods by replacing in-memory byte array operations with streaming operations to prevent out-of-memory errors when handling large files. The change introduces a new helper utility to process InputStreams using temporary files and buffers.
- Replaces in-memory byte array conversion with buffered streaming for all cloud storage providers
- Introduces a new helper class to handle InputStream processing with content type detection
- Removes unused commons-io dependencies from modules that no longer need them
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| gxcloudstorage-common/src/main/java/com/genexus/db/driver/ExternalProviderHelper.java | Adds new helper method for streaming InputStreams with content type detection |
| gxcloudstorage-ibmcos/src/main/java/com/genexus/db/driver/ExternalProviderIBM.java | Replaces byte array conversion with streaming upload |
| gxcloudstorage-googlecloudstorage/src/main/java/com/genexus/db/driver/ExternalProviderGoogle.java | Updates upload method to use streaming approach |
| gxcloudstorage-azureblob/src/main/java/com/genexus/db/driver/ExternalProviderAzureStorage.java | Improves buffering and uses helper for content type detection |
| gxcloudstorage-awss3-v2/src/main/java/com/genexus/db/driver/ExternalProviderS3V2.java | Replaces ByteBuffer approach with streaming |
| gxcloudstorage-awss3-v1/src/main/java/com/genexus/db/driver/ExternalProviderS3V1.java | Updates to use streaming instead of byte arrays |
| common/src/main/java/com/genexus/util/GXFileInfo.java | Adds streaming method to avoid loading entire file into memory |
| Multiple pom.xml files | Updates dependencies and removes unused commons-io references |
| } | ||
|
|
||
| public static InputStreamWithLength getInputStreamContentLength(InputStream input) throws IOException { | ||
| File tempFile = File.createTempFile("upload-", ".tmp"); |
There was a problem hiding this comment.
Creating temporary files without specifying a secure directory could create files in a world-readable location. Consider using a secure temporary directory or setting appropriate file permissions.
| File tempFile = File.createTempFile("upload-", ".tmp"); | |
| File tempFile = File.createTempFile("upload-", ".tmp"); | |
| tempFile.setReadable(false, false); // Disable read for all users | |
| tempFile.setWritable(false, false); // Disable write for all users | |
| tempFile.setExecutable(false, false); // Disable execute for all users | |
| tempFile.setReadable(true, true); // Enable read for owner only | |
| tempFile.setWritable(true, true); // Enable write for owner only |
| return value; | ||
| } | ||
|
|
||
| public static InputStreamWithLength getInputStreamContentLength(InputStream input) throws IOException { |
There was a problem hiding this comment.
If an exception occurs after creating the temporary file but before returning the InputStreamWithLength, the temporary file will not be cleaned up, causing a resource leak. Consider adding a try-catch block to clean up the temp file on error.
| byte[] targetArray = IOUtils.toByteArray(input); | ||
| storageClient.create(blobInfo, targetArray); | ||
|
|
||
| storageClient.createFrom(blobInfo, streamInfo.tempFile.toPath()); |
There was a problem hiding this comment.
There's no null check for streamInfo.tempFile before calling toPath(). If the helper method fails to create a temp file, this could result in a NullPointerException.
Manual cherry pick to beta success |
Cherry pick to beta failed, 2 conflicted files in commit e131aea
|
Manual cherry pick to beta success |
Cherry pick to beta failed, 4 conflicted files in commit e131aea
|
#980) * The method createNewFile converts all the inputstream to memory. This could cause out of memory errors. Now the process use a buffer to avoid this. Issue: 205339 * The method createNewFile converts all the inputstream to memory. This could cause out of memory errors. Now the process use a buffer to avoid this. Issue: 205339 * Tries to determine the content-type of files uploaded to external storage. Previously, if the file extension was .tmp, it was always set to image/jpeg, which caused issues—such as when uploading a PDF: upon downloading it, the file couldn't be opened properly. * Update Tika version to be java 8 compliant. * Add log4j impl for slf4j to avoid "No SLF4J providers were found" in Tika logs * Implemented better way to delete temporary files (cherry picked from commit 1fa5ad3)
… could cause out of memory errors.
Now the process use a buffer to avoid this.
Issue: 205339