JDK-8263442 : Potential bug in jdk.internal.net.http.common.Utils.CONTEXT_RESTRICTED
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.net
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2021-03-11
  • Updated: 2024-01-10
  • Resolved: 2021-03-23
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 17
17 b15Fixed
Related Reports
Relates :  
JDK-8323292 - Improper handling of Proxy-Authorization header
Relates :  
JDK-8213189 - Make restricted headers in HTTP Client configurable and remove Date by default
Description
SonarCloud reports:
  Ensure this "Optional" could never be null and remove this null-check.

Here:
 
    public static final BiPredicate<String, String> CONTEXT_RESTRICTED(HttpClient client) {
        return (k, v) -> client.authenticator() == null ||  // <---- here
                ! (k.equalsIgnoreCase("Authorization")
                        && k.equalsIgnoreCase("Proxy-Authorization"));
    }

client.authenticator() returns Optional<Authenticator>, so maybe client.authenticator().isPresent() was intended?
Comments
Changeset: bd7a184b Author: Michael McMahon <michaelm@openjdk.org> Date: 2021-03-23 13:25:56 +0000 URL: https://git.openjdk.java.net/jdk/commit/bd7a184b
23-03-2021
Cool. So we can drop "Potential" from the synopsis. Feel free :)
11-03-2021
I don't understand the report. What is the "optional" referred to? However, there is a bug in the predicate. It should be: public static final BiPredicate<String, String> CONTEXT_RESTRICTED(HttpClient client) { return (k, v) -> !client.authenticator().isPresent() || (!k.equalsIgnoreCase("Authorization") && !k.equalsIgnoreCase("Proxy-Authorization")); } This predicate should return true if there is no authenticator set, or if there is an authenticator but so long as the header is not one of the "authorization" headers. I guess Sonarcloud is detecting the erroneous !(x==A && x==B) which can never return false
11-03-2021
It's clearly a bug. I've attached a test case which demonstrates the issue. 200 OK is returned with the suggested fix above, and 500 with the current JDK. It's extremely unlikely that anyone was setting an authenticator AND setting the authorization header. And you're right the Optional test is required as well. It shouldn't be testing for null. Should be !isPresent(). Will adjust the suggested fix.
11-03-2021
Yes - that's probaly why the other bug (&& instead of ||) went unnoticed. Anyway as I said this might not be a "simple" bug. We don't want to suddenly break code that might have taken the opportunity to supply preemptive authorization headers even in the presence of an authenticator (if that works). So the first step would be to verify if that could work. If it can't maybe we can experiment with a fix. If it can, then maybe we should simply have this predicate always return 'true'.
11-03-2021
client.authenticator() is Optional<Authenticator>. Comparing `client.authenticator() == null` is probably incorrect, and needs to be `client.authenticator().isPresent()`? Updated the report to make it clear.
11-03-2021
This would need investigation: I am not sure we even need to restrict these headers when an authenticator is present. So maybe we should consider changing this BiPredicate to always return true (which is what it does today since the returned optional is never null). Some reading of the AutheticationFilter and some testing are probably required before attempting a fix.
11-03-2021