Copilot commented on code in PR #3184:
URL: https://github.com/apache/tika/pull/3184#discussion_r4029621402


##########
tika-core/src/main/java/org/apache/tika/metadata/Metadata.java:
##########
@@ -445,7 +446,18 @@ public void add(final String name, final String value) {
      * reserved key by name rather than by its {@link Property}.
      */
     public void addTrusted(final String name, final String value) {
-        writeLimiter.add(name, value, metadata);
+        writeLimiter.add(name, wellFormed(name, value), metadata);

Review Comment:
   This only normalizes values at the write call, but `Metadata.getValues` 
exposes the backing `String[]` (Metadata.java:405-415), so a caller can mutate 
a value after this check and leave `\uD800` in the object. `MetadataSerializer` 
and the plain `PipesWorker` metadata map then write that raw value and can 
still fail the JSON/Smile serialization; sanitize at the serialization 
boundaries or make the returned arrays defensive.



##########
tika-core/src/main/java/org/apache/tika/metadata/Metadata.java:
##########
@@ -445,7 +446,18 @@ public void add(final String name, final String value) {
      * reserved key by name rather than by its {@link Property}.
      */
     public void addTrusted(final String name, final String value) {
-        writeLimiter.add(name, value, metadata);
+        writeLimiter.add(name, wellFormed(name, value), metadata);
+    }
+
+    /**
+     * Every string value is stored well-formed: a lone surrogate becomes 
U+FFFD at the write,
+     * so no reader (a Smile encoder, for one) meets one. {@code tk:content} 
is exempt: it came
+     * through {@link org.apache.tika.sax.SafeContentHandler}, which already 
did this, and it
+     * is the one value large enough for the scan to cost anything.
+     */
+    private static String wellFormed(String name, String value) {
+        return TikaCoreProperties.TIKA_CONTENT.getName().equals(name) ? value
+                : StringUtils.wellFormed(value);

Review Comment:
   This normalizes only metadata values, not field names. 
`Metadata.add(KeyPrefix, ...)` accepts document-derived suffixes verbatim, so a 
suffix containing an unpaired surrogate still becomes a malformed name that 
`MetadataSerializer` writes directly and Jackson can reject. Normalize the name 
before passing it to the limiter as well (while keeping the `tk:content` 
exemption scoped to the value).



-- 
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]

Reply via email to