[OpenJDK 2D-Dev] 5106550: PNG writer merge standard metadata fails for TextEntry sans #IMPLIED attributes

[Andrew Brygin]

Hello Martin,

  As you probably saw your fix for 5106550 was just  pushed into 2d 
workspace
  (http://hg.openjdk.java.net/jdk7/2d/jdk/rev/9d14b0582e1a)

  There is short status for other two your suggested fixes:
   - 5082756: finished engineering review, waiting for CCC decision.
   - 6541476: as we had commited fix for this bug id I'd submitted new 
bug report (6782079)
        for this problem.  Fix is being reviewed now.

  Both these suggested fixes were tweaked during engineering review.
  I'll send you updated webrevs separately as they seems to be a bit too
  large to send everyone on this list : ) (about 1Mb).

Thanks,
Andrew
 
Martin von Gagern wrote:
> Bug ID: 5106550; State: 3-Accepted, bug; Priority: 4-Low
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5106550
>
> The bug lies in the fact that for TextEntry nodes in standard metadata
> format, PNGMetadata requires additional attributes besides the ones
> marked #REQUIRED in the DTD. Namely missing encoding, language or
> compression will result in an IIOInvalidTreeException:
>   
>> Exception in thread "main" javax.imageio.metadata.IIOInvalidTreeException: Required attribute encoding not present!
>> 	at com.sun.imageio.plugins.png.PNGMetadata.fatal(PNGMetadata.java:1085)
>> 	at com.sun.imageio.plugins.png.PNGMetadata.getAttribute(PNGMetadata.java:1208)
>> 	at com.sun.imageio.plugins.png.PNGMetadata.getAttribute(PNGMetadata.java:1217)
>> 	at com.sun.imageio.plugins.png.PNGMetadata.mergeStandardTree(PNGMetadata.java:1921)
>> 	at com.sun.imageio.plugins.png.PNGMetadata.mergeTree(PNGMetadata.java:1232)
>> 	at MergeStdCommentTest.main(MergeStdCommentTest.java:37)
>>     
>
> The patch fixes this by
> 1. not reading the "encoding" attribute at all,
>    as it isn't even used in the following code
> 2. having the "language" attribute default to "",
>    as http://www.w3.org/TR/PNG/#11iTXt states that
>    "if the language tag is empty, the language is unspecified"
> 3. having the "compression" attribute default to "none",
>    as this is the default given in the DTD
> 4. ignore any node with missing or empty "keyword",
>    as the PNG standard requires a keyword of length at least 1
>    according to http://www.w3.org/TR/PNG/#11tEXt
>
> I changed invocations from the previously used
>     private String getAttribute(Node node, String name)
> which implied a required argument to the more flexible
>     private String getStringAttribute(Node node,
>                                       String name,
>                                       String defaultValue,
>                                       boolean required)
>
> Open question:
> Do you agree in dropping nodes with missing keywords?
> This follows the concept of not merging parts of the standard metadata
> model which have no counterpart in a specific file format, but might
> still lead to unexpected behaviour.
>
> I previously had this fix submitted to jdk7-dev, and mentioned in a
> posting "Bug fixes for com.sun.imageio.plugins.png.PNGMetadata" here.
> http://mail.openjdk.java.net/pipermail/jdk7-dev/2008-October/000272.html
> http://mail.openjdk.java.net/pipermail/2d-dev/2008-November/000540.html
>
> In the meantime, I have had a look at the mq extension of mercurial.
> Thus the attached patch is now a single patch straight out of my patch
> queue. I submit it here, waiting for comments, discussion, sponsorship.
> Once you consider it ready for inclusion, I can commit it localy and
> post a patch exported by hg.
>
> Greetings,
>  Martin von Gagern
>