On Wed, 7 Aug 2024 22:40:29 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
> this PR has been out for too long, could you please merge the latest master
> branch in?
I did yesterday, or did something go wrong?
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/BinarySerializer.java
> line 111:
>
>> 109: }
>> 110:
>> 111: int nRelationships = is.readShort();
>
> same here: should we check for a positive value?
>
> as a general rule, we should be validating the input as it might come from
> untrusted sources, right? L79 and other places?
I considered doing more here, but as this is all just moved code, I'm hesitant
to change it as part of this PR. For example, if there is a faulty binary CSS
file which has a negative value for the short, then the original code will just
skip the loop. If I add a check, it will change the behavior.
> modules/javafx.graphics/src/main/java/com/sun/javafx/css/BinarySerializer.java
> line 126:
>
>> 124: else {
>> 125: assert false : "error deserializing CompoundSelector:
>> Combinator = " + ordinal;
>> 126: relationships.add(Combinator.DESCENDANT);
>
> throw an exception instead?
This is original code, just moved. Do you want me to change it still?
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1333#issuecomment-2274473668
PR Review Comment: https://git.openjdk.org/jfx/pull/1333#discussion_r1708049742
PR Review Comment: https://git.openjdk.org/jfx/pull/1333#discussion_r1708047126
