[
https://issues.apache.org/jira/browse/TIKA-5057?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18142036#comment-18142036
]
ASF GitHub Bot commented on TIKA-5057:
--------------------------------------
Copilot commented on code in PR #3564:
URL: https://github.com/apache/tika/pull/3564#discussion_r4525711673
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/onenote/OneNoteDocument.java:
##########
@@ -50,6 +59,26 @@ void setAssocGuidToRef(ExtendedGUID guid, FileChunkReference
ref) {
guidToRef.put(guid, ref);
}
+ void putObject(ExtendedGUID oid, FileNodePtr ptr) {
+ guidToObject.put(oid, ptr);
+ revisionObjects.computeIfAbsent(currentRevision, k -> new
HashMap<>()).put(oid, ptr);
+ }
+
+ /** The object as revision {@code rid} has it: declared there or in a
dependency. */
+ FileNodePtr objectIn(ExtendedGUID rid, ExtendedGUID oid) {
+ ExtendedGUID r = rid;
+ for (int i = 0; r != null && i <= revisionMap.size(); i++) {
+ Map<ExtendedGUID, FileNodePtr> objects = revisionObjects.get(r);
+ FileNodePtr ptr = objects == null ? null : objects.get(oid);
Review Comment:
Each object lookup restarts the dependency walk, so resolving M unchanged
objects declared near the base of an R-revision chain takes O(M × R) map
lookups. Extraction can become quadratic as both content and revision history
grow, and the file-node visit budget does not count these scans. Build a
cycle-safe effective-object index for the selected revision, or cache inherited
resolutions along the dependency chain.
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/onenote/OneNoteParserConfig.java:
##########
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tika.parser.microsoft.onenote;
+
+import java.io.Serializable;
+
+/**
+ * Configuration for {@link OneNoteParser}, set in the parser's JSON config or
per parse in the
+ * {@link org.apache.tika.parser.ParseContext}.
+ */
+public class OneNoteParserConfig implements Serializable {
+
+ private static final long serialVersionUID = 1L;
+
+ private boolean includePageHistory = false;
+ private boolean includeDeletedContent = false;
+
+ /**
+ * Whether text includes every earlier revision and saved page version,
not only each page's
+ * current one.
+ */
+ public boolean isIncludePageHistory() {
+ return includePageHistory;
+ }
+
+ public void setIncludePageHistory(boolean includePageHistory) {
+ this.includePageHistory = includePageHistory;
Review Comment:
With `includePageHistory=true`, `OneNoteParser` enables
`crawlAllFileNodesFromRoot` but leaves `onlyLatestRevision=true`, so the
classic walker's history filter still suppresses marked saved-page versions.
Set `onlyLatestRevision` to `!config.isIncludePageHistory()` when constructing
the walker options, and add a regression case for text beneath
`VersionHistoryGraphSpaceContextNodes` or `VERSION_HISTORY_CONTENT`.
> General improvements to onenote parser
> --------------------------------------
>
> Key: TIKA-5057
> URL: https://issues.apache.org/jira/browse/TIKA-5057
> Project: Tika
> Issue Type: Task
> Reporter: Tim Allison
> Priority: Trivial
>
> When I have extra tokens, I'll have an agent focus on a parser at a time for
> robustness and simplicity. Let's start with onenote. CC [~henrylindemanglean]
> :robot: swarm recommends:
> {noformat}
> Native .one path (the common format, unrelated to TIKA-5055):
> 1. Exponential parse on shared child lists (OneNotePtr:714). A file-node
> list referenced twice is parsed twice. Only the current path is checked for
> cycles, so a crafted file of a few KB blows up as 2^depth. Three reviewers
> found this independently.
> 2. Exponential tree walk (OneNoteTreeWalker:648). The same shape: shared
> objects are walked once per reference path. Each visit also builds and keeps
> a HashMap of decoded properties, which OneNoteParser:144 throws away.
> 3. Wrong references, lost text (OneNotePtr:1185). The OID/OSID/ContextID
> stream position is copied to a local variable and never written back. Every
> reference property starts again at stream[0], so later references are never
> walked. It also removes the
> cap on reference counts.
> 4. Duplicate text in real files (OneNoteTreeWalker:569-592). Reproduced:
> testOneNote2.one produces 176 paragraphs, 105 of them "Untitled picture.png".
> Image filenames and TextExtendedAscii text are never deduplicated and ignore
> the revision filter.
> 5. Unchecked counts and budgets (OneNotePtr:838, :518, :809):
> - ObjectInfoDependencyOverrideData counts are unbounded.
> - The global-ID cap applies per node, with no total.
> - A negative FileDataStoreObject length gets through.
> 6. Smaller issues:
> - Embedded file data is read in full before shouldParseEmbedded is asked
> (OneNoteTreeWalker:435).
> - The UTF-16 NUL terminator stays in author names and image filenames.
> Legacy string dump, used whenever a parse falls back
> (OneNoteLegacyDumpStrings:95):
> - Never resets its buffer at 1 MB boundaries. A long run is emitted again
> at every boundary, so work grows quadratically and normal files over 1 MB get
> duplicated text.
> - Small bugs: it drops the last byte of each buffer,
> Character.isWhitespace(i) tests the loop index instead of the character, and
> split(" ") allocates one String per word.
> Fallback after a partial walk (OneNoteParser:172, :197): on both paths, the
> full legacy dump is added after whatever the failed walk already wrote, so
> content appears twice.
> FSSHTTPB:
> - GUIDs are MD5 hashes, not decoded (BitReader:140, AdapterHelper:37). They
> come from UUID.nameUUIDFromBytes. As a result:
> - Encrypted sections are never detected; the check at
> MSOneStoreParser:328 can't match.
> - Embedded relationship IDs contain hashed GUIDs.
> - Every cell mapping copies its whole revision chain
> (MSOneStoreParser:362), so the cost grows with mappings × objects.
> - Slow spots:
> - removeSupersededObjects is O(n²) (:393).
> - resourceInfoFromReferencingObject scans every object for each unnamed
> blob (MSOneStorePackage:1140).
> - hasUsablePicture copies a whole blob just to check it's non-empty
> (:1118).
> - Small port bugs:
> - cProperties is read as signed, so values ≥ 0x8000 abort the parse.
> - An undefined property type throws an NPE.
> - StorageManifestSchemaGUID reads from offset 0 instead of the current
> position.
> - BitConverter's bounds check ignores the index.
> Dead code:
> - The serialize/write path: 65 files, about 970 lines.
> - The FSSHTTPD chunking/build side.
> - getContent() and its builders.
> - Unreferenced classes: Error, PropertyIDType, RequestTypes, UMath.
> - Debug print() methods and option branches that are never set.
> {noformat}
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
