Limit WireFormatInfo platform details to a reasonable length#2088
Conversation
Now that apache#2083 limits value sizes for properties inside of WireFormatInfo we should add a check to make sure platform details do not go past that limit. Limit has been set to 256 bytes as that should be more than enough as the details are usually less than half that. This is not turned of by default but if someone is using it this will prevent potential future problems for too large a value.
| details = platformInfo.toString(); | ||
| } catch (Throwable e) { | ||
| // truncate to the max allowed length if too long | ||
| details = platformInfo.length() > PLATFORM_DETAILS_MAX_LENGTH ? |
There was a problem hiding this comment.
This cuts off the details without any indication they were cut off, which is probably fine. I have seen some implementation do this by inserting an ellipsis after a given point and then appending a small bit trailing value such that you get a sort of visual indication that it was clipped. Probably not needed here so just making sure you thought about that.
There was a problem hiding this comment.
Yeah I think it's fine here, it's off by default and its now 512 and shouldn't really be hit but we could improve it later if needed. This is more of just a fail safe. I doubt anyone even turns this setting on to be honest.
|
Some practical limits from a quick search show 256 is right up at the practical usage. 384 or 512 may be slightly better to avoid concatenation for known real-world data values. Note: Cloud provider kernel version numbers can get long.
|
|
@mattrpav - thanks for pointing that out, i can just bump to 512 which is still not that big. 512 is our default max so I can just make it the same I suppose |
| static final int MAX_PROPERTY_SIZE = 64; | ||
| // Used to validate property values that allocate buffers, limit to 512 bytes | ||
| static final int MAX_PROPERTY_BUFFER_SIZE = 512; | ||
| public static final int MAX_PROPERTY_BUFFER_SIZE = 512; |
There was a problem hiding this comment.
Good catch I forgot that you made the default for WireFormatInfo 512 from the original 256
Now that #2083 limits value sizes for properties inside of WireFormatInfo we should add a check to make sure platform details do not go past that limit. Limit has been set to 512 bytes as that should be more than enough as the details are usually less than half that. This is not turned of by default but if someone is using it this will prevent potential future problems for too large a value. (cherry picked from commit 60af386)
Now that #2083 limits value sizes for properties inside of WireFormatInfo we should add a check to make sure platform details do not go past that limit. Limit has been set to 512 bytes as that should be more than enough as the details are usually less than half that. This is not turned of by default but if someone is using it this will prevent potential future problems for too large a value. (cherry picked from commit 60af386)
cshannon
changed the title
3 participants
Now that #2083 limits value sizes for properties inside of WireFormatInfo we should add a check to make sure platform details do not go past that limit. Limit has been set to 512 bytes as that should be more than enough as the details are usually less than half that. This is not turned of by default but if someone is using it this will prevent potential future problems for too large a value.