fix: isByteResponse returns empty body in BuildResponse after #232

[glowww]

Bug

BuildResponse never reads the response body when isByteResponse=true, producing an empty payload wrapped in a text/plain data URI regardless of the actual response content.

Repro:

srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
    w.Header().Set("Content-Type", "application/octet-stream")
    w.Write([]byte{0x00, 0x01, 0x02, 0x03, 0xFF})
}))
defer srv.Close()

resp, _ := http.Get(srv.URL)
out, _ := BuildResponse("", false, resp, nil, RequestInput{IsByteResponse: true})
fmt.Println(out.Body)

Actual: data:text/plain; charset=utf-8;base64,
Expected: data:application/octet-stream;base64,AAECA/8=

Root cause

#232 (commit d0fc595, "fix EOF errors caused by the charset reader by previewing the response body whether at least one byte exists") added a first-byte preview + io.MultiReader + io.ReadAll to guard against charset.NewReader raising EOF on zero-length text bodies. The block — including the io.ReadAll — was placed inside if !isByteResponse:

if !isByteResponse {
    firstByte := make([]byte, 1)
    n, err := io.ReadFull(resp.Body, firstByte)
    // ...
    if n == 0 {
        respBodyBytes = nil
    } else {
        bodyReader = io.MultiReader(bytes.NewReader(firstByte[:n]), resp.Body)
        bodyReader, err = charset.NewReader(bodyReader, ct)
        // ...
        respBodyBytes, err = io.ReadAll(bodyReader)
    }
}

So when isByteResponse=true, that entire block is skipped. respBodyBytes is never populated; the body is never read. Downstream:

mimeType := http.DetectContentType(respBodyBytes)   // "text/plain; charset=utf-8"
base64.StdEncoding.EncodeToString(respBodyBytes)    // ""

which is exactly the string I see in the repro above.

I think the intent of #232 was to gate charset detection behind !isByteResponse (because charset sniffing is only meaningful for text responses), but the patch ended up gating the whole read behind that branch.

Fix

Hoist the peek + io.ReadAll out of the conditional so both paths read the body. Keep charset.NewReader inside !isByteResponse. The empty-body guard from #232 is preserved via the n == 0 short-circuit, which runs before either path.

firstByte := make([]byte, 1)
n, err := io.ReadFull(resp.Body, firstByte)
if err != nil && !errors.Is(err, io.EOF) {
    return Response{}, NewTLSClientError(err)
}

if n == 0 {
    respBodyBytes = nil
} else {
    bodyReader = io.MultiReader(bytes.NewReader(firstByte[:n]), resp.Body)

    if !isByteResponse {
        // Automatically detect the charset for non-byte responses
        bodyReader, err = charset.NewReader(bodyReader, ct)
        if err != nil {
            return Response{}, NewTLSClientError(err)
        }
    }

    if input.StreamOutputPath != nil {
        respBodyBytes, err = readAllBodyWithStreamToFile(bodyReader, input)
    } else {
        respBodyBytes, err = io.ReadAll(bodyReader)
    }
    if err != nil {
        return Response{}, NewTLSClientError(err)
    }
}

Tests

Added cffi_src/factory_test.go with four cases:

Case	On master	With fix
isByteResponse=true + non-empty binary body	FAIL (byte-response payload is empty)	PASS
isByteResponse=true + empty body	PASS	PASS
isByteResponse=false + empty text body (the #232 case)	PASS	PASS
isByteResponse=false + non-empty text body	PASS	PASS

The first one is the regression guard — it fails on current master with the exact symptom described above.

Scope

One function in cffi_src/factory.go plus a new cffi_src/factory_test.go. No behavior change for non-byte callers; no change to the StreamOutputPath path beyond running in the normalized block. Pre-existing BuildResponse callers in cffi_dist/main.go unaffected.