jakartaee / servlet

Jakarta Servlet
https://eclipse.org/ee4j/servlet
Other
266 stars 87 forks source link

Remove usage of java.security.AccessController? #443

Closed arjantijms closed 1 year ago

arjantijms commented 2 years ago

The Cookie class uses the AccessController, which is marked for removal in JDK 17:

static {
        boolean enforced = AccessController.doPrivileged(new PrivilegedAction<Boolean>() {
            @Override
            public Boolean run() {
                return Boolean.valueOf(System.getProperty("org.glassfish.web.rfc2109_cookie_names_enforced", "true"));
            }
        });
        if (enforced) {
            TSPECIALS = "/()<>@,;:\\\"[]?={} \t";
        } else {
            TSPECIALS = ",; ";
        }
    }

Shall we already remove this for Servlet 6.0.0 / Jakarta EE 10?

markt-asf commented 2 years ago

Is there a platform view of whether running under a security manager is support for Jakarta EE 10?

arjantijms commented 2 years ago

@markt-asf There basically is. I brought this to the attention of the platform team in a platform call, and there was consensus for putting out the statement that running under a security manager is at least deprecated.

Patching in @ivargrimstad

In case we don't outright remove it, there are maybe two alternative options:

  1. At least add a comment that this usage of the AccessController is deprecated and will be removed in the future.
  2. Add another switch (yes I know, it's not so nice) to make it possible to use the API without ever having the AccessController actually called. It's still referenced in the code then, but that's a step up from actually calling it.
stuartwdouglas commented 2 years ago

AFAIK the intention is for AccessController to continue to exist in the JDK for a while after removal, it will just become a no-op, so I think we can leave this for now. There is a huge amount of existing code that references AccessController so I don't think it is going to completely disappear for a while.

arjantijms commented 2 years ago

AFAIK the intention is for AccessController to continue to exist in the JDK for a while after removal,

I hope so, although at the moment it does seem to trigger warnings being logged. So that's why at least for the APIs it may not be a bad idea to disable the checks via a switch. Then again, maybe the warning can be disabled by the same kind of switch (at the JDK level).