Copilot commented on code in PR #3211:
URL: https://github.com/apache/tika/pull/3211#discussion_r4054525237
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pkg-module/src/main/java/org/apache/tika/parser/pkg/ZipParser.java:
##########
@@ -242,7 +243,7 @@ public void parse(TikaInputStream tis, ContentHandler
handler, Metadata metadata
parseWithZipFile(zipFile, tis, handler, metadata, context, config);
} else {
// Use streaming - enable rewind for DATA_DESCRIPTOR retry
- tis.enableRewind();
+ tis.enableRewind(context.get(CacheMemoryBudget.class));
Review Comment:
This budget argument is too late for the common Zip path:
`ZipFileHelper`/detection may already have called `getSeekableByteChannel()`,
which switches a stream-backed source into caching mode with a null budget.
`CachingSource.enableRewind` then returns without changing that cache, so large
ZIP inputs still bypass the shared `CacheMemoryBudget` despite this call (the
integrity-check call below has the same problem). The budget must be supplied
before the initial seekable-channel setup, or the cache must support adopting
it.
##########
tika-core/src/main/java/org/apache/tika/parser/inference/InferenceDispatcher.java:
##########
@@ -170,14 +170,15 @@ private void offer(InputKind kind, MediaType type,
Metadata target, Metadata par
if (!runs(binding, context) || !binding.accepts(kind, type,
target)) {
continue;
}
- // a media unit is the whole file; its maxBytes bounds one
segment, in the task
+ // a media unit is the whole file; maxBytes and maxChunks bound
its segments, in the task
if (kind != InputKind.MEDIA && binding.getMaxBytes() >= 0 && size
> binding.getMaxBytes()) {
state.dropped.merge(binding.getId() + " over maxBytes", 1,
Integer::sum);
continue;
}
List<InferenceUnit> units =
state.byBinding.computeIfAbsent(binding.getId(),
k -> new ArrayList<>());
- if (binding.getMaxChunks() >= 0 && units.size() >=
binding.getMaxChunks()) {
+ if (kind != InputKind.MEDIA && binding.getMaxChunks() >= 0
+ && units.size() >= binding.getMaxChunks()) {
Review Comment:
Moving both limits behind `kind != MEDIA` makes `maxChunks`/`maxBytes`
silently ineffective for any custom `InferenceTask` configured on a MEDIA
binding: `InferenceTask` is pluggable and its contract does not require
applying binding budgets, while only `EmbedTask` currently interprets media
segment caps. Such a task can receive an unbounded number of whole-file units
and grow `state.byBinding`; restrict MEDIA to tasks with an explicit budget
contract or enforce a separate dispatcher-level file/segment limit.
--
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]
