LDEV-5387 / PR #3502: Redundant GET requests in HTTPResource fallback

hacking
1

Following the recent merge of PR #3502 for LDEV-5387, Tom Chiverton commented on Jira:

“Does this PR cause a double GET ? I don’t see it saving the response from the fall back GET to be used later”

Looking at HTTPResource.java, Tom’s concern is valid — and depending on the operation, it can trigger up to 3 separate GET requests and 2 HEAD requests for a single tag/function call.

Where it happens

In core/.../HTTPResource.java:

if (rejectsHead(code)) {
    HTTPResponse rsp = null;
    try {
        rsp = HTTPEngine4Impl.get(url, data.username, data.password, _getTimeout(), true, null, data.userAgent, pd, null);
        code = rsp.getStatusCode();
    }
    finally {
        HTTPEngine.closeEL(rsp); // <--- Closed and discarded immediately
    }
}
headStatusCode = code;
return code;

The fallback GET in getStatusCode() reads the status, closes the connection in finally, and only records headStatusCode = code. Crucially, it does not assign rsp to this.http.

The request traces

  1. fileRead(url):

    • ResourceUtil.toResourceExisting() → res.exists() → getStatusCode():
      • Request 1 (HEAD): returns 403 / 405.
      • Request 2 (GET #1): fallback GET in getStatusCode() to confirm status, immediately closed.
    • res.getInputStream() → calls getHTTPResponse(true):
      • Request 3 (GET #2): brand-new GET over the wire to pull the stream.
    • Total: 1 HEAD + 2 GETs

  2. <cfcontent file="url">:

    • toResourceExisting() → exists():
      • Request 1 (HEAD) (fails 403)
      • Request 2 (GET #1) (fallback in getStatusCode(), closed)
    • file.getContentType() (if type omitted):
      • Checks if (http == null) → Request 3 (HEAD #2) (still fails 403)
    • file.length():
      • Calls getHTTPResponse(false). Since http == null, fires Request 4 (GET #2), reads length, and closes in finally.
    • file.getInputStream():
      • Calls getHTTPResponse(true), firing Request 5 (GET #3) to stream the response.
    • Total: 2 HEADs + 3 GETs

Why the test missed it

In test/tickets/LDEV5387.cfc, the mock server tracks requests via attributes.requests.add(method & " " & path), but the spec only asserts:

expect( trim( result.fileContent ) ).toBe( "LDEV-5387" );

There are no assertions checking arrayLen(attributes.requests) or verifying that only a single GET is issued.

Questions for the team / bot

  1. Can we update HTTPResource to preserve this.http when the fallback GET succeeds, rather than closing it in getStatusCode()?
  2. How should HTTPResource.getInputStream() handle reuse vs recreation (currently hardcoded as getHTTPResponse(true)) when an open response stream already exists on the resource?
  3. Could we add a request-count assertion to test/tickets/LDEV5387.cfc to prevent multi-request regressions going forward?
  4. is @GianTschingt configured to see replies on jira tickets? I also sent a PM on this forum but it wasn’t read, afaik.
2

Thanks Phil, and Tom for spotting it. You’re right: the fallback GET only keeps the status code and throws away the response, so a server that rejects HEAD gets hit several times. We’ll fix it on 7.1 so the response from the fallback GET is reused instead of sending new requests, and we’ll add a request-count assertion to the LDEV-5387 test so this can’t come back unnoticed. The ticket is Jira

Jira comments do reach us. The forum is still the quickest way to get a reply, though.