Copilot commented on code in PR #445:
URL:
https://github.com/apache/pulsar-client-node/pull/445#discussion_r2629321040
##########
src/Message.cc:
##########
@@ -156,6 +165,57 @@ Napi::Value Message::GetProducerName(const
Napi::CallbackInfo &info) {
return Napi::String::New(env,
pulsar_message_get_producer_name(this->cMessage.get()));
}
+Napi::Value Message::GetEncryptionContext(const Napi::CallbackInfo &info) {
+ Napi::Env env = info.Env();
+ if (!ValidateCMessage(env)) {
+ return env.Null();
+ }
+
+ auto encCtxOpt = this->cMessage.get()->message.getEncryptionContext();
+ if (!encCtxOpt) {
+ return env.Null();
+ }
+
+ // getEncryptionContext returns std::optional<const EncryptionContext*>
+ const pulsar::EncryptionContext *encCtxPtr = *encCtxOpt;
+ if (!encCtxPtr) {
+ return env.Null();
+ }
+ const pulsar::EncryptionContext &encCtx = *encCtxPtr;
+
+ if (encCtx.keys().empty() && encCtx.param().empty() &&
encCtx.algorithm().empty()) {
+ return env.Null();
+ }
+
+ Napi::Object obj = Napi::Object::New(env);
+
+ Napi::Array keys = Napi::Array::New(env);
+ int i = 0;
+ for (const auto &keyInfo : encCtx.keys()) {
+ Napi::Object keyObj = Napi::Object::New(env);
+ keyObj.Set("key", Napi::String::New(env, keyInfo.key));
+ keyObj.Set("value", Napi::String::New(env, keyInfo.value));
+
+ Napi::Object metadataObj = Napi::Object::New(env);
+ for (const auto &meta : keyInfo.metadata) {
+ metadataObj.Set(meta.first, Napi::String::New(env, meta.second));
+ }
+ keyObj.Set("metadata", metadataObj);
+
+ keys.Set(i++, keyObj);
+ }
+ obj.Set("keys", keys);
+
+ obj.Set("param", Napi::String::New(env, encCtx.param()));
+ obj.Set("algorithm", Napi::String::New(env, encCtx.algorithm()));
+ obj.Set("compressionType", Napi::Number::New(env,
(int)encCtx.compressionType()));
Review Comment:
The TypeScript type definition indicates that compressionType should be of
type CompressionType (a union of string literals: 'Zlib' | 'LZ4' | 'ZSTD' |
'SNAPPY'), but the C++ implementation returns it as a number. This creates a
type mismatch between the implementation and the TypeScript definition. The C++
code should either convert the enum value to the appropriate string
representation to match the TypeScript type, or the TypeScript type should be
updated to number.
##########
src/Message.cc:
##########
@@ -20,6 +20,14 @@
#include "Message.h"
#include "MessageId.h"
#include <pulsar/c/message.h>
+#include <pulsar/Message.h>
+#include <pulsar/MessageBuilder.h>
+#include <pulsar/EncryptionContext.h>
+
+struct _pulsar_message {
+ pulsar::MessageBuilder builder;
+ pulsar::Message message;
+};
Review Comment:
This struct definition is accessing the internal implementation details of
the pulsar_message_t opaque type. This is fragile and relies on undocumented
internal structure. The struct layout could change in future versions of the
Pulsar C++ library, breaking this code. Consider using the official Pulsar C
API to access encryption context if available, or ensure this approach is
documented and validated against the specific Pulsar version being used.
```suggestion
```
##########
tests/encryption.test.js:
##########
@@ -0,0 +1,152 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+const path = require('path');
+const fs = require('fs');
+const Pulsar = require('../index');
+
+class MyCryptoKeyReader extends Pulsar.CryptoKeyReader {
+ constructor(publicKeys, privateKeys) {
+ super();
+ this.publicKeys = publicKeys;
+ this.privateKeys = privateKeys;
+ }
+
+ getPublicKey(keyName, _metadata) {
+ const keyPath = this.publicKeys[keyName];
+ if (keyPath) {
+ try {
+ const key = fs.readFileSync(keyPath);
+ return { key, _metadata };
+ } catch (e) {
+ return null;
+ }
+ }
+ return null;
+ }
+
+ getPrivateKey(keyName, _metadata) {
+ const keyPath = this.privateKeys[keyName];
+ if (keyPath) {
+ try {
+ const key = fs.readFileSync(keyPath);
+ return { key, _metadata };
Review Comment:
The returned object uses '_metadata' as the field name, but the C++ code
expects 'metadata' (as seen in parseEncryptionKeyInfo at line 67 in
CryptoKeyReader.cc). This mismatch means the metadata will not be properly
parsed and passed to the underlying Pulsar library. Change '_metadata' to
'metadata' in the return statement.
##########
tests/encryption.test.js:
##########
@@ -0,0 +1,152 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+const path = require('path');
+const fs = require('fs');
+const Pulsar = require('../index');
+
+class MyCryptoKeyReader extends Pulsar.CryptoKeyReader {
+ constructor(publicKeys, privateKeys) {
+ super();
+ this.publicKeys = publicKeys;
+ this.privateKeys = privateKeys;
+ }
+
+ getPublicKey(keyName, _metadata) {
+ const keyPath = this.publicKeys[keyName];
+ if (keyPath) {
+ try {
+ const key = fs.readFileSync(keyPath);
+ return { key, _metadata };
+ } catch (e) {
+ return null;
+ }
+ }
+ return null;
+ }
+
+ getPrivateKey(keyName, _metadata) {
+ const keyPath = this.privateKeys[keyName];
+ if (keyPath) {
+ try {
+ const key = fs.readFileSync(keyPath);
+ return { key, _metadata };
+ } catch (e) {
+ return null;
+ }
+ }
+ return null;
+ }
+}
+
+(() => {
+ describe('Encryption', () => {
+ let client;
+ const publicKeyPath = path.join(__dirname,
'certificate/public-key.client-rsa.pem');
+ const privateKeyPath = path.join(__dirname,
'certificate/private-key.client-rsa.pem');
+
+ beforeAll(() => {
+ client = new Pulsar.Client({
+ serviceUrl: 'pulsar://localhost:6650',
+ operationTimeoutSeconds: 30,
+ });
+ });
+
+ afterAll(async () => {
+ await client.close();
+ });
+
+ test('End-to-End Encryption', async () => {
+ const topic =
`persistent://public/default/test-encryption-${Date.now()}`;
+
+ const cryptoKeyReader = new MyCryptoKeyReader(
+ { 'my-key': publicKeyPath },
+ { 'my-key': privateKeyPath },
+ );
+
+ const producer = await client.createProducer({
+ topic,
+ encryptionKeys: ['my-key'],
+ cryptoKeyReader,
+ cryptoFailureAction: 'FAIL',
+ });
+
+ const consumer = await client.subscribe({
+ topic,
+ subscription: 'sub-encryption',
+ cryptoKeyReader,
+ cryptoFailureAction: 'CONSUME',
+ subscriptionInitialPosition: 'Earliest',
+ });
+
+ const msgContent = 'my-secret-message';
+ await producer.send({
+ data: Buffer.from(msgContent),
+ });
+
+ const msg = await consumer.receive();
+ expect(msg.getData().toString()).toBe(msgContent);
Review Comment:
The test verifies that the message content is correctly decrypted but
doesn't verify the encryption context for successfully decrypted messages.
Consider adding assertions to verify that getEncryptionContext() returns the
expected encryption metadata even when decryption succeeds, to ensure the
feature works completely in the success case as well.
##########
tests/encryption.test.js:
##########
@@ -0,0 +1,152 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+const path = require('path');
+const fs = require('fs');
+const Pulsar = require('../index');
+
+class MyCryptoKeyReader extends Pulsar.CryptoKeyReader {
+ constructor(publicKeys, privateKeys) {
+ super();
+ this.publicKeys = publicKeys;
+ this.privateKeys = privateKeys;
+ }
+
+ getPublicKey(keyName, _metadata) {
+ const keyPath = this.publicKeys[keyName];
+ if (keyPath) {
+ try {
+ const key = fs.readFileSync(keyPath);
+ return { key, _metadata };
Review Comment:
The returned object uses '_metadata' as the field name, but the C++ code
expects 'metadata' (as seen in parseEncryptionKeyInfo at line 67 in
CryptoKeyReader.cc). This mismatch means the metadata will not be properly
parsed and passed to the underlying Pulsar library. Change '_metadata' to
'metadata' in the return statement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
