Bug 66666 - Remove non-reachable functions from ssl.c
Summary: Remove non-reachable functions from ssl.c
Status: RESOLVED FIXED
Alias: None
Product: Tomcat Native
Classification: Unclassified
Component: Library (show other bugs)
Version: 2.0.2
Hardware: All All
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-06-22 09:18 UTC by Michael Osipov
Modified: 2023-06-29 07:10 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Osipov 2023-06-22 09:18:11 UTC
The following methods have been deprecated from its Java counterpart SSL.java, but have never been removed from ssl.c:

    @Deprecated
    public static native boolean randLoad(String filename);
    @Deprecated
    public static native boolean randSave(String filename);
    @Deprecated
    public static native boolean randMake(String filename, int len,
                                          boolean base64);
    @Deprecated
    public static native long newBIO(long pool, BIOCallback callback)
            throws Exception;
    @Deprecated
    public static native int closeBIO(long bio);
    @Deprecated
    public static native void setPasswordCallback(PasswordCallback callback);
    @Deprecated
    public static native void setPassword(String password);
    @Deprecated
    public static native String getLastError();
    @Deprecated
    public static native boolean hasOp(int op);
    @Deprecated
    public static native void setBIO(long ssl, long rbio, long wbio);
    @Deprecated
    public static native int getError(long ssl, int ret);
    @Deprecated
    public static native void setShutdown(long ssl, int mode);
    @Deprecated
    public static native String getNextProtoNegotiated(long ssl);

I believe we can safely remove the C code from ssl.c
Comment 1 Michael Osipov 2023-06-22 10:35:15 UTC
As far as I can see, when setPasswordCallback() is gone

typedef struct {
    char            password[SSL_MAX_PASSWORD_LEN];
    const char     *prompt;
    tcn_callback_t cb;
} tcn_pass_cb_t;

can be reduced to
typedef struct {
    char            password[SSL_MAX_PASSWORD_LEN];
    const char     *prompt;
} tcn_pass_cb_t;

and the code associated with since no callback can be passed anymore from Java, the tcnative builtin callback will be used.
Comment 2 Christopher Schultz 2023-06-22 20:26:01 UTC
We have been hesitant to remove things from tcnative even if they are not used by the Java components.

Some (unspecified) downstream users may be relying on these "extraneous" functions.

Are you proposing that these be removed from tcnative 2.x or also from 1.2.x?

I would support removing them from 2.x but maybe not 1.2.x.
Comment 3 Michael Osipov 2023-06-23 08:08:06 UTC
(In reply to Christopher Schultz from comment #2)
> We have been hesitant to remove things from tcnative even if they are not
> used by the Java components.
> 
> Some (unspecified) downstream users may be relying on these "extraneous"
> functions.
> 
> Are you proposing that these be removed from tcnative 2.x or also from 1.2.x?
> 
> I would support removing them from 2.x but maybe not 1.2.x.

Hi Chris,

I do not intend to touch 1.2.x in this regard. Only 2.x. The functions to be removed aren't part of a public API, we do not install any header files either. It should have been done with https://github.com/apache/tomcat-native/commit/e7db81bf5a493b44fbc1772c49fbc063c250b05a#diff-610e4592fa703b3c32818bf4baec54fdb8092d1b54819a1d86643fed3920f8e1.
If someone has truly relied on the C code only, he's done a serious mistake. Our C code is meant to be accessed through Java only.
Comment 4 Mark Thomas 2023-06-23 08:13:45 UTC
Let me check and see if there is a reason I didn't remove this code or if I just missed it.
Comment 5 Mark Thomas 2023-06-23 08:44:04 UTC
I can't see any reason I kept those other than oversight. No objections to removal in 2.0.x.
Comment 6 Michael Osipov 2023-06-23 08:51:24 UTC
(In reply to Mark Thomas from comment #5)
> I can't see any reason I kept those other than oversight. No objections to
> removal in 2.0.x.

Great, will work on a PR.
Comment 7 Michael Osipov 2023-06-23 08:57:56 UTC
Moving back to new...
Comment 8 Michael Osipov 2023-06-23 10:53:38 UTC
PR provided.
Comment 9 Michael Osipov 2023-06-29 07:10:09 UTC
Fixed in:
- main for 2.0.5 and onwards