Follow the redirects HttpURLConnection declines (307/308, cross-protocol) - #1151
Open
aalhossary wants to merge 1 commit into
Open
Follow the redirects HttpURLConnection declines (307/308, cross-protocol)#1151aalhossary wants to merge 1 commit into
aalhossary wants to merge 1 commit into
Conversation
The JDK follows 301, 302 and 303 within a protocol, but never follows 307 or 308, and never follows a redirect that changes http to https. Both gaps have broken this build: CATH began answering http with a 301 to https, and ECOD now answers with a 308 to a rewritten path. A browser follows either without comment, so neither service had reason to expect it would break us. openConnectionFollowingRedirects handles what the JDK leaves, resolving relative locations, capping the chain at five hops and reporting a loop rather than chasing it. A redirect from https to http is refused: the transport must never be downgraded silently. Anything refused is returned as it is, so checkHttpStatus still decides what a non-2xx status means. The decision is split into redirectTargetFor(code, location, url) so the rules can be tested without a server; the end-to-end tests use a local HttpServer rather than a real service. Fixes biojava#1149
aalhossary
added a commit
to aalhossary/biojava
that referenced
this pull request
Aug 30, 2026
Covers what is merged since 7.2.6 and what is open and expected to land: the download and checksum work, the CATH and ECOD fixes, the contact and ASA performance tweaks, electron density, and the JUnit 5 migration. Five entries are for pull requests that are still open - biojava#1134, biojava#1147, biojava#1148, biojava#1150 and biojava#1151 - and should be checked against what actually merged before the release is tagged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1149.
HttpURLConnectionfollows 301, 302 and 303 within a protocol. It never follows 307 or 308, and never follows a redirect that changes http to https. Both gaps have broken this build:EcodInstallationTestA browser follows either without comment, so neither service had reason to expect the change would break us.
What this adds
openConnectionFollowingRedirects(URL, timeout)handles what the JDK leaves:Locationresolved against the request. ECOD's is relative.checkHttpStatusstill decides what a non-2xx means.Used by
downloadFile,downloadFileWithValidationand theURLoverload ofcreateValidationFiles.The decision is split into
redirectTargetFor(code, location, url)so the rules are testable without a server.Note on ECOD specifically
This deliberately does not change
DOMAINS_PATH. ECOD's own site and API still publish/ecod/distributions/…; the 308 to/ecod-legacy/is an internal rewrite they introduced when their new app took over/ecod/. BioJava's URL is already the advertised one — the only defect was not following the redirect. Hardcoding/ecod-legacy/would pin us to something unadvertised.Testing
mvn test -pl biojava-core -Dtest=FileDownloadRedirectTest— 13 tests, no network:HttpServer: a 308 and a 307 followed, the validated download storing the real body with a matching.size, a loop, an over-long chain, and a redirect with noLocation.Full
biojava-coresuite: 535 tests, no failures.Scope
Other call sites have the same exposure —
AllChemCompProvider,SiftsChainToUniprotMapping,Astral,PDBStatus,URLConnectionTools, andUniprotProxySequenceReader, whose hand-rolled version carries the comment "This method should be moved to a utility class in BioJava 5.0". Migrating them is a follow-up rather than part of this, since each needs its own judgement; inventory filed separately.