You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tika.apache.org by "henry-lindeman-glean (via GitHub)" <gi...@apache.org> on 2026/09/30 18:57:36 UTC

[PR] [TIKA-4944] Extract GUIDs from onenote files [tika]

henry-lindeman-glean opened a new pull request, #3278:
URL: https://github.com/apache/tika/pull/3278

   
   
   <!--
     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.
   -->
   
   Thanks for your contribution to [Apache Tika](https://tika.apache.org/)! Your help is appreciated!
   
   Before opening the pull request, please verify that
   * there is an open issue on the [Tika issue tracker](https://issues.apache.org/jira/projects/TIKA) which describes the problem or the improvement. We cannot accept pull requests without an issue because the change wouldn't be listed in the release notes.
   * the issue ID (`TIKA-XXXX`)
     - is referenced in the title of the pull request
     - and placed in front of your commit messages surrounded by square brackets (`[TIKA-XXXX] Issue or pull request title`)
   * commits are squashed into a single one (or few commits for larger changes)
   * Tika builds and unit tests pass with `./mvnw clean install` (`clean test` alone cannot resolve the pipes plugin zips)
   * if you used a generative AI tool: follow the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) (`Generated-by: <tool>` in the commit message), and consider running the pre-flight in `.skills/devs/pr-review/SKILL.md` — fix what it finds; don't paste its report here
   * there should be no conflicts when merging the pull request branch into the *recent* `main` branch. If there are conflicts, please try to rebase the pull request branch on top of a freshly pulled `main` branch
   * if you add new module that downstream users will depend upon add it to relevant group in `tika-bom/pom.xml`.
   
   We will be able to faster integrate your pull request if these conditions are met. If you have any questions how to fix your problem or about using Tika in general, please sign up for the [Tika mailing list](http://tika.apache.org/mail-lists.html). Thanks!
   
   ```
   ❯ java -jar tika-app/target/tika-app-4.1.1-SNAPSHOT.jar -x ~/glean/tika/Downloadme/parsing-test-2/Section-1.one
   INFO  [main] 10:05:34,215 org.apache.tika.cli.TikaCLI As a convenience, TikaCLI has turned on several non-default features
   as specified in tika-app/src/main/resources/tika-config-default-single-file.json.
   See: TIKA-2374, TIKA-4017, TIKA-4354 and TIKA-4472).
   This is not the default behavior in Tika generally or in tika-server.
   INFO  [main] 10:05:34,290 org.apache.tika.config.loader.TikaLoader temporary files go to java.io.tmpdir=/var/folders/r3/0ktdq26562d324x6wxwj10y40000gn/T
   INFO  [main] 10:05:36,375 org.apache.tika.config.loader.ParserLoader text recognizers: none found among the loaded parsers; images and rendered pages are not enriched
   <?xml version="1.0" encoding="UTF-8"?><html xmlns="http://www.w3.org/1999/xhtml">
   <head>
   <meta name="tk:content-type-magic-detected" content="application/onenote; format=one"/>
   <meta name="tk:resource-name" content="Section-1.one"/>
   <meta name="Content-Length" content="86561"/>
   <meta name="tk:parsed-by" content="org.apache.tika.parser.CompositeParser"/>
   <meta name="tk:parsed-by" content="org.apache.tika.parser.DefaultParser"/>
   <meta name="tk:parsed-by" content="org.apache.tika.parser.microsoft.onenote.OneNoteParser"/>
   <meta name="Content-Type" content="application/onenote; format=one"/>
   <title/>
   </head>
   <body><div class="page" id="{0044A32F-B467-4A61-946C-765ADF524E8D}"><p>Page 1</p>
   <p>Wednesday, August 12, 2026</p>
   <p>10:24 AM</p>
   <p>Text</p>
   <p>More text</p>
   <p>Bold Text</p>
   <p>Italic Text</p>
   <p>Underlined test</p>
   <p>Red text</p>
   <p>Highlighted text</p>
   <p>ddd</p>
   </div>
   <div class="page" id="{01F78CA7-DD93-4208-861C-9034B28BA411}"><p>Page sdskgjhsdlf</p>
   <p>Wednesday, August 12, 2026</p>
   <p>10:26 AM</p>
   <p>Bullet 1</p>
   <p>Bullet 2</p>
   <p>Indented bullet</p>
   <p>Indented square</p>
   <p>Number 1</p>
   <p>Number 2</p>
   <p>Letter a</p>
   <p>Numeral I</p>
   <p>Outdented number</p>
   <p>More text</p>
   <p>Table cell 1a</p>
   <p>Table cell 4a</p>
   <p>Table cell 2b</p>
   <p>Table cell 3b</p>
   <p>Table cell 4b</p>
   <p>Comic sans</p>
   <p>Dd</p>
   <p>D</p>
   </div>
   </body></html>%                                             
   ```


-- 
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: dev-unsubscribe@tika.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TIKA-4944] Extract GUIDs from onenote files [tika]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3278:
URL: https://github.com/apache/tika/pull/3278#discussion_r4218119185


##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/onenote/OneNoteTreeWalker.java:
##########
@@ -615,6 +624,20 @@ private Map<String, Object> processPropertyValue(PropertyValue propertyValue,
                         // have the onlyLatestRevision = false
                         handleRichEditTextUnicode(content.size());
                     }
+                } else if (propertyValue.propertyId.propertyEnum ==
+                        OneNotePropertyEnum.NotebookManagementEntityGuid
+                        && content.size() == 16) {
+                    ByteBuffer guidBuffer = ByteBuffer.allocate(16);
+                    dif.read(guidBuffer);
+                    String guid = pooledGuid(GUID.fromMicrosoftBytes(guidBuffer.array()).toString());
+                    propMap.put("notebookManagementEntityGuid", guid);
+                    // Version-history copies of a page are object-space roots; only the
+                    // current page's metadata is referenced via MetaDataObjectsAboveGraphSpace.
+                    if (objectType != OneNoteJcid.PAGE_METADATA || (parentPropertyId != null
+                            && parentPropertyId.propertyEnum ==
+                            OneNotePropertyEnum.MetaDataObjectsAboveGraphSpace)) {
+                        addClassicEntityGuid(objectType, guid);
+                    }

Review Comment:
   `dif.read(guidBuffer)` is not guaranteed to fill the buffer in a single call; if it returns fewer than 16 bytes, the remaining bytes stay `0` and you’ll silently compute the wrong GUID. Please read in a loop until `guidBuffer` has no remaining bytes (or fail if EOF) before decoding; this also avoids potential `NullPointerException` risk if decode ever returns `null`.



##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/onenote/fsshttpb/MSOneStorePackage.java:
##########
@@ -235,45 +244,68 @@ public void walkTree(OneNoteTreeWalkerOptions options, Metadata metadata,
             metadata.add(TikaCoreProperties.TIKA_META_EXCEPTION_WARNING, warning);
         }
         parseWarnings.clear();
-        if (!cells.isEmpty()) {
-            // Walk each page cell (object space) as a tree, starting from the root objects of
-            // its current revision and following the object references in property order. This
-            // emits the text in document order. The pages are walked in the order in which the
-            // section object space references them; cells that hold older versions of a page
-            // (the same object space in a different revision context) are skipped.
-            List<RevisionStoreCell> pageCells = new ArrayList<>();
-            List<RevisionStoreCell> otherCells = new ArrayList<>();
-            splitCells(pageCells, otherCells);
-            for (RevisionStoreCell cell : pageCells) {
-                xhtml.startElement("div", "class", "page");
-                try {
-                    walkCell(cell, options, metadata, xhtml);
-                } finally {
-                    xhtml.endElement("div");
+        try {
+            collectSectionFileIdentityGuids();
+            if (!cells.isEmpty()) {
+                // Walk each page cell (object space) as a tree, starting from the root objects of
+                // its current revision and following the object references in property order. This
+                // emits the text in document order. The pages are walked in the order in which the
+                // section object space references them; cells that hold older versions of a page
+                // (the same object space in a different revision context) are skipped.
+                List<RevisionStoreCell> pageCells = new ArrayList<>();
+                List<RevisionStoreCell> otherCells = new ArrayList<>();
+                splitCells(pageCells, otherCells);
+                for (RevisionStoreCell cell : pageCells) {
+                    emitPage(cell, options, metadata, xhtml);
+                }
+                for (RevisionStoreCell cell : otherCells) {
+                    walkCell(cell, indexObjectsById(cell.objectGroups), options, metadata, xhtml);
+                }
+            } else {
+                // no cell information available - walk the object groups in revision order
+                Map<ExGuid, RevisionStoreObject> objectsById = indexObjectsById(OtherFileNodeList);
+                Set<ExGuid> visited = new HashSet<>();
+                for (RevisionStoreObjectGroup objectGroup : OtherFileNodeList) {
+                    for (RevisionStoreObject object : objectGroup.objects) {
+                        walkObject(object, objectsById, visited, AuthorRole.NONE, options, metadata,
+                                xhtml, 0);

Review Comment:
   This loop will throw `NullPointerException` if `OtherFileNodeList` contains a `null` group or a group with `objects == null` (both patterns are exercised elsewhere in tests as “malformed” cases). Add `null` guards for `objectGroup`, `objectGroup.objects`, and (ideally) `object` before iterating/walking so damaged/fuzzed inputs can fall back safely instead of crashing.



##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/onenote/OneNotePtr.java:
##########
@@ -148,33 +148,13 @@ public OneNoteHeader deserializeHeader() throws IOException, TikaException {
     }
 
     private GUID deserializeGUID() throws IOException {
-        int[] guid = new int[16];
+        byte[] guidBytes = new byte[16];
         for (int i = 0; i < 16; ++i) {
-            guid[i] = dif.read();
+            guidBytes[i] = (byte) dif.read();
         }
-        int[] guid2 = new int[16];
-        // re-order [0,1,2,3] to little endian
-        guid2[0] = guid[3];
-        guid2[1] = guid[2];
-        guid2[2] = guid[1];
-        guid2[3] = guid[0];
-        // re-order [4,5,6,7] to little endian
-        guid2[4] = guid[5];
-        guid2[5] = guid[4];
-        guid2[6] = guid[7];
-        guid2[7] = guid[6];
-        // the rest is already in right order.
-        guid2[8] = guid[8];
-        guid2[9] = guid[9];
-        guid2[10] = guid[10];
-        guid2[11] = guid[11];
-        guid2[12] = guid[12];
-        guid2[13] = guid[13];
-        guid2[14] = guid[14];
-        guid2[15] = guid[15];
-
+        GUID guid = GUID.fromMicrosoftBytes(guidBytes);
         offset = dif.position();
-        return new GUID(guid2);
+        return guid;

Review Comment:
   `dif.read()` returns `-1` at EOF; casting that to `byte` silently turns it into `0xFF`, and `GUID.fromMicrosoftBytes` will then happily decode a “valid-looking” GUID instead of failing. Please detect EOF during the loop (e.g., read into an `int`, throw an `IOException`/`TikaException` if it’s `-1`) so truncated/corrupt headers don’t yield incorrect GUIDs.



-- 
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: dev-unsubscribe@tika.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TIKA-4944] Extract GUIDs from onenote files [tika]

Posted by "tballison (via GitHub)" <gi...@apache.org>.
tballison commented on PR #3278:
URL: https://github.com/apache/tika/pull/3278#issuecomment-6044666946

   Doh. Thank you.


-- 
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: dev-unsubscribe@tika.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TIKA-4944] Extract GUIDs from onenote files [tika]

Posted by "henry-lindeman-glean (via GitHub)" <gi...@apache.org>.
henry-lindeman-glean commented on PR #3278:
URL: https://github.com/apache/tika/pull/3278#issuecomment-6048651751

   those all look reasonable.
   
   ran more ai reviews on my own too. One thing it came up with I didn't take since the fix would be in tika-core and seems like it's known and the failure case is kinda pathological anyway:
   ```
   Quadratic publish at the cap (security + performance, independently): publish hands 
   100k values to Metadata.set(Property, String[]), whose limiter appends one-by-one 
   with growing-array copies — ~5·10⁹ reference copies cumulative (~20 GB churn) worst 
   case, executed inside finally where it can delay limit-exception propagation. The cap 
   bounds it but doesn't make it cheap. Decide: lower cap, batch differently, or accept 
   with a comment.
   ```


-- 
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: dev-unsubscribe@tika.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TIKA-4944] Extract GUIDs from onenote files [tika]

Posted by "tballison (via GitHub)" <gi...@apache.org>.
tballison commented on PR #3278:
URL: https://github.com/apache/tika/pull/3278#issuecomment-6058087640

   K, that's a known feature. We could cache the guids in an arraylist and then write String[] at the end of the parse?


-- 
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: dev-unsubscribe@tika.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TIKA-4944] Extract GUIDs from onenote files [tika]

Posted by "tballison (via GitHub)" <gi...@apache.org>.
tballison commented on PR #3278:
URL: https://github.com/apache/tika/pull/3278#issuecomment-6066377773

   Looks good. I'll merge after we get green. Thank you for iterating on this with us @henry-lindeman-glean !


-- 
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: dev-unsubscribe@tika.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TIKA-4944] Extract GUIDs from onenote files [tika]

Posted by "tballison (via GitHub)" <gi...@apache.org>.
tballison merged PR #3278:
URL: https://github.com/apache/tika/pull/3278


-- 
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: dev-unsubscribe@tika.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TIKA-4944] Extract GUIDs from onenote files [tika]

Posted by "henry-lindeman-glean (via GitHub)" <gi...@apache.org>.
henry-lindeman-glean commented on code in PR #3278:
URL: https://github.com/apache/tika/pull/3278#discussion_r4221618344


##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/onenote/OneNoteTreeWalker.java:
##########
@@ -615,6 +624,20 @@ private Map<String, Object> processPropertyValue(PropertyValue propertyValue,
                         // have the onlyLatestRevision = false
                         handleRichEditTextUnicode(content.size());
                     }
+                } else if (propertyValue.propertyId.propertyEnum ==
+                        OneNotePropertyEnum.NotebookManagementEntityGuid
+                        && content.size() == 16) {
+                    ByteBuffer guidBuffer = ByteBuffer.allocate(16);
+                    dif.read(guidBuffer);
+                    String guid = pooledGuid(GUID.fromMicrosoftBytes(guidBuffer.array()).toString());
+                    propMap.put("notebookManagementEntityGuid", guid);
+                    // Version-history copies of a page are object-space roots; only the
+                    // current page's metadata is referenced via MetaDataObjectsAboveGraphSpace.
+                    if (objectType != OneNoteJcid.PAGE_METADATA || (parentPropertyId != null
+                            && parentPropertyId.propertyEnum ==
+                            OneNotePropertyEnum.MetaDataObjectsAboveGraphSpace)) {
+                        addClassicEntityGuid(objectType, guid);
+                    }

Review Comment:
   Partial read in OneNoteTreeWalker (claimed High) — false positive.
   Copilot says dif.read(guidBuffer) isn't guaranteed to fill the 16-byte buffer and the NPE risk if decode returns null. But dif is a OneNoteDirectFileResource, whose read(ByteBuffer) loops until the buffer is drained and explicitly throws IOException("End of stream reached earlier than expected") on premature EOF. A silent short read is impossible. The NPE claim is also impossible: guidBuffer.array() is always exactly 16 bytes, and GUID.fromMicrosoftBytes only returns null for wrong-length input.



-- 
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: dev-unsubscribe@tika.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TIKA-4944] Extract GUIDs from onenote files [tika]

Posted by "henry-lindeman-glean (via GitHub)" <gi...@apache.org>.
henry-lindeman-glean commented on PR #3278:
URL: https://github.com/apache/tika/pull/3278#issuecomment-6044617424

   @tballison I think you meant to put [this comment](https://github.com/apache/tika/pull/3277#issuecomment-5963874799) here
   
   Anyway,
   1 - easy fix, is done
   2 - is asking me to test the link components against the test fixtures but I'm pretty sure I don't have access to the test fixtures in onedrive... it works with the files I tested with though.
   3 - the classic walker was picking up a version-history page. In test-tika-3970-dupetext.one, the extra page GUID {7B093AC2…} is only ever reached as the root of its own object space; current pages are always reached through MetaDataObjectsAboveGraphSpace. The classic walker now records page GUIDs only when they're reached through that property, so the file yields one page GUID. Turning the check off makes both new tests fail, as intended.
   4 - added
   
   sectionGuids is now sectionGuid
   


-- 
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: dev-unsubscribe@tika.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [TIKA-4944] Extract GUIDs from onenote files [tika]

Posted by "tballison (via GitHub)" <gi...@apache.org>.
tballison commented on PR #3278:
URL: https://github.com/apache/tika/pull/3278#issuecomment-6044776267

   2. y, i get it.
   
   Robot's latest. Please push back if the robot is just finding stuff because I asked it to. Some of these sound reasonable to me, but you know this area of the code better than I do. :rofl: 
   
   ```
    Remaining maintainer decision
     - onenote:entityGuids is a catch-all for any JCID the switch doesn't name. It is empty on every fixture, and a frozen key
       with no known producer is pure surface. I'd drop it, or at minimum give every new key a one-line javadoc naming the
       source object (0x30 page metadata, 0x0B page node, header guidFile vs FileIdentityGuid) so the semantics freeze with the
       name.
     - OneNoteJcid, OneNoteGuidCollector, and GUID.fromMicrosoftBytes are new public classes/methods. Public is the norm for
       parser internals in Tika, so fine unless you want them package-private with the fsshttpb subpackage moved.
   
     Edge case / usability (punt-able)
     - Classic files have no <div class="page"> at all, so page GUIDs can't be tied to page text there. A follow-on JIRA: emit
       a page div per jcidPageNode in the classic walker.
   
     Hygiene
     - findPageGuid(cell) duplicates the root-resolution logic of walkCell (unresolvedRoot/resolvedContentRoot). Share one
       "roots to walk" helper so they can't diverge.
     - testGuidsCollectedBeforeWalkFailurePublished relies on a NullPointerException from a null objectGroups. A null guard
       later would break the test for the wrong reason. Override indexObjectsById to throw instead.
     - MSOneStorePackageTest repeats the same 16-byte literal eight-plus times; one helper would cut the file substantially.
     - Bags are lexically sorted, so document order of pages is lost. The div ids recover it for FSSHTTPB only. Fine as is, but
       a deliberate choice.
   ```


-- 
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: dev-unsubscribe@tika.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org