Skip to content

FileDownloadUtils detects redirects but does not follow the ones HttpURLConnection won't (307/308, cross-protocol) #1149

Description

@aalhossary

FileDownloadUtils learned in #1133 to detect redirects and refuse to cache their bodies. It never learned to follow the ones HttpURLConnection will not follow by itself — and that gap has now broken the build twice in nine months, for two different services.

The JDK's two blind spots

HttpURLConnection follows 301, 302 and 303 within the same protocol. It does not follow 307 or 308 at all, and it does not follow any redirect that changes http to https. Both apply to us right now, with setInstanceFollowRedirects(true):

for (String u : new String[]{
        "http://prodata.swmed.edu/ecod/distributions/ecod.latest.domains.txt",
        "http://download.cathdb.info/cath/releases/"}) {
    HttpURLConnection c = (HttpURLConnection) new URL(u).openConnection();
    c.setRequestMethod("HEAD");
    c.setInstanceFollowRedirects(true);          // the default
    System.out.printf("%s -> %d  Location=%s%n", u, c.getResponseCode(), c.getHeaderField("Location"));
}
.../ecod/distributions/ecod.latest.domains.txt  -> 308  Location=/ecod-legacy/distributions/ecod.latest.domains.txt
http://download.cathdb.info/cath/releases/      -> 301  Location=https://download.cathdb.info/cath/releases/

The ECOD one is the sharper case: same host, same protocol, relative Location — a redirect that would be followed transparently if it were a 301, and is simply dropped because it is a 308.

What it has cost us so far

When Service Redirect Symptom
2025-12 CATH 301 http→https eight months of red CI (#1138); the redirect body was cached as classification data, so the failure surfaced later and elsewhere as an NPE
2026-08-29 ECOD 308 path move nightly red on all EcodInstallationTest (runs 33303708377, 33247173632)

ECOD relaunched their site, so /ecod/ is now an application and the static files were rewritten to /ecod-legacy/distributions/. Their own pages still link to /ecod/distributions/, and their API returns absolute URLs on that path, so the old address remains the advertised one — this is an internal rewrite, not a published move. Chasing the new path in our code would be following an implementation detail they do not advertise, which is why this issue proposes following the redirect rather than editing DOMAINS_PATH.

Worth noting the ECOD failure was reported clearly, by service and URL, because of #1133's status check:

EcodInstallationTest.testDownloads:93 » HttpStatus
  HTTP 308 Permanent Redirect for http://prodata.swmed.edu/ecod/distributions/ecod.develop204.domains.txt

That is the detector doing its job. It is the follower that is missing.

Proposal

Add redirect following to FileDownloadUtils.downloadFileWithValidation (and the connection path behind checkHttpStatus), for the cases the JDK declines:

  • 307 / 308 — re-issue the same method to Location.
  • 301 / 302 / 303 across protocols — re-issue, but only http→https, never https→http, so a redirect can never silently downgrade transport.
  • Resolve relative Location headers against the request URL (ECOD's is relative).
  • Cap the chain, five hops seems ample, and fail with the existing HttpStatusException if exceeded or if a loop is detected.
  • Log the followed hop at INFO once per download, so a permanent move is visible in logs rather than silent — that is how we would learn a service has moved without waiting for it to break.

The behaviour after a successful follow should be exactly as today: validate, write sidecars, atomic move. Nothing about caching changes.

Testing

TestChemCompRedirectNotCached already stands up a local com.sun.net.httpserver.HttpServer and needs no network; the same fixture can serve 301/302/303/307/308, relative and absolute, cross-protocol, and a loop. That keeps the coverage offline and fast, which matters given #1146.

Not proposed here

Switching to java.net.http.HttpClient, which follows 307/308 natively and would make this disappear. That is a larger change than the problem warrants right now, and it is a Java 11 baseline question rather than a bug fix — but it is the obvious long-term direction and worth recording as the alternative.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions