-
Notifications
You must be signed in to change notification settings - Fork 18
The method createNewFile converts all the inputstream to memory. This… #980
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
4c656db
860e6bf
fea969c
e131aea
7d9ba6e
4b034f7
c61bbd9
bc33a6f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3,6 +3,8 @@ | |||||||||||||||
import com.genexus.util.Encryption; | ||||||||||||||||
import com.genexus.util.GXService; | ||||||||||||||||
|
||||||||||||||||
import java.io.*; | ||||||||||||||||
|
||||||||||||||||
public class ExternalProviderHelper { | ||||||||||||||||
|
||||||||||||||||
public static String getServicePropertyValue(GXService s, String propName, boolean isSecure){ | ||||||||||||||||
|
@@ -23,4 +25,30 @@ public static String getEnvironmentVariable(String name, boolean required) throw | |||||||||||||||
} | ||||||||||||||||
return value; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
public static InputStreamWithLength getInputStreamContentLength(InputStream input) throws IOException { | ||||||||||||||||
File tempFile = File.createTempFile("upload-", ".tmp"); | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||||
try (OutputStream out = new FileOutputStream(tempFile)) { | ||||||||||||||||
byte[] buffer = new byte[8192]; | ||||||||||||||||
int bytesRead; | ||||||||||||||||
while ((bytesRead = input.read(buffer)) != -1) { | ||||||||||||||||
out.write(buffer, 0, bytesRead); | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
long size = tempFile.length(); | ||||||||||||||||
InputStream newInput = new FileInputStream(tempFile); | ||||||||||||||||
return new InputStreamWithLength(newInput, size, tempFile); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
public static class InputStreamWithLength { | ||||||||||||||||
iroqueta marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||
public final InputStream inputStream; | ||||||||||||||||
public final long contentLength; | ||||||||||||||||
public final File tempFile; // nullable | ||||||||||||||||
|
||||||||||||||||
public InputStreamWithLength(InputStream inputStream, long contentLength, File tempFile) { | ||||||||||||||||
this.inputStream = inputStream; | ||||||||||||||||
this.contentLength = contentLength; | ||||||||||||||||
this.tempFile = tempFile; | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,6 @@ | |
import com.google.api.services.storage.model.StorageObject; | ||
import com.google.auth.oauth2.ServiceAccountCredentials; | ||
import com.google.cloud.storage.*; | ||
import org.apache.commons.io.IOUtils; | ||
import org.apache.logging.log4j.LogManager; | ||
import org.apache.logging.log4j.Logger; | ||
|
||
|
@@ -170,16 +169,28 @@ private void setBlobAcl(BlobId blobId, ResourceAccessControlList acl) { | |
} | ||
|
||
public String upload(String externalFileName, InputStream input, ResourceAccessControlList acl) { | ||
ExternalProviderHelper.InputStreamWithLength streamInfo = null; | ||
try { | ||
streamInfo = ExternalProviderHelper.getInputStreamContentLength(input); | ||
|
||
BlobId blobId = BlobId.of(bucket, externalFileName); | ||
BlobInfo blobInfo = BlobInfo.newBuilder(blobId).build(); | ||
byte[] targetArray = IOUtils.toByteArray(input); | ||
storageClient.create(blobInfo, targetArray); | ||
|
||
storageClient.createFrom(blobInfo, streamInfo.tempFile.toPath()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||
|
||
setBlobAcl(blobId, acl); | ||
return getResourceUrl(blobInfo, acl); | ||
} catch (IOException ex) { | ||
handleIOException(ex); | ||
return ""; | ||
} finally { | ||
if (streamInfo != null && streamInfo.tempFile != null && streamInfo.tempFile.exists()) { | ||
try { | ||
streamInfo.tempFile.delete(); | ||
} catch (Exception e) { | ||
logger.warn("Could not delete temporary file: " + streamInfo.tempFile.getAbsolutePath(), e); | ||
} | ||
} | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Copilot uses AI. Check for mistakes.