Fix bounds check on envelope for 32-bit archs by emcfarlane · Pull Request #887 · connectrpc/connect-go

emcfarlane · 2025-09-26T14:33:46Z

Fix bounds check on construction of payload envelopes for 32-bit archs.

Fixes #886

Signed-off-by: Edward McFarlane <emcfarlane@buf.build>

jhump · 2025-09-27T15:35:21Z

envelope.go

+	sizeUint64 := uint64(size)
+	if size < 0 || sizeUint64 > math.MaxUint32 {
 		return [5]byte{}, fmt.Errorf("connect.makeEnvelopePrefix: size %d out of bounds", size)
 	}
 	prefix := [5]byte{}
 	prefix[0] = flags
-	binary.BigEndian.PutUint32(prefix[1:5], uint32(size))
+	binary.BigEndian.PutUint32(prefix[1:5], uint32(sizeUint64))


Do you really even need the extra var? Also, why not convert to int64 instead of uint64?

Suggested change

sizeUint64 := uint64(size)

if size < 0 || sizeUint64 > math.MaxUint32 {

return [5]byte{}, fmt.Errorf("connect.makeEnvelopePrefix: size %d out of bounds", size)

}

prefix := [5]byte{}

prefix[0] = flags

binary.BigEndian.PutUint32(prefix[1:5], uint32(size))

binary.BigEndian.PutUint32(prefix[1:5], uint32(sizeUint64))

if size < 0 || int64(size) > math.MaxUint32 {

return [5]byte{}, fmt.Errorf("connect.makeEnvelopePrefix: size %d out of bounds", size)

}

prefix := [5]byte{}

prefix[0] = flags

binary.BigEndian.PutUint32(prefix[1:5], uint32(size))

I agree, casting from int (potentially int32 or int64 depending on arch) to uint64 looks odd even though it should not introduce any issues as negative numbers aren't allowed anyways and if size was negative the cast value is larger than MaxUint32. And we also don't need the extra var here as you suggested but that's also just a minor optimization. I'm fine with keeping it like this as the functionality should now be correct but I do prefer your change suggestion for consistency.

The extra var is to guide the linter that the cast is safe. Otherwise we need an annotation or .golangci.lint exception. Cast to uint64 as that seemed the more natural thing to compare to math.MaxUint32.

Didn't know the linter actually complains in that case, nevermind then. 👍

jhump

Seems good, but I think would have expected something ever so slightly different.

MDr164 · 2025-09-29T11:15:33Z

The solution suggested in #888 looks a bit cleaner to me as it makes it more obvious that MaxInt could be smaller than MaxUint for certain systems but either one works.

Signed-off-by: Edward McFarlane <emcfarlane@buf.build>

MDr164

On a somewhat related note: Would it make sense to run the tests that are currently run in CI for 32bit as well? I think the GH Actions Linux ARM runners support 32bit binaries as well (the Windows variant dropped support for 32bit iirc). Or the regular runners could at least build-test the examples, for that you can use 64bit as well of course.

emcfarlane · 2025-09-29T16:01:43Z

@MDr164 GitHub doesn't supply hosted runners on 32Bit archs. We could cross build as a check. I looked for support for a linter in golangci to no avail. We are also planning to do releases which would catch this build as we would need to cross build to all supported targets.

This PR contains the following updates: | Package | Change | Age | Confidence | |---|---|---|---| | [connectrpc.com/connect](https://github.com/connectrpc/connect-go) | `v1.18.1` -> `v1.19.1` | [![age](https://developer.mend.io/api/mc/badges/age/go/connectrpc.com%2fconnect/v1.19.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/connectrpc.com%2fconnect/v1.18.1/v1.19.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>connectrpc/connect-go (connectrpc.com/connect)</summary> ### [`v1.19.1`](https://github.com/connectrpc/connect-go/releases/tag/v1.19.1) [Compare Source](connectrpc/connect-go@v1.19.0...v1.19.1)  #### What's Changed ##### Bugfixes - Fix bounds check on envelope for 32-bit archs by [@emcfarlane](https://github.com/emcfarlane) in [#887](connectrpc/connect-go#887) - Fix CallInfo header/trailer propagation on error responses by [@emcfarlane](https://github.com/emcfarlane) in [#892](connectrpc/connect-go#892) **Full Changelog**: <connectrpc/connect-go@v1.19.0...v1.19.1> ### [`v1.19.0`](https://github.com/connectrpc/connect-go/releases/tag/v1.19.0) [Compare Source](connectrpc/connect-go@v1.18.1...v1.19.0) This release introduces the highly requested "simple" flag for code generation, making Connect significantly more ergonomic for everyday RPC development. The new simple flag in protoc-gen-connect-go generates cleaner, more intuitive client and handler interfaces that eliminate request/response wrappers for most use cases. This addresses community feedback about verbosity and provides a more straightforward API. When enabled, metadata (headers/trailers) can be passed through context instead of explicit wrapper objects, optimizing for the common case where developers don't need direct access to HTTP headers. #### What's Changed ##### Enhancements - Add simple flag for more ergonomic generated code by [@bufdev](https://github.com/bufdev) and [@smaye81](https://github.com/smaye81) in [#851](connectrpc/connect-go#851) - Update Go to v1.24 and document http.Protocol use removing the dependency on `golang.org/x/net/http2` by [@maxbrunet](https://github.com/maxbrunet) in [#873](connectrpc/connect-go#873), [#877](connectrpc/connect-go#877) - Add support for Edition 2024 by [@emcfarlane](https://github.com/emcfarlane) in [#878](connectrpc/connect-go#878) ##### Bugfixes - Include valid spec and headers when calling recover handler for streaming RPCs by [@jhump](https://github.com/jhump) in [#817](connectrpc/connect-go#817) ##### Other changes - Go version support updated to latest two instead of three by [@jhump](https://github.com/jhump) in [#837](connectrpc/connect-go#837) - CI testing improvements by [@pkwarren](https://github.com/pkwarren) and [@jhump](https://github.com/jhump) in [#838](connectrpc/connect-go#838), [#839](connectrpc/connect-go#839) - Code quality improvements by [@mattrobenolt](https://github.com/mattrobenolt) and [@bufdev](https://github.com/bufdev) in [#841](connectrpc/connect-go#841), [#867](connectrpc/connect-go#867) - Documentation improvements by [@adlion](https://github.com/adlion) and [@stefanvanburen](https://github.com/stefanvanburen) in [#821](connectrpc/connect-go#821), [#880](connectrpc/connect-go#880) #### New Contributors - [@adlion](https://github.com/adlion) made their first contribution in [#821](connectrpc/connect-go#821) - [@maxbrunet](https://github.com/maxbrunet) made their first contribution in [#873](connectrpc/connect-go#873) - [@stefanvanburen](https://github.com/stefanvanburen) made their first contribution in [#880](connectrpc/connect-go#880) **Full Changelog**: <connectrpc/connect-go@v1.18.1...v1.19.0> </details> --- ### Configuration 📅 **Schedule**: Branch creation - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC), Automerge - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/1078 Reviewed-by: earl-warren <earl-warren@noreply.code.forgejo.org> Co-authored-by: Renovate Bot <bot@kriese.eu> Co-committed-by: Renovate Bot <bot@kriese.eu>

GoReleaser failed at v0.7.3. https://github.com/grafana/mcp-grafana/releases/tag/v0.7.3 https://github.com/grafana/mcp-grafana/actions/runs/18408789518/job/52455289488 ``` ⨯ release failed after 6m38s error= │ build failed: exit status 1: # connectrpc.com/connect Error: │ ../../../go/pkg/mod/connectrpc.com/connect@v1.19.0/envelope.go:378:24: math.MaxUint32 (untyped int constant 4294967295) overflows int target=linux_386_sse2 Error: The process '/opt/hostedtoolcache/goreleaser-action/2.12.5/x64/goreleaser' failed with exit code 1 ``` This commit updates connect-go to the latest version to resolve the problem. connectrpc/connect-go#887 https://github.com/connectrpc/connect-go/releases/tag/v1.19.1

emcfarlane added 3 commits September 26, 2025 16:32

Fix bounds check on envelope for 32-bit archs

08aa0a2

Signed-off-by: Edward McFarlane <emcfarlane@buf.build>

Fix lint

68968d7

Signed-off-by: Edward McFarlane <emcfarlane@buf.build>

Improve

76d2795

Signed-off-by: Edward McFarlane <emcfarlane@buf.build>

mikhailv mentioned this pull request Sep 27, 2025

Fix size check for envelope prefix (32bit build) #888

Closed

jhump reviewed Sep 27, 2025

View reviewed changes

jhump approved these changes Sep 27, 2025

View reviewed changes

earl-warren approved these changes Sep 28, 2025

View reviewed changes

Merge branch 'main' into ed/fixBoundCheck32Bit

71c0b4d

MDr164 approved these changes Sep 29, 2025

View reviewed changes

emcfarlane added 2 commits September 29, 2025 16:09

Use int46

32f2229

Signed-off-by: Edward McFarlane <emcfarlane@buf.build>

Add doc

d895b48

Signed-off-by: Edward McFarlane <emcfarlane@buf.build>

MDr164 approved these changes Sep 29, 2025

View reviewed changes

emcfarlane merged commit af35892 into main Oct 7, 2025
9 checks passed

emcfarlane deleted the ed/fixBoundCheck32Bit branch October 7, 2025 15:08

tahadostifam mentioned this pull request Oct 7, 2025

connect-go 1.19.0 fails to build on 32bit ARM #886

Closed

This was referenced Oct 7, 2025

fixed integer overflow in envelope.go #893

Closed

Prepare for v1.19.1 #895

Merged

suzuki-shunsuke mentioned this pull request Oct 11, 2025

fix: update connectrpc/connect-go to resolve the compile error grafana/mcp-grafana#316

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix bounds check on envelope for 32-bit archs#887

Fix bounds check on envelope for 32-bit archs#887
emcfarlane merged 6 commits intomainfrom
ed/fixBoundCheck32Bit

emcfarlane commented Sep 26, 2025

Uh oh!

jhump Sep 27, 2025

Uh oh!

MDr164 Sep 29, 2025

Uh oh!

emcfarlane Sep 29, 2025 •

edited

Loading

Uh oh!

MDr164 Sep 29, 2025

Uh oh!

jhump left a comment

Uh oh!

MDr164 commented Sep 29, 2025

Uh oh!

MDr164 left a comment •

edited

Loading

Uh oh!

emcfarlane commented Sep 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

emcfarlane commented Sep 26, 2025

Uh oh!

jhump Sep 27, 2025

Choose a reason for hiding this comment

Uh oh!

MDr164 Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

emcfarlane Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MDr164 Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

jhump left a comment

Choose a reason for hiding this comment

Uh oh!

MDr164 commented Sep 29, 2025

Uh oh!

MDr164 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emcfarlane commented Sep 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

emcfarlane Sep 29, 2025 •

edited

Loading

MDr164 left a comment •

edited

Loading