Copilot commented on code in PR #6811:
URL: https://github.com/apache/texera/pull/6811#discussion_r3634977204
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/file/FileScanUtils.scala:
##########
@@ -39,6 +39,33 @@ import scala.collection.mutable
import scala.jdk.CollectionConverters.IteratorHasAsScala
private[file] object FileScanUtils {
+
+ private val MAX_SAFE_SIZE = 100L * 1024L * 1024L // 100 MB
+
+ private def safeToByteArray(entry: InputStream, attributeType:
FileAttributeType): Array[Byte] = {
+ val out = new ByteArrayOutputStream()
+ val buffer = new Array[Byte](8192)
+ var bytesRead = entry.read(buffer)
+ var totalBytes = 0L
+
+ while (bytesRead != -1) {
+ totalBytes += bytesRead
+ if (totalBytes > MAX_SAFE_SIZE) {
+ val largeBinaryHint = attributeType match {
+ case FileAttributeType.BINARY => "Please use 'large binary'
attribute type instead."
+ case FileAttributeType.SINGLE_STRING =>
+ "Please split the file or use a chunked reading method."
+ case _ => "File is too large to fit in memory."
+ }
+ throw new RuntimeException(
+ s"File exceeds maximum safe memory size of 100MB for
'${attributeType.getName}' type. $largeBinaryHint"
+ )
+ }
Review Comment:
`MAX_SAFE_SIZE` is used as a byte count, but the name doesn’t indicate units
and the exception message hard-codes "100MB" separately. This can drift if the
limit is ever changed, and the constant is less self-explanatory than other
size limits in the codebase (which typically include a `_BYTES` suffix).
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/scan/file/FileScanUtils.scala:
##########
@@ -39,6 +39,33 @@ import scala.collection.mutable
import scala.jdk.CollectionConverters.IteratorHasAsScala
private[file] object FileScanUtils {
+
+ private val MAX_SAFE_SIZE = 100L * 1024L * 1024L // 100 MB
+
+ private def safeToByteArray(entry: InputStream, attributeType:
FileAttributeType): Array[Byte] = {
+ val out = new ByteArrayOutputStream()
+ val buffer = new Array[Byte](8192)
+ var bytesRead = entry.read(buffer)
+ var totalBytes = 0L
Review Comment:
The new 100MB guard in `safeToByteArray` changes behavior in a
safety-critical way (preventing OOM / GC thrash), but there’s no
regression/unit test covering the new failure mode. `FileScanUtilsSpec` already
exists; adding a test that creates a >100MB file/zip entry and asserts
`createTuplesFromFile` fails fast with the expected message would prevent
accidental regressions (e.g., limit removal or message drift).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
