Incorrect "battr" value produces null pointer on JSON deseriailzation about openrtb HOT 4 CLOSED

You have a good point/find that deserializing this JSON should never fail with a NullPointerException. I guess the best option is not failing at all, just ignoring the unsupported values. Will be fixed in next release.

Now on the second problem of providing some kind of support for non-standard values: this is one explicit tradeoff made by the library, we wanted both the safety and convenience of fields typed as enumerations, instead of having tons of int32 fields everywhere. The flipside is that supporting extended enum values is not possible, at least not in any elegant way (yes I could play with protobuf's support for "unknown values"... it would require reserving separate tag IDs and hacks in several places). But the most important factor is that this tradeoff is actually minimal, because this kind of extension is not generally allowed; the only enum explicitly documented to allow exchange-specific values is the at (Auction Type) IIRC; and even there you have to use ordinal numbers above 500, you can't just pick the first "free" number. This is highly likely to clash with new standard creative attributes that may be added in future versions of the spec (unless the spec just adopts as standard this new attribute from Rubicon; but that doesn't scale to multiple vendors doing the same hack). In the native spec this is similar, vendor-specific enums all above 500, except that this is explicitly allowed for all enums from that spec.

I suggest that you fork the library and just add a new enum element for attribute 17, if not losing it is critical.

from openrtb.

Hi,

17 is just a value to localize error for you, that we have an error on values above maximum provided by specification. I have no need to handle this specific value. Rubicon used values above 10000 in attached example. Maybe it's an outdated sample.

Your solution is absolutely fine with me if using values not from specification is prohibited be spec for this attribute. In that case - look at it issue as a report about NullPointer.

One general note - it would be great to have two options dealing with elements not from schema/specification - "ignore" and "fail whole process with exception".

Thank you,
Dmitry

from openrtb.

Happy to know Rubicon doesn't break the spec in this way, that seemed surprising considering their role creating the standard :) The NPE was an easy fix... can't wait for Protobuf 3 GA so we can stop writing handcoded JSON parsers for this :(

Optional failure is a good idea too.

from openrtb.

Just got in the code to fix this, you can test with master but we should have a release soon.

from openrtb.