Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom logging in Jetty 12 beta2 can fail due to NullPointerException in org.eclipse.jetty.server.Request #9960

Closed
minduch opened this issue Jun 24, 2023 · 2 comments · Fixed by #9971
Labels
Bug For general bugs on Jetty side

Comments

@minduch
Copy link

minduch commented Jun 24, 2023

Jetty version
Jetty 12 beta2 - Embedded

Java version/vendor (use: java -version)
openjdk version "17.0.7" 2023-04-18
OpenJDK Runtime Environment Temurin-17.0.7+7 (build 17.0.7+7)
OpenJDK 64-Bit Server VM Temurin-17.0.7+7 (build 17.0.7+7, mixed mode, sharing)

OS type/version
Windows 11 - Microsoft Windows [Version 10.0.22631.1906]

Description
In case of errors in the code of the Jetty 12 emebbor, the custom log attempts to write out the request, but will fail due to NullPointerException's. Of course, Jetty is not to blame for the implementors problematic code, but this is nicely handled by try-catches and console logging, except if you add a custom logger to the server.

I have seens various such errors, and most of them are located in the org.eclipse.jetty.server.Request class such as get methods returning a String. For example:

    static String getRemoteAddr(Request request)
    {
        if (request == null)
            return null;
        SocketAddress remote = request.getConnectionMetaData().getRemoteSocketAddress();
        if (remote instanceof InetSocketAddress inetSocketAddress)
        {
            if (inetSocketAddress.isUnresolved())
                return inetSocketAddress.getHostString();

            InetAddress address = inetSocketAddress.getAddress();
            String result = address == null
                ? inetSocketAddress.getHostString()
                : address.getHostAddress();
            return HostPort.normalizeHost(result);
        }
        return remote.toString();
    }

could go NullPointerException at the last line. Perhaps this should be rewritten to something like:

        return (remote != null)? remote.toString(): "null" /* or something else? */;

This applies to some potential methods in the Request class, from what I have seen until now:

  • getLocalAddr
  • getRemoteAddr
  • getServerName

In contrast, you have methods such as getRemotePort not having this problem as it doesn't attempt to use the remotevariable for method calls in its class:

    static int getRemotePort(Request request)
    {
        if (request == null)
            return -1;
        SocketAddress remote = request.getConnectionMetaData().getRemoteSocketAddress();
        if (remote instanceof InetSocketAddress)
            return ((InetSocketAddress)remote).getPort();
        return -1;
    }

Above you see the return -1; call not attempting to use remote that could potentially be null.

How to reproduce?
Having an embedded Jetty with a custom logger and causing exceptions in the embeddors code that are then caught by Jetty handlers.

The one that really came up to be the most troublesome was getRemoteAddr.

@minduch minduch added the Bug For general bugs on Jetty side label Jun 24, 2023
gregw added a commit that referenced this issue Jun 26, 2023
Fix #9960 with NPE protection for bad requests.
gregw added a commit that referenced this issue Jun 26, 2023
Fix #9960 with NPE protection for bad requests.
gregw added a commit that referenced this issue Jun 26, 2023
Improved Locale implementation
@joakime
Copy link
Contributor

joakime commented Jun 26, 2023

PR #9971 opened for this issue

@joakime joakime linked a pull request Jun 26, 2023 that will close this issue
gregw added a commit that referenced this issue Jun 27, 2023
Improved Locale implementation updates from review
gregw added a commit that referenced this issue Jun 27, 2023
Improved Locale implementation using languageTag
gregw added a commit that referenced this issue Jun 28, 2023
gregw added a commit that referenced this issue Jun 28, 2023
@gregw
Copy link
Contributor

gregw commented Jun 28, 2023

finally done.... after locale excursion!

@gregw gregw closed this as completed Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants