RFR: 8216363: NullPointerException in java.util.logging.Handler#isLoggable

[Mandy Chung]

Hi Daniel,

I wonder how a null LogRecord can be passed to Handler::isLoggable
in the code path during logging.

The package summary specifies that NPE will be thrown for null
argument unless it's explicitly specified.  Handler.isLoggable
does specify to return false if LogRecord is null.  If you change
the spec to throw NPE if record is null, this impacts all callers
of isLoggable(record) if any of them assumes it returns false
if not.  In addition, logging tends to be gracious as it's a
cross-cutting concern and not to disturb the application runtime
until it's a runtime issue.  So I think isLoggable accepting null
may be by design.

Handler.publish(LogRecord) specifies to ignore null record and
the implementation calls Handler.isLoggable(record). If the spec
is changed to throw NPE, Handler::publish will need to change
too.  This impacts all APIs that accept null LogRecord while
assuming isLoggable(null) returns false.

I think normal logging purpose would pass non-null records
otherwise this issue should have been reported long ago.

Fixing the implementation of Handler::isLoggable to return false
if null to match the specification seems similar risk to changing
the spec.  What do you think?

Mandy

On 2/14/19 11:10 AM, Daniel Fuchs wrote:
> Hi,
> 
> Please find below a doc fix for:
> 
> 8216363: NullPointerException in java.util.logging.Handler#isLoggable
> https://bugs.openjdk.java.net/browse/JDK-8216363
> 
> webrev:
> http://cr.openjdk.java.net/~dfuchs/webrev_8216363/webrev.00/index.html
> 
> java.util.logging.Handler specifies that Handler.isLoggable(null)
> returns false. Unfortunately, the implementation throws NPE instead.
> 
> For the sake of backward compatibility - it might be more
> prudent to simply alter the API specification to document
> the behavior of the default implementation, while allowing
> subclasses to change that behavior. In fact, one already
> does: StreamHandler::isLoggable(LogRecord) already returns
> false and doesn't throw NPE.
> 
> 
> best regards,
> 
> -- daniel