On Fri, 4 Apr 2025 10:37:58 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
>> Please review this test-only PR which introduces testing of the unspecified >> but long-standing fallback to FTP for non-local files in the 'file' URL >> scheme. >> >> This in preparation for the upcoming proposal to disable the feature by >> default in JDK-8353440. >> >> Since we cannot reliably bind an FTP server to port 21, the test instead >> uses an HTTP proxy, binding to an ephemeral port. This menas we don't test >> non-proxy code paths. We still test that the FTP fallback is used, which is >> the key point here. (We aim to test file URL connections, not FTP URL >> connection internals) >> >> An alternative here could be to just verify that the returned URLConnection >> is an instance of FtpURLConnection. However, I opted for an end-to-end test >> here, since the amount of extra code seems reasonable. >> >> By temporarly moving this test to tier1, I was able to confirm this test >> runs green also on Windows GHA. > > Eirik Bjørsnøs has updated the pull request incrementally with one additional > commit since the last revision: > > Use a fake host name since no actual FTP request will be attempted @dfuch Tagnential observation: `proxyServer.stop(2)` will always wait 2 seconds here. I was assuming it would only do that if there was an ongoing exchange. Looking at the `ServerImpl` class, it looks like `finished` is never set to true. Specifically, `ServerImpl.Dispatcher::handleEvent` does not set it to true since that happens before `terminating` is set to true. How is `HttpServer::stop` expected to work? Is something wrong with my expectations that this should return quickly with no inflight exchanges? ------------- PR Comment: https://git.openjdk.org/jdk/pull/24418#issuecomment-2778281722
