You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A glaringly large oversight I made when designing Feather was to assume that the parameter index (which is the , which can range from 0 to 255, fits in a byte. What I missed was that is true for unsigned bytes; Java's byte is a signed type, which ranges from -128 to 127. This causes issues if a parameter index actually falls after 127, which would cause a wraparound of the value (when cast to a byte) to a negative value.
While the various APIs don't seem to validate parameter indexes are positive (which is an oversight), the IO adapters do verify that; for example, the GSON adapter does that validate on read. This causes an asymmetrical problem where creating in-memory objects and exporting them is fine, but reading those exports will cause an error if they contain parameter indexes greater than 127.
The solution is to modify the API such that parameter indexes are stored in a larger data type than a byte. Using a short is acceptable, and would align with the intent of using byte to limit values to within the acceptable range of 0 to 255 for parameter indexes. However, I think usability can be improved by using an int instead, to avoid consumers needing to explicitly downcast parameter stored as ints to bytes. (We will incorporate parameter validation in the APIs, to ensure parameters are still within the 0-255 limit of the JVM.)
This will be a breaking API change, and as such slated for 2.0.0.
The text was updated successfully, but these errors were encountered:
A glaringly large oversight I made when designing Feather was to assume that the parameter index (which is the , which can range from 0 to 255, fits in a
byte
. What I missed was that is true for unsigned bytes; Java'sbyte
is a signed type, which ranges from -128 to 127. This causes issues if a parameter index actually falls after 127, which would cause a wraparound of the value (when cast to abyte
) to a negative value.While the various APIs don't seem to validate parameter indexes are positive (which is an oversight), the IO adapters do verify that; for example, the GSON adapter does that validate on read. This causes an asymmetrical problem where creating in-memory objects and exporting them is fine, but reading those exports will cause an error if they contain parameter indexes greater than 127.
The solution is to modify the API such that parameter indexes are stored in a larger data type than a
byte
. Using ashort
is acceptable, and would align with the intent of usingbyte
to limit values to within the acceptable range of 0 to 255 for parameter indexes. However, I think usability can be improved by using anint
instead, to avoid consumers needing to explicitly downcast parameter stored asint
s tobyte
s. (We will incorporate parameter validation in the APIs, to ensure parameters are still within the 0-255 limit of the JVM.)This will be a breaking API change, and as such slated for 2.0.0.
The text was updated successfully, but these errors were encountered: