Expose {libnspr4, libnss3, libsmime3} as Libray objects

NEW
Assigned to

Status

()

Toolkit
OS.File
5 years ago
2 years ago

People

(Reporter: Yoric, Assigned: Emma Benoit, Mentored)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [lang=c++][good first bug])

Attachments

(1 attachment, 2 obsolete attachments)

In bug 855325, glandium wrote:

«
Speaking with bsmedberg on IRC, an alternative that would be both future-proof (because we may fold libraries even more and nss3 could end up going away too) and easier on addons would be to expose some constant or function that allows to request library x (nspr4, nss3, smime3, you name it) and get back a ctypes library (as would be returned by ctypes.open) for the right library, whatever it is on the given platform.

There is already something along these lines with OS.Constants.Path.libxul, although it returns a path (but it's only used with ctypes.open).
»
Summary: Expose OS.Constants.Path.{libnspr4, libnss3, libsmime3} → Expose {libnspr4, libnss3, libsmime3} as Libray objects
This should be a good mentored bug, for someone willing to understand js-ctypes.
Whiteboard: [mentor=Yoric][lang=js]
Glandium, given that you're at the origin of this topic, you're probably the person who knows best how it should be used. Would you be satisfied if this was made available as a .jsm (for the main thread)/.js (for worker threads)?
Flags: needinfo?(mh+mozilla)
WFM. Since it's to expose ctypes libraries, it doesn't have to be available from C++.
Flags: needinfo?(mh+mozilla)

Comment 4

5 years ago
Hi! I am just a beginner but I am interested in working on that bug.
Assignee: nobody → lelipse
Nonguierma, I haven't heard from you since our initial contact. What's your status on that bug? Do you need assistance?
Flags: needinfo?(lelipse)
I realize that the description of the bug is rather unclear, plus I have given it a little thought, so here's an updated description:

Some of our JavaScript code needs to be able to determine the path to some of our C libraries: libnspr4, libnss3, libsmime3. At the moment, there is no good way to do this, and the objective of this bug is to add this feature.

We have similar code to let us find the path to libxul from JavaScript, residing in dom/system/OSFileConstants.cpp, so the code that publishes these paths should go in the same file.
Mike, what is the best way to find the path to libnspr4, libnss3, libsmime3?
Should we call PR_GetLibraryFilePathname?
Flags: needinfo?(lelipse) → needinfo?(mh+mozilla)
(Sorry for the long delay, I missed it in my dashboard)

(In reply to David Rajchenbach Teller [:Yoric] from comment #7)
> Mike, what is the best way to find the path to libnspr4, libnss3, libsmime3?
> Should we call PR_GetLibraryFilePathname?

Why not just hardcode it depending on what the preprocessor says about MOZ_FOLD_LIBS?
Flags: needinfo?(mh+mozilla)
Mike, you can you be more precise for our contributor?
Flags: needinfo?(mh+mozilla)
How are you doing with this bug, Nonguierma?
Flags: needinfo?(lelipse)
De-assigning, as discussed over e-mail.
Mike, I'm still interested in details regarding comment 8.
Assignee: lelipse → nobody
Flags: needinfo?(lelipse)
Whiteboard: [mentor=Yoric][lang=js] → [mentor=Yoric][lang=js][Async:P3]
glandium: ping on the needinfo.

Updated

4 years ago
Whiteboard: [mentor=Yoric][lang=js][Async:P3] → [Async:ready][mentor=Yoric][lang=js]
ping glandium
For the foreseeable future, all those libs are ${DLL_PREFIX}nss3${DLL_SUFFIX} when MOZ_FOLD_LIBS is set, and individual libraries when it is not set.
Flags: needinfo?(mh+mozilla)
And what's the name of the individual libraries? ${DLL_PREFIX}nspr4${DLL_SUFFIX}, etc?
Flags: needinfo?(mh+mozilla)
yes
Flags: needinfo?(mh+mozilla)
Some details for whoever decides to pick this bug.

The code lives in dom/system/OSFileConstants.cpp.
The objective is to define the following constants:
 - OS.Constants.Path.libnspr4;
 - OS.Constants.Path.libnss3;
 - OS.Constants.Path.libsmime3.

If preprocessor directive MOZ_FOLD_LIBS is set, these constants should be identical to the existing constant OS.Constants.Path.libxul. Otherwise, these constants should be defined by contacenating DLL_PREFIX + "nspr4" + DLL_SUFFIX.

For more details on how to define constants, see the definition of OS.Constants.Path.libxul, in the same file: http://dxr.mozilla.org/mozilla-central/source/dom/system/OSFileConstants.cpp?from=OSFileConstants.cpp#895-912
Whiteboard: [Async:ready][mentor=Yoric][lang=js] → [mentor=Yoric][lang=c++][good first bug]
Mentor: dteller@mozilla.com
Whiteboard: [mentor=Yoric][lang=c++][good first bug] → [lang=c++][good first bug]

Comment 18

3 years ago
Hello, I am a newbie and I would like to fix this bug. Could someone guide me with this?

Comment 19

3 years ago
This would be my first bug, so I would like to ask this. Firstly is the directive MOZ_FOLD_LIBS set? Or am I expected to hard code it just like for the libxul library in the OSFileConstants.cpp ?

Comment 20

3 years ago
Oh so upon more reading of libxul definition I understand that I am supposed to code these libraries also in the same manner right?
Since Surya also contacted me by e-mail, let me paste the relevant bit of the exchange:

  nsAutoString libnspr4;
#if defined(MOZ_FOLD_LIBS)
  // libnspr4 is folded inside libxul
  libnspr4 = libxul;
#else
  // libsnpr4 lives in its own library
  libnspr4.AppendLiteral(DLL_PREFIX);
  libnspr4.AppendLiteral("nspr4");
  libnspr4.AppendLiteral(DLL_SUFFIX);
#endif // defined MOZ_FOLD_LIBS

if (!SetStringProperty(cx, objPath, "libnspr4", libnspr4)) {
  return false;
}
(Assignee)

Comment 22

3 years ago
Created attachment 8443953 [details] [diff] [review]
Exposing libnss3, libnspr4 and libsmime3 as library objects

Sorry Surya if you had started working on it, as it was unassigned I fixed it.
(Assignee)

Updated

3 years ago
Attachment #8443953 - Flags: review?(dteller)
Comment on attachment 8443953 [details] [diff] [review]
Exposing libnss3, libnspr4 and libsmime3 as library objects

Review of attachment 8443953 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me.
I'm not putting r+ yet because I'd like some tests.
Take a look at function `test_xul` in dom/system/tests/worker_constants.js. This function tests that we can open libxul and import DumpJSStack(), which is one of the functions of libxul.

We need to pick 
- a function from libnss3 – for instance https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/SSL_functions/sslfnc.html#__NSS_Init_
- a function from libnspr4 – for instance https://developer.mozilla.org/en-US/docs/PL_strcpy
- a function from libsmime3 - I'm not quite sure yet.
Attachment #8443953 - Flags: review?(dteller) → feedback+
Comment on attachment 8443953 [details] [diff] [review]
Exposing libnss3, libnspr4 and libsmime3 as library objects

Review of attachment 8443953 [details] [diff] [review]:
-----------------------------------------------------------------

Folded libraries are all in libnss3, not libxul.
(Assignee)

Updated

3 years ago
Assignee: nobody → kylma
(Assignee)

Comment 25

3 years ago
Created attachment 8444079 [details] [diff] [review]
Fix folding library to libnss3, tests for libnss3/nspr4 added

Can someone confirm that ctypes.int is the right type to use for the function
NSS_Init which returns a SECStatus ?

I'm confused by the function to use for libnspr4: the documentation is unclear
whether it's PL_strcpy or PR_strcpy. I tried both but none seems to be exported
in nspr4.
(Assignee)

Updated

3 years ago
Attachment #8443953 - Attachment filename: libnss3_nspr4_smime3_as_library_object → bug_862317_0001
Attachment #8443953 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8444079 - Flags: feedback?(dteller)

Comment 26

3 years ago
Hey so does this mean I can't work on this anymore?? I am new to this hence asking!
Surya: Well, Emma is nearly done, so it would probably be more productive if you found another bug.
(In reply to Emma Benoit from comment #25)
> Created attachment 8444079 [details] [diff] [review]
> Fix folding library to libnss3, tests for libnss3/nspr4 added
> 
> Can someone confirm that ctypes.int is the right type to use for the function
> NSS_Init which returns a SECStatus ?

In case of doubt, you can use `void*`. Since we're not going to actually call the function, that shouldn't be a problem.

> I'm confused by the function to use for libnspr4: the documentation is
> unclear
> whether it's PL_strcpy or PR_strcpy. I tried both but none seems to be
> exported
> in nspr4.

Weird. Do you have ideas about that, Mike?
Flags: needinfo?(mh+mozilla)
Comment on attachment 8444079 [details] [diff] [review]
Fix folding library to libnss3, tests for libnss3/nspr4 added

Review of attachment 8444079 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.

::: dom/system/tests/worker_constants.js
@@ +83,5 @@
> +  isnot(null, OS.Constants.Path.libnss3, "libnss3 is defined");
> +  try {
> +    lib = ctypes.open(OS.Constants.Path.libnss3);
> +    lib.declare("NSS_Init", ctypes.default_abi, ctypes.int, ctypes.char.ptr);
> +  } catch(x) {

Nit: `catch (x)`
Attachment #8444079 - Flags: feedback?(dteller) → feedback+
(Assignee)

Comment 30

3 years ago
Created attachment 8446101 [details] [diff] [review]
So which functions should I use to test libsmime3 and libnspr4 ?

Can someone confirm that ctypes.int is the right type to use for the function
NSS_Init which returns a SECStatus ?

I'm confused by the function to use for libnspr4: the documentation is unclear
whether it's PL_strcpy or PR_strcpy. I tried both but none seems to be exported
in nspr4.
(Assignee)

Updated

3 years ago
Attachment #8444079 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8446101 - Attachment description: Fix folding library to libnss3, tests for libnss3/nspr4 added → So which functions should I use to test libsmime3 and libnspr4 ?
Attachment #8446101 - Attachment is patch: false
PL_strcpy is supposed to be exported, and it's used by some gecko code, so if it's not exported, there's a problem.
Flags: needinfo?(mh+mozilla)
Attachment #8446101 - Attachment is patch: true
I realize that this we somehow lost the ball on this bug. Sorry about that, Emma. Are you still interested in working on it?
Flags: needinfo?(kylma)
(Assignee)

Comment 33

3 years ago
I'm still interested to work on it
Flags: needinfo?(kylma)
You need to log in before you can comment on or make changes to this bug.