Gathering certificates can get stuck if the server doesn't respond

[danthe1st]

Someone on Stack Overflow experienced an issue where installing updates got stuck waiting for the certificates (which was introduced with #235).

httpResponse.get() might wait forever if the request takes forever which is possible because no timeout is set and HttpClient blocks forever by default. I didn't find any other uses of HttpRequest.newBuilder() in p2.

I suggest adding a timeout to this and I am willing to contribute that (unless someone else really wants to do that).

Are there any opinions on a good timeout? I would suggest something along the line of 2-5 seconds since the HTTP request taking more time would indicate some sort of issue in which case not accepting the certificate is probably fine.

[merks]

Here are some timeout values used by ECF which is used by p2.

Having a default value that can be modified via a system property would be good I think.

The tricky question is how to handle a timeout, with a retry like in other places in p2? And, most importantly, how is the failure to identify/retrieve the host certificate presented to the user?

Is this some type of problem with the host not supporting a head request? Is there a way I can reproduce the problem. Sometime is a little fishy if we can load the repository but we cannot get a certificate...

[danthe1st]

Is this some type of problem with the host not supporting a head request?

I think it happens if the server accepts the connection but infinitely waits for the response so it would only happen if something is wrong with the server (but it should still not lead to it being stuck forever). I don't know what exactly caused it for the user who experienced it but I guess the server causing the issue probably had some issue with HEAD requests or similar.

The tricky question is how to handle a timeout, with a retry like in other places in p2?

I guess using some sort of retry makes sense. For how to present the failure, the other code for certificate handling just doesn't add the certificate which I think makes sense. However, I think at least adding a warning to the error log makes sense.

Having a default value that can be modified via a system property would be good I think.

I agree with that and I think it might make sense to allow configuring it with an existing system property like reusing sun.net.client.defaultConnectTimeout such that users don't need to configure yet another property.

[danthe1st]

I can easily reproduce using the following steps:

• Run nc -l 8080 to create a socket on port 8080
• Go to Preferences > Install/Update > Trust > Authorities
• Change http to Allow
• Click Add
• Enter http://localhost:8080

Then it automatically executes gatherCertificates() which takes forever without a timeout.

Given that information, it seems to not be necessarily specific to HEAD request (at least when adding the authority manually).

[merks]

FYI, I'm on Windows and my bash doesn't have an nc command...

[danthe1st]

FYI, I'm on Windows and my bash doesn't have an nc command...

Entering new ServerSocket(8080).accept() (or similar) in jshell should do the same as it isn't necessary to do anything with the connection.