Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JDK 21 - Compliance Client's method handlers not receiving the entire request body #1519

Closed
lqiu96 opened this issue Jul 1, 2024 · 1 comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@lqiu96
Copy link
Contributor

lqiu96 commented Jul 1, 2024

We are trying to include testing for JDK 21 and our tests are flaky for the Compliance client. We believe the change is behavior is because of some internal JDK changes updated how the HttpRequests are sent over the network.

In JDK 17 and prior, the internal code had a byte buffer of ~8k bytes but the new changes start the capacity at 512 bytes. We think that this change made it so that it now takes multiple "writes" over the stream to send data over (i.e. it used to be able to be sent in one "write", but now takes 2+ "writes")

We believe that the golang io.Reader's Read() call does not block until all the "writes" have finished. Specifically this line:

bodyReader := io.TeeReader(r.Body, &jsonReader)
would Read and it is possible to only Read the first "write" (given that multiple "writes" could be made for a large request). We have logged the numBytes read (https://pkg.go.dev/io#Reader) and see that some messages see numBytes less than the expected value.

Environment details

  • Programming language: Java
  • OS: Mac + Linux
  • Language runtime version: JDK 21 (I believe this is also an issue from JDK 19+)

Logs from Gapic Showcase Server

Error: 2024/07/01 17:05:20 error reading body params '*': proto: syntax error (line 1:513): invalid value

2024/07/01 17:08:59   urlRequestHeaders:
    User-Agent: "Google-HTTP-Java-Client/1.44.2 (gzip)"
    X-Goog-Api-Client: "gl-java/21.0.1__Eclipse-Adoptium__Temurin-21.0.1-12 gapic/ gax/2.50.1-SNAPSHOT rest/"
    Accept: "*/*"
    Content-Length: "514"
    Accept-Encoding: "gzip"
    Content-Type: "application/json; charset=utf-8"
    Connection: "keep-alive"
2024/07/01 17:08:59 Num Bytes Read: 512

The Content-Length header says there are 514 bytes, but the reader only reports that 512 bytes are read.

The logs above are from this updated snippet in th showcase server:

	var jsonReader bytes.Buffer
	bodyReader := io.TeeReader(r.Body, &jsonReader)
	rBytes := make([]byte, r.ContentLength)
	numBytesRead, err := bodyReader.Read(rBytes)
	if err != nil && err != io.EOF {
		backend.Error(w, http.StatusBadRequest, "error reading body content: %s", err)
		return
	}

	backend.StdLog.Printf("Num Bytes Read: %d", numBytesRead)

Reproducer

Compliance ITs: https://github.com/googleapis/sdk-platform-java/blob/308aeafc9f04795d2e1df8206c84689b11c4323a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITHttpAnnotation.java#L123-L142

Run with JDK 21

Possible Solution

Update the clients to use io.ReadAll(r.body) when reading the request body.

We have made this change locally and ran the Java Compliance ITs 1000x. We do not see this flaky test anymore and have a strong suspicion that this should fix it.

@lqiu96 lqiu96 added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Jul 1, 2024
@lqiu96
Copy link
Contributor Author

lqiu96 commented Jul 10, 2024

Resolved in #1520

@lqiu96 lqiu96 closed this as completed Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

1 participant