Bug ID: 5082756; State: 6-Fix Understood, bug; Priority: 4-Low http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5082756
While the DTDs for the different metdata formats usually specify boolean values as ( "TRUE" | "FALSE" ), the implementations tend to use "true" and "false" instead. There are two possible approaches to this problem: 1. Have the implementation match the specification, i.e. use upper case 2. Adjust the specification to match implementation, i.e. use lower case While the former has the benefit of only touching internal com.sun implementation classes, the latter has the long therm benefit that implementations can use the default String conversion methods from class Boolean without further case conversion. While I would have wished for the specification to use lower case in the first place, I would now stick with the way it is, and adjust the implementation to upper case. Both approaches can be implemented with different degrees of tolerance when accepting values: a) strict: only allow the values also allowed by the DTD b) two possibilities: allow both "true" and "TRUE" c) ignore case: also allow "tRuE" Here I am in favor of the middle way. This gives you backward compatibility, but won't allow additional values without reason. The benefit is that with this it is more likely that an application developed under OpenJDK will work under other JREs as well. The downside of course is that applications developed for a JRE that completely ignores case would fail on OpenJDK. As I expect OpenJDK to have a larger market share, I would think this less likely to get unnoticed, though. There is also a slight performance benefit from not allowing mixed case, but I guess thats negligible here. So corresponding to my personal preferences, the attached patch changes all attribute creations to uppercase, and allows for both cases in PNGMetadata, but no mixed forms. The GIFMetadata implementation which already uses equalsIgnoreCase and thus allows for mixed case I left untouched for the moment. For the sake of consistency, we might to change that to the more strict two cases instead. I haven't any test case ready yet. I'll write one when we are agreed on the intended behaviour. The attached patch is from my mercurial patch queue. Once you consider it ready for inclusion, I will commit it locally and export a mercurial patch instead. Greetings, Martin von Gagern
diff --git a/src/share/classes/com/sun/imageio/plugins/gif/GIFImageMetadata.java b/src/share/classes/com/sun/imageio/plugins/gif/GIFImageMetadata.java --- a/src/share/classes/com/sun/imageio/plugins/gif/GIFImageMetadata.java +++ b/src/share/classes/com/sun/imageio/plugins/gif/GIFImageMetadata.java @@ -153,7 +153,7 @@ node.setAttribute("imageWidth", Integer.toString(imageWidth)); node.setAttribute("imageHeight", Integer.toString(imageHeight)); node.setAttribute("interlaceFlag", - interlaceFlag ? "true" : "false"); + interlaceFlag ? "TRUE" : "FALSE"); root.appendChild(node); // Local color table @@ -185,9 +185,9 @@ node.setAttribute("disposalMethod", disposalMethodNames[disposalMethod]); node.setAttribute("userInputFlag", - userInputFlag ? "true" : "false"); + userInputFlag ? "TRUE" : "FALSE"); node.setAttribute("transparentColorFlag", - transparentColorFlag ? "true" : "false"); + transparentColorFlag ? "TRUE" : "FALSE"); node.setAttribute("delayTime", Integer.toString(delayTime)); node.setAttribute("transparentColorIndex", diff --git a/src/share/classes/com/sun/imageio/plugins/gif/GIFStreamMetadata.java b/src/share/classes/com/sun/imageio/plugins/gif/GIFStreamMetadata.java --- a/src/share/classes/com/sun/imageio/plugins/gif/GIFStreamMetadata.java +++ b/src/share/classes/com/sun/imageio/plugins/gif/GIFStreamMetadata.java @@ -202,7 +202,7 @@ compression_node.appendChild(node); node = new IIOMetadataNode("Lossless"); - node.setAttribute("value", "true"); + node.setAttribute("value", "TRUE"); compression_node.appendChild(node); // NumProgressiveScans not in stream diff --git a/src/share/classes/com/sun/imageio/plugins/jpeg/JPEGMetadata.java b/src/share/classes/com/sun/imageio/plugins/jpeg/JPEGMetadata.java --- a/src/share/classes/com/sun/imageio/plugins/jpeg/JPEGMetadata.java +++ b/src/share/classes/com/sun/imageio/plugins/jpeg/JPEGMetadata.java @@ -955,7 +955,7 @@ // Lossless - false IIOMetadataNode lossless = new IIOMetadataNode("Lossless"); - lossless.setAttribute("value", "false"); + lossless.setAttribute("value", "FALSE"); compression.appendChild(lossless); // NumProgressiveScans - count sos segments diff --git a/src/share/classes/com/sun/imageio/plugins/png/PNGMetadata.java b/src/share/classes/com/sun/imageio/plugins/png/PNGMetadata.java --- a/src/share/classes/com/sun/imageio/plugins/png/PNGMetadata.java +++ b/src/share/classes/com/sun/imageio/plugins/png/PNGMetadata.java @@ -827,7 +827,7 @@ } node = new IIOMetadataNode("BlackIsZero"); - node.setAttribute("value", "true"); + node.setAttribute("value", "TRUE"); chroma_node.appendChild(node); if (PLTE_present) { @@ -889,7 +889,7 @@ compression_node.appendChild(node); node = new IIOMetadataNode("Lossless"); - node.setAttribute("value", "true"); + node.setAttribute("value", "TRUE"); compression_node.appendChild(node); node = new IIOMetadataNode("NumProgressiveScans"); @@ -1157,12 +1157,13 @@ } } String value = attr.getNodeValue(); - if (value.equals("true")) { + // Allow lower case booleans for backward compatibility + if (value.equals("TRUE") || value.equals("true")) { return true; - } else if (value.equals("false")) { + } else if (value.equals("FALSE") || value.equals("false")) { return false; } else { - fatal(node, "Attribute " + name + " must be 'true' or 'false'!"); + fatal(node, "Attribute " + name + " must be 'TRUE' or 'FALSE'!"); return false; } }