Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Support for Unsigned Integer Elements, Enhance JSON Encoding/Decoding Robustness, and Replace sprintf with Safer snprintf #526

Merged
merged 6 commits into from
Jul 7, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions Sources/KSCrashRecording/Monitors/KSCrashMonitor_AppState.c
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,35 @@ static int onIntegerElement(const char *const name, const int64_t value, void *c
return onFloatingPointElement(name, value, userData);
}

static int onUnsignedIntegerElement(const char *const name, const uint64_t value, void *const userData)
{
KSCrash_AppState *state = userData;

if (strcmp(name, kKeyFormatVersion) == 0) {
if (value != kFormatVersion) {
KSLOG_ERROR("Expected version 1 but got %" PRIu64, value);
return KSJSON_ERROR_INVALID_DATA;
}
} else if (strcmp(name, kKeyLaunchesSinceLastCrash) == 0) {
if (value <= INT_MAX) {
state->launchesSinceLastCrash = (int)value;
} else {
KSLOG_ERROR("launchesSinceLastCrash (%" PRIu64 ") exceeds INT_MAX", value);
return KSJSON_ERROR_INVALID_DATA;
}
} else if (strcmp(name, kKeySessionsSinceLastCrash) == 0) {
if (value <= INT_MAX) {
state->sessionsSinceLastCrash = (int)value;
} else {
KSLOG_ERROR("sessionsSinceLastCrash (%" PRIu64 ") exceeds INT_MAX", value);
return KSJSON_ERROR_INVALID_DATA;
}
}

// For other fields or if the value doesn't fit in an int, treat it as a floating point
return onFloatingPointElement(name, (double)value, userData);
}

static int onNullElement(__unused const char *const name, __unused void *const userData) { return KSJSON_OK; }

static int onStringElement(__unused const char *const name, __unused const char *const value,
Expand Down Expand Up @@ -184,6 +213,7 @@ static bool loadState(const char *const path)
callbacks.onEndData = onEndData;
callbacks.onFloatingPointElement = onFloatingPointElement;
callbacks.onIntegerElement = onIntegerElement;
callbacks.onUnsignedIntegerElement = onUnsignedIntegerElement;
callbacks.onNullElement = onNullElement;
callbacks.onStringElement = onStringElement;

Expand Down
109 changes: 96 additions & 13 deletions Sources/KSCrashRecordingCore/KSJSONCodec.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@
#include <ctype.h>
#include <errno.h>
#include <fcntl.h>
#include <float.h>
#include <inttypes.h>
#include <limits.h>
#include <math.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
Expand Down Expand Up @@ -276,27 +278,87 @@ int ksjson_addFloatingPointElement(KSJSONEncodeContext *const context, const cha
{
int result = ksjson_beginElement(context, name);
unlikely_if(result != KSJSON_OK) { return result; }
char buff[30];
sprintf(buff, "%lg", value);
return addJSONData(context, buff, (int)strlen(buff));

char buff[64];
int written;

if (isnan(value)) {
written = snprintf(buff, sizeof(buff), "null");
} else if (isinf(value)) {
written = snprintf(buff, sizeof(buff), value > 0 ? "1e999" : "-1e999");
} else {
// Check if the value can be represented as a float
float floatValue = (float)value;
if (fabs(value - floatValue) <= FLT_EPSILON * fabs(value)) {
// It's within float precision, so format it as a float
written = snprintf(buff, sizeof(buff), "%.*g", FLT_DIG, floatValue);
} else {
// It's a double, so use full double precision
written = snprintf(buff, sizeof(buff), "%.*g", DBL_DIG, value);
}

// Ensure it's a valid JSON number
if (written > 0 && written < (int)sizeof(buff)) {
char *dot = strchr(buff, '.');
char *e = strchr(buff, 'e');
if (dot == NULL && e == NULL) {
// Add ".0" for integers
written = snprintf(buff, sizeof(buff), "%.1f", value);
} else if (dot != NULL && e == NULL) {
// Trim trailing zeros for fractions
char *end = buff + written - 1;
while (end > dot && *end == '0') {
*end-- = '\0';
written--;
}
// If we accidentally trimmed everything after the dot, add a zero back
if (end == dot) {
*++end = '0';
written++;
}
}
}
}

if (written < 0 || written >= (int)sizeof(buff)) {
GLinnik21 marked this conversation as resolved.
Show resolved Hide resolved
return KSJSON_ERROR_INVALID_DATA;
}

return addJSONData(context, buff, written);
}

int ksjson_addIntegerElement(KSJSONEncodeContext *const context, const char *const name, int64_t value)
{
int result = ksjson_beginElement(context, name);
unlikely_if(result != KSJSON_OK) { return result; }
char buff[30];
sprintf(buff, "%" PRId64, value);
return addJSONData(context, buff, (int)strlen(buff));

char buff[21]; // Enough for -9223372036854775808 to 9223372036854775807
int written;

written = snprintf(buff, sizeof(buff), "%" PRId64, value);

if (written < 0 || written >= (int)sizeof(buff)) {
return KSJSON_ERROR_INVALID_DATA;
}

return addJSONData(context, buff, written);
}

int ksjson_addUIntegerElement(KSJSONEncodeContext *const context, const char *const name, uint64_t value)
{
int result = ksjson_beginElement(context, name);
unlikely_if(result != KSJSON_OK) { return result; }
char buff[30];
sprintf(buff, "%" PRIu64, value);
return addJSONData(context, buff, (int)strlen(buff));

char buff[21]; // Enough for 0 to 18446744073709551615
int written;

written = snprintf(buff, sizeof(buff), "%" PRIu64, value);

if (written < 0 || written >= (int)sizeof(buff)) {
return KSJSON_ERROR_INVALID_DATA;
}

return addJSONData(context, buff, written);
}

int ksjson_addNullElement(KSJSONEncodeContext *const context, const char *const name)
Expand Down Expand Up @@ -886,10 +948,21 @@ static int decodeElement(const char *const name, KSJSONDecodeContext *context)
}

if (!isFPChar(*context->bufferPtr) && !isOverflow) {
if (sign > 0 || accum <= ((uint64_t)LLONG_MAX + 1)) {
int64_t signedAccum = (int64_t)accum;
signedAccum *= sign;
return context->callbacks->onIntegerElement(name, signedAccum, context->userData);
if (sign > 0) {
if (accum <= (uint64_t)LLONG_MAX) {
// Positive number within int64_t range
return context->callbacks->onIntegerElement(name, (int64_t)accum, context->userData);
} else {
// Positive number exceeding int64_t range, use unsigned
return context->callbacks->onUnsignedIntegerElement(name, accum, context->userData);
}
} else {
if (accum <= ((uint64_t)LLONG_MAX + 1)) {
// Negative number within int64_t range
int64_t signedAccum = -(int64_t)accum;
return context->callbacks->onIntegerElement(name, signedAccum, context->userData);
}
// If negative and exceeding int64_t range, fall through to floating point
}
}

Expand Down Expand Up @@ -1016,6 +1089,14 @@ static int addJSONFromFile_onIntegerElement(const char *const name, const int64_
return result;
}

static int addJSONFromFile_onUnsignedIntegerElement(const char *const name, const uint64_t value, void *const userData)
{
JSONFromFileContext *context = (JSONFromFileContext *)userData;
int result = ksjson_addUIntegerElement(context->encodeContext, name, value);
context->updateDecoderCallback(context);
return result;
}

static int addJSONFromFile_onNullElement(const char *const name, void *const userData)
{
JSONFromFileContext *context = (JSONFromFileContext *)userData;
Expand Down Expand Up @@ -1072,6 +1153,7 @@ int ksjson_addJSONFromFile(KSJSONEncodeContext *const encodeContext, const char
.onEndData = addJSONFromFile_onEndData,
.onFloatingPointElement = addJSONFromFile_onFloatingPointElement,
.onIntegerElement = addJSONFromFile_onIntegerElement,
.onUnsignedIntegerElement = addJSONFromFile_onUnsignedIntegerElement,
.onNullElement = addJSONFromFile_onNullElement,
.onStringElement = addJSONFromFile_onStringElement,
};
Expand Down Expand Up @@ -1127,6 +1209,7 @@ int ksjson_addJSONElement(KSJSONEncodeContext *const encodeContext, const char *
.onEndData = addJSONFromFile_onEndData,
.onFloatingPointElement = addJSONFromFile_onFloatingPointElement,
.onIntegerElement = addJSONFromFile_onIntegerElement,
.onUnsignedIntegerElement = addJSONFromFile_onUnsignedIntegerElement,
.onNullElement = addJSONFromFile_onNullElement,
.onStringElement = addJSONFromFile_onStringElement,
};
Expand Down
50 changes: 44 additions & 6 deletions Sources/KSCrashRecordingCore/KSJSONCodecObjC.m
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ - (id)initWithEncodeOptions:(KSJSONEncodeOption)encodeOptions decodeOptions:(KSJ
_callbacks->onEndData = onEndData;
_callbacks->onFloatingPointElement = onFloatingPointElement;
_callbacks->onIntegerElement = onIntegerElement;
_callbacks->onUnsignedIntegerElement = onUnsignedIntegerElement;
_callbacks->onNullElement = onNullElement;
_callbacks->onStringElement = onStringElement;
_prettyPrint = (encodeOptions & KSJSONEncodeOptionPretty) != 0;
Expand Down Expand Up @@ -197,6 +198,14 @@ static int onIntegerElement(const char *const cName, const int64_t value, void *
return onElement(codec, name, element);
}

static int onUnsignedIntegerElement(const char *const cName, const uint64_t value, void *const userData)
{
NSString *name = stringFromCString(cName);
id element = [NSNumber numberWithUnsignedLongLong:value];
KSJSONCodec *codec = (__bridge KSJSONCodec *)userData;
return onElement(codec, name, element);
}

static int onNullElement(const char *const cName, void *const userData)
{
NSString *name = stringFromCString(cName);
Expand Down Expand Up @@ -279,17 +288,46 @@ static int encodeObject(KSJSONCodec *codec, id object, NSString *name, KSJSONEnc
}

if ([object isKindOfClass:[NSNumber class]]) {
switch (CFNumberGetType((__bridge CFNumberRef)object)) {
CFNumberType numberType = CFNumberGetType((__bridge CFNumberRef)object);
switch (numberType) {
case kCFNumberFloat32Type:
case kCFNumberFloat64Type:
case kCFNumberFloatType:
case kCFNumberCGFloatType:
case kCFNumberDoubleType:
return ksjson_addFloatingPointElement(context, cName, [object doubleValue]);
case kCFNumberCharType:
return ksjson_addBooleanElement(context, cName, [object boolValue]);
default:
return ksjson_addIntegerElement(context, cName, [object longLongValue]);
// Char could be signed or unsigned, so we need to check its value
if ([object charValue] == 0 || [object charValue] == 1) {
return ksjson_addBooleanElement(context, cName, [object boolValue]);
}
// Fall through to integer handling if it's not a boolean
case kCFNumberSInt8Type:
case kCFNumberSInt16Type:
case kCFNumberSInt32Type:
case kCFNumberSInt64Type:
case kCFNumberShortType:
case kCFNumberIntType:
case kCFNumberLongType:
case kCFNumberLongLongType:
case kCFNumberNSIntegerType:
case kCFNumberCFIndexType:
// Check if the value is negative
if ([object compare:@0] == NSOrderedAscending) {
bamx23 marked this conversation as resolved.
Show resolved Hide resolved
return ksjson_addIntegerElement(context, cName, [object longLongValue]);
} else {
// Non-negative value, could be larger than LLONG_MAX
return ksjson_addUIntegerElement(context, cName, [object unsignedLongLongValue]);
}
default: {
// For any unhandled types, try unsigned first, then fall back to signed if needed
unsigned long long unsignedValue = [object unsignedLongLongValue];
if (unsignedValue > LLONG_MAX) {
return ksjson_addUIntegerElement(context, cName, unsignedValue);
} else {
return ksjson_addIntegerElement(context, cName, [object longLongValue]);
}
}
}
}

Expand Down Expand Up @@ -360,7 +398,7 @@ + (NSData *)encode:(id)object options:(KSJSONEncodeOption)encodeOptions error:(N
KSJSONCodec *codec = [self codecWithEncodeOptions:encodeOptions decodeOptions:KSJSONDecodeOptionNone];

int result = encodeObject(codec, object, NULL, &JSONContext);
if (error != nil) {
if (error != NULL) {
*error = codec.error;
}
return result == KSJSON_OK ? data : nil;
Expand All @@ -378,7 +416,7 @@ + (id)decode:(NSData *)JSONData options:(KSJSONDecodeOption)decodeOptions error:
code:0
description:@"%s (offset %d)", ksjson_stringForError(result), errorOffset];
}
if (error != nil) {
if (error != NULL) {
*error = codec.error;
}

Expand Down
12 changes: 12 additions & 0 deletions Sources/KSCrashRecordingCore/include/KSJSONCodec.h
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,18 @@ typedef struct KSJSONDecodeCallbacks {
*/
int (*onIntegerElement)(const char *name, int64_t value, void *userData);

/** Called when an unsigned integer element is decoded.
*
* @param name The element's name.
*
* @param value The element's value.
*
* @param userData Data that was specified when calling ksjson_decode().
*
* @return KSJSON_OK if decoding should continue.
*/
int (*onUnsignedIntegerElement)(const char *name, uint64_t value, void *userData);

/** Called when a null element is decoded.
*
* @param name The element's name.
Expand Down
Loading
Loading