[OpenJDK 2D-Dev] 5082756: Image I/O plug-ins set metadata boolean attributes to "true" or "false"

[Andrew Brygin]

Hello Martin,

 fix looks fine for me in general, just one minor note:

 There is getBooleanAttribute() method in the GIFMetadata class, which
 probably should be modified as a part of this fix. This method  uses
 equalsIgnoreCase() to compare attribute values and refers this bug
 as a reason for this "too tolerant" approach. It also seems to be 
inconsistent
 with suggested implementation of similar method in the PNGMetadata class.

 Could you also provide a regression test for this change?

Thanks,
Andrew


Martin von Gagern wrote:
> 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
>