Make PR_ImportTCPSocket() a public, officially supported function

NEW
Assigned to

Status

NSPR
NSPR
4 years ago
4 years ago

People

(Reporter: Miloslav Trmač, Assigned: Wan-Teh Chang)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release)
Build ID: 20130917102605

Steps to reproduce:

Please make PR_ImportTCPSocket() officially supported, i.e. move it out of the private/ subdirectory.

Having this function available would make it easier to integrate NSS/NSPR with existing server code, where the listen/select/accept loop wouldn't need to know about NSS at all; only the accept()ed OS-level socket would be imported into NSS.

This is related to bug #888581.

Updated

4 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 1

4 years ago
(a) do you suggest to export PR_ImportTCPSocket, only? What about PR_ImportUDPSocket?

(b) function headers in prio.h appear to have complete documentation headers. This change would require a patch that adds such documentation, too.

(c) I don't know how strict the documentation requirements of NSPR are, but ideally the public NSPR reference would have to get extended to document this API.

(d) In file pprio.h, above the declaration of PR_ImportTCPSocket, there's a comment section that explains the special expectations of NSPR. In my understanding, once you import a file handle into NSPR, then NSPR owns the file handle and may change it's attributes. Is such existing behaviour acceptable for your usage scenario? Do you agree that the importing application would have to stop using OS functions on the socket after the import call, but use NSPR APIs, only?

(e) It seems that PR_ImportTCPSocket doesn't work for IPv6 on Windows. This would have to be documented in the API description.

It seems the primary work here is one of documentation and description of assumptions and limitations of the API.
(Reporter)

Comment 2

4 years ago
Created attachment 8341752 [details] [diff] [review]
PR_ImportTCPSocket.patch

(In reply to Kai Engert (:kaie) from comment #1)
> (a) do you suggest to export PR_ImportTCPSocket, only? What about
> PR_ImportUDPSocket?

Only TCP for now.  There is a fairly widespread practice of dealing with TCP sockets at the native level (with POSIX, using the for (;;){ accept();fork(); } iditom for daemons; with OpenSSL, doing accept() and connect() using the OS API and then SSL_set_fd() with an established TCP connection), and integrating NSS into such programs with little disruption is useful.

This is has not been the case for UDP, or at least not to such extent.  Now that NSS supports DTS, it might make more sense, but I don't have any experience with this.  Given the difference in connection establishment between TCP and UDP, I'm not sure that allowing the native-connect/NSPR-usage split for UDP is as necessary.

> (b) function headers in prio.h appear to have complete documentation
> headers. This change would require a patch that adds such documentation, too.
Attached a proof-of-concept patch.
 
> (c) I don't know how strict the documentation requirements of NSPR are, but
> ideally the public NSPR reference would have to get extended to document
> this API.
Yes, this would go to https://developer.mozilla.org/en-US/docs/NSPR_API_Reference/I_O_Functions .

OTOH https://developer.mozilla.org/en-US/docs/NSPR_API_Reference starts with
> I am inclined to delay ANY edits to the text, until this massive text is migrated here (with redirects and all) so that this text becomes the "only" source for NSPR API Reference.

> (d) In file pprio.h, above the declaration of PR_ImportTCPSocket, there's a
> comment section that explains the special expectations of NSPR. In my
> understanding, once you import a file handle into NSPR, then NSPR owns the
> file handle and may change it's attributes. Is such existing behaviour
> acceptable for your usage scenario? Do you agree that the importing
> application would have to stop using OS functions on the socket after the
> import call, but use NSPR APIs, only?
Yes, that's fine.

> (e) It seems that PR_ImportTCPSocket doesn't work for IPv6 on Windows. This
> would have to be documented in the API description.
That's unfortunate.  This seems to be only required for knowing what socket type to pass to AcceptEx; wouldn't it be possible to get this socket type
using iAddressFamily from getsockopt( SO_PROTOCOL_INFO ), as described at http://msdn.microsoft.com/en-us/library/windows/desktop/ms738544%28v=vs.85%29.aspx and http://msdn.microsoft.com/en-us/library/windows/desktop/ms741675%28v=vs.85%29.aspx ?  I'm afraid I have very little Windows programming experience.
> 
> It seems the primary work here is one of documentation and description of
> assumptions and limitations of the API.
You need to log in before you can comment on or make changes to this bug.