Skip to content

Four JNI wrappers skip ReleaseStringUTFChars() when isCopy is JNI_FALSE #27

Description

@K-ANOY

I found four hand-written JNI wrappers that only release the string returned by GetStringUTFChars() when the isCopy out-parameter is JNI_TRUE. isCopy reports whether the VM made a copy or returned a direct pointer — it does not say whether a release is required. Every successful GetStringUTFChars() must be paired with ReleaseStringUTFChars(), so when the VM returns a direct pointer (isCopy == JNI_FALSE) these methods leak the acquisition on their normal success path.

File: src/java/gs1encoders_wrap.c

Functions: gs1encoderSetDataStrJNI, gs1encoderSetAIdataStrJNI,
gs1encoderSetScanDataJNI, gs1encoderGetDLuriJNI

Relevant code (all four share the same condition; gs1encoderSetDataStrJNI shown):

str = (*env)->GetStringUTFChars(env, value, &isCopy);
ret = gs1_encoder_setDataStr((gs1_encoder*)ctx, str);

if (isCopy == JNI_TRUE)                                  // wrong guard
    (*env)->ReleaseStringUTFChars(env, value, str);

return ret;

Both the copied buffer (isCopy == JNI_TRUE) and the direct pointer (isCopy == JNI_FALSE) are borrowed through JNI and must be handed back with
ReleaseStringUTFChars(); the release is also what ends any pinning/bookkeeping the
VM set up for the acquisition. Returning from the native method does not release it
implicitly. So on any JVM (or string representation) that returns a direct pointer,
each call to these four methods leaves an outstanding acquisition — reachable from the
ordinary public GS1Encoder.setDataStr() / setAIdataStr() / setScanData() /
getDLuri() operations, and invisible to functional tests because the result is still
correct.

The same file already shows the correct rule in gs1encoderInitExJNI(), which does
not request isCopy and releases every successfully acquired non-null pointer:

if (syntaxDictionary) {
    path = (*env)->GetStringUTFChars(env, syntaxDictionary, NULL);
    opts.syntaxDictionary = path;
}
ret = (jlong)gs1_encoder_init_ex(NULL, &opts);
if (path != NULL)
    (*env)->ReleaseStringUTFChars(env, syntaxDictionary, path);

Suggested fix: drop the isCopy check and release every successful acquisition; also
null-check the result so a failed acquisition is not passed into the C API. For
gs1encoderSetDataStrJNI (and identically for the other two setters):

const char* str = (*env)->GetStringUTFChars(env, value, NULL);
if (str == NULL) {
    return JNI_FALSE;   /* exception already pending */
}
jboolean ret = gs1_encoder_setDataStr((gs1_encoder*)ctx, str);
(*env)->ReleaseStringUTFChars(env, value, str);
return ret;

For the optional stem in gs1encoderGetDLuriJNI:

const char* str = stem ? (*env)->GetStringUTFChars(env, stem, NULL) : NULL;
if (stem != NULL && str == NULL) {
    return NULL;
}
const char* out = gs1_encoder_getDLuri((gs1_encoder*)ctx, (char*)str);
if (str != NULL) {
    (*env)->ReleaseStringUTFChars(env, stem, str);
}
return out ? (*env)->NewStringUTF(env, out) : NULL;

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions