franslundberg / binson-java Goto Github PK

We now adopt a strict parsing approach. So, integer ranges should be checked to make sure they are indeed stored with as few bytes as bytes. As required by BINSON-SPEC-1 at binson.org.

The binson specification recommends that “An object should have less than
100 fields.” The java the implementation does not appear to implement this check,
enabling Denial of Service attacks.

Recommendations:

• The 40MB restriction appears pretty efficient to reduce vulnerability
to this type of attack. Keep this check!
• Implement a default maximum number of fields per level. For example,
abort with exception if more than 100 fields.

Use a limitation of total number of fields for the whole (top-level) Binson object including fields of children.
Remove existing "maxFieldCount".

Overall, I agree. The default behavior should not accept trailing data.

However, in general, more flexible parsing should be considered. For example starting at an offset in a byte array can be useful. Allowing trailing data or not. Also, maximum sizes. And more...

Parsing bytes with fields that are not in lexicographical order results in a successful parse.

The following test case fails. obj.toBytes() results in a correctly serialized Binson object, with the fields in lexicographical order. Throwing a suitable exception is probably a good idea since the bytes are not a valid Binson serialization and should therefore not be parsed as such.

@Test
public void testFromBytesWithUnorderedFields() {
    byte[] unordered = {0x40, 
                 0x14, 0x01, 0x42, 0x14, 0x01, 0x42,  // "B": "B"
                 0x14, 0x01, 0x41, 0x14, 0x01, 0x42,  // "A": "A"
                 0x41};
    
    Binson obj = Binson.fromBytes(unordered);
    
    Assert.assertArrayEquals(unordered, obj.toBytes());
}

And when parsing an object with duplicate fields the last duplicate field is the one present in the resulting Binson object.

The following test case fails. obj.toBytes() results in a correctly serialized Binson object, but with fields "missing" from the input. Throwing a suitable exception is probably a good idea since the bytes are not a valid Binson serialization and should therefore not be parsed as such.

@Test
public void testFromBytesWithDuplicateFields() {
    byte[] duplicates = {0x40, 
                   0x14, 0x01, 0x42, 0x14, 0x01, 0x42,  // "B": "B"
                   0x14, 0x01, 0x42, 0x14, 0x01, 0x42,  // "B": "A"
                   0x41};
    
    Binson obj = Binson.fromBytes(duplicates); // { "B": "A" }
    
    Assert.assertArrayEquals(duplicates, obj.toBytes());
}

See https://docs.oracle.com/javase/9/docs/api/java/lang/Double.html

according to http://binson.org/
true = %x44
false = %x45
somehow in this library , it is reversed.
public static final int FALSE = 0x44;
public static final int TRUE = 0x45;
Please check if the website is wrong or the code is wrong.

Agreed. The schema spec is insufficient.

Go through to improve code a little.

• Warnings on use of inefficient and since java-9 deprecated constructors Boolean(), Double(), replaced by Boolean.valueOf(), Double.valueOf().

• Warning: Non-serializable comparator.

• Warning: No default in a switch-case.

• Hex.toBytes() returns null on some input, Dodgy code recommends
  returning empty. The function is has three behaviors, returns null on
  some values, return byte[] on some values, throws exception on some
  values.

• ...all classes in this package are marked public instead of package
  internal. Class publication level does not match intended usage, i.e. the
  package class structuring of the binson-java API is not optimal.
  FL: hmm. Suggestion does not work. Classes in org.binson package must be able to access classes in org.binson.lowlevel. Limitation of Java. Not one that I like.

Will be fixed with the ParseParams approach of limiting the number of fields that can be parsed.

It seems like the keyset of Strings returned by keySet() does not return the field names in correct order. Or are there any other ways of looping through the fields?

This code reproduces this:

package org.binson;

public class Bug {

    public static void main(String[] args) {

        Binson b = new Binson()
                .put("c", "A")
                .put("i", 20)
                .put("o", "s")
                .put("z", new Binson().put("A", "B").put("ch", new byte[]{0x01,0x02}));
        System.out.println(b.toPrettyJson());


        for (String field : b.keySet()) {
            System.out.println("Field name: " + field);
        }

    }

}

The output from this is:

{
  "c": "A", 
  "i": 20, 
  "o": "s", 
  "z": {
    "A": "B", 
    "ch": "0x0102"
  }
}

Field name: c
Field name: i
Field name: z
Field name: o

I wrote a simple JUnit test case for this:

package org.binson;

import org.junit.Test;
import static org.junit.Assert.assertEquals;

public class KeySetTest {

    @Test
    public void keySetShouldBeOrdered() {
        Binson b = new Binson()
                .put("c", "A")
                .put("i", 20)
                .put("o", "s")
                .put("z", new Binson().put("A", "B").put("ch", new byte[]{0x01,0x02}));

        String fieldNames[] = new String[]{"c","i","o","z"};

        int i = 0;

        for (String field : b.keySet()) {
            assertEquals(field, fieldNames[i]);
            i++;
        }

    }
}

The reason i noticed this is that i have written a code generator that writes C code. I then use this to generate a lot of test cases. I assume this has to do with the HashMap that is used. I see in the JSON output that the set of string is converted to an array and then sorted:

String[] keys = obj.keySet().toArray(EMPTY_STRING_ARRAY);
Arrays.sort(keys, BinsonFieldNameComparator.INSTANCE);

When digging into this, i also see that the interface Output seems like a better approach than mine :)

Binson does not implement a map, but the specification foresees it
usefulness and provides a recommendation of how to implement it:
“It is recommended that a map (associate array) is stored as a single
array. The order of the array values should be: key of first key-value
pair, value of first key-value pair, key of second key-value pair, value of
second key-value pair and so on.”

A concern regarding this approach is that complexity externalized to the
application implementers instead of the core developers, possibly yielding
multiple and possibly incompatible implementations.

Seems like a badly formatted binary binson string:

String gris = "401405636861696e40140263684219e600792f20f6ce0b1a6714526484b4ea600d29bd25fc83b72945660cb37617300e3fd32e7523ee8000ba4007a1fd134fed69ee653f168f5bc5f39a88f7ca19cc51014014016118010f1402667218209d1e40e2f2d456f9ab21a7c84a231bc94f8790a322520f1a6061e6cc0aa9f7c91401704218210a357239b70ed94c611c4fafef19299d01c18cdabae1e24dbbdd56d6aef60c765d1802141514026c754314027463180c500110cc7c0170f37c0100001402746e140a44656d6f436c69656e741402746f18205529ce8ccf68c0b8ac19d437ab0f5b32723782608e93c6264f184ba152c2357b4143411404686f7374401403646f63421402656e14004314046e616d65421402656e140348656a4314037075621820357239b70ed94c611c4fafef19299d01c18cdabae1e24dbbdd56d6aef60c765d41140a6b6579486f6c6465727342401403646f63421402656e14004314046e616d65421402656e14044861776b43140370756218209d1e40e2f2d456f9ab21a7c84a231bc94f8790a322520f1a6061e6cc0aa9f7c941401403646f63421402656e14004314046e616d65421402656e140a44656d6f436c69656e7443140370756218205529ce8ccf68c0b8ac19d437ab0f5b32723782608e93c6264f184ba152c2357b41431404756e6974401403646f63421402656e14275065726d697373696f6e20746f206c6f636b20616e6420756e6c6f636b20746865206c6f636b2e43140668704e616d6514026c7514056c6576656c100214046e616d65421402656e140b6c6f636b2f756e6c6f636b43140170421802141514026c7543140474797065140268704141";

Is the sequence 4219e600 ok? bytesLen e600?

We should use:

binson-1.4-javadoc.jar

(instead of version at the end).
To be compatible with Maven/Nexus approach.

Recreate by creating subclass Sub of Binson. Add a Sub instance to a Binson object and output.

new Binson().put("sub", new Sub()).toBinsonString()

There is a problem in OutputWriter.writeValue(). Switch statement does not work.

Hex.toBytes() returns null on some input, Dodgy code recommends
returning empty. The function is has three behaviors, returns null on
some values, return byte[] on some values, throws exception on some
values.

Pls try next code:

byte[] b = new Binson()
                         .put("", new Binson()
                                 .put("", new Binson())
                                        .put("", new Binson()))
                  .toBytes();

System.out.println("len: " + b.length + ", " + hex(b));

Result:

len: 10, "40140040140040414141"

When looking on above hexdump, it's easy to see innermost put() was skipped.

...or something. A way to get the field names in proper order. To, for example, iterate the fields.

Be consistent with Binson.getXxx() method. Throw an exception when an expected field of the expected type does not exist.

Fixed already!

4.4 No Map / Associate array implementation

Example (untested pseudo-code) of an array-to-map implementation for
BinsonArray:
Map toMap(int maxFields) {
if (0 != list.size()%2) throw new BinsonException(…);
fields = list.size()/2;
if (fields > maxFields) throw new BinsonException(…);
map = …;
for (int field = 0; field < fields; field++) {
key = list.get(field2);
value = list.get(field2+1);
if (map.hasKey(key)) throw BinsonException(…);
map.put(key, value);
}
return map;
}

Recommendations:
• Implement reference code for converting from array to map and vice
versa.

See https://docs.oracle.com/javase/9/docs/api/java/lang/Boolean.html#Boolean-boolean-