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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
