Skip to content

loader: include the standard HTTP cookie jar - #5655

Open
yohimik wants to merge 3 commits into
tinygo-org:devfrom
yohimik:codex/http-cookiejar
Open

yohimik wants to merge 3 commits into
tinygo-org:devfrom
yohimik:codex/http-cookiejar

Conversation

@yohimik

@yohimik yohimik commented Sep 5, 2026

Copy link
Copy Markdown

The net/http directory override hides Go's net/http/cookiejar package.

Merge net/http to expose the standard cookie jar. Keep TinyGo's HTTP files and subpackage overrides. Add overrides for cgi and fcgi so these unsupported packages remain unavailable. Initialize src/net in the Nix job.

Tests:

  • Walk the real src/net/http tree and check override coverage. A directory override includes its full subtree.
  • Check that the merge selects TinyGo HTTP files and subpackages, selects Go's cookie jar, and excludes Go's cgi and fcgi.
  • Run a host test that stores a secure cookie and refuses it over HTTP. Mark this test as host-only.

Local validation: both loader tests passed in a harness with the unchanged production merge functions. Checks with a missing override failed as expected. The cookie test passed with Go. Full local loader tests could not build because LLVM headers are absent.

@sum-elier

Copy link
Copy Markdown

Does this fix #4484?

@yohimik

yohimik commented Sep 19, 2026

Copy link
Copy Markdown
Author

yes

@yohimik
yohimik marked this pull request as ready for review September 19, 2026 20:33
@deadprogram

Copy link
Copy Markdown
Member

Thanks @yohimik I have a few important notes edited from an automated review.

  1. This is the biggest one. The PR description is mostly about the Crier and Dispat releases, fork CI and binary sizes. For a six-file loader change this hides the content. Please shorten it to the problem, the change and the specific tests. If you follow AGENTS.md it should help to reduce the amount of text. The large amount of text makes it nearly impossible to review changes. Despite that, here are a few specific comments.

  2. With net/http/ merged, only TinyGo files are linked into the merged GOROOT. A TinyGo subdirectory appears only if it has an entry in pathsToOverride. Because src/net is a submodule with its own release cycle, a new subpackage added there would disappear from the merged GOROOT with no error. The new test uses a synthetic tree, so it cannot find this. Please add a test that walks the real src/net/http and checks that each subdirectory has an override entry.

  3. Go's net/http/cgi and net/http/fcgi now also come through the merge, but they do not build against the TinyGo crypto/tls. An import of net/http/cgi fails with net/http/cgi/child.go:109:32: unknown field HandshakeComplete in struct literal of type tls.ConnectionState, and net/http/fcgi fails in the same way because it imports cgi. Before this change the message was that the package is not in std. Please either add override entries for these two directories, or record this change of behavior in the PR description.

  4. In main_test.go, the reason for the second runPlatTests call is not obvious. Please add a short comment that cookiejar.go is a host-only test.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants