add function to set default certificate subject for ESM compatibility#1519
add function to set default certificate subject for ESM compatibility#1519PriyankuBiswas-EH wants to merge 4 commits into
Conversation
|
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a supported way to mutate the module-level defaultCertificateSubject for ESM consumers by introducing a setter function.
Changes:
- Introduced
setDefaultCertificateSubject(value)to updatedefaultCertificateSubjectfrom within the module - Documented why direct assignment to an exported
letdoesn’t work for ESM namespace imports and provided an example
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…idation and error handling
| // 3. Validate the overall shape: one or more /KEY=VALUE segments. | ||
| // Keys are one or more word characters; values may be anything except a raw slash | ||
| // (nested values would need quoting, which is out of scope here). | ||
| const subjectPattern = /^(?:\/[A-Za-z][A-Za-z0-9]*=[^/]+)+$/; |
There was a problem hiding this comment.
We have a utility to parse a CN string and verify it in node-opcua-pki or node-opcua-crupt. I'd suggest reusing existing material rather than maintaining a hard-to-validate REGEXP.
We need a single source of truth.
There was a problem hiding this comment.
Hi @erossignon , I updated the code as per your comment, reusing the existing function.
…prove error handling
The "export let" in "node-opcua-common/make_subject.ts" is technically mutable from within the module, but ESM module namespace objects are frozen from the outside. The cleanest short-term fix is using require() in a CJS context. The real fix needs to come from the library itself by exposing a setter function.
The library should expose a setter function like "setDefaultCertificateSubject(value: string)" that mutates the internal variable from within the module (which is always allowed). This is the proper fix. The current design is broken for ESM consumers.