Skip to content

Read the plugin request until end of input, not the first short read - #60

Merged
andersfugmann merged 1 commit into
andersfugmann:mainfrom
jeong-sik:fix/read-all-short-reads
Sep 26, 2026
Merged

andersfugmann merged 1 commit into
andersfugmann:mainfrom
jeong-sik:fix/read-all-short-reads

Conversation

@jeong-sik

Copy link
Copy Markdown
Contributor

Problem

read_all in src/plugin/protoc_gen_ocaml.ml stops at the first input call that returns fewer than 1024 bytes, and treats that as the end of the request. Stdlib.input does not promise full reads:

A return value between 0 and [len] exclusive means that not all requested [len] characters were read, either because no more characters were available at that time, or because the implementation found it convenient to do a partial read; input must be called again to read the remaining characters, if desired.

On macOS a pipe hands the first read 512 bytes. The CodeGeneratorRequest is then cut at 512 bytes, and the plugin fails:

Fatal error: exception Ocaml_protoc_plugin.Result.Error (`Premature_end_of_input)
--ocaml_out: protoc-gen-ocaml: Plugin failed with status code 2.

Reproduction

Environment: macOS 26.6.1 (arm64), protoc 34.0 from Homebrew (33.5 fails the same way), OCaml 5.5.1.

  • On main (7cea929), dune runtest fails. There are 10 plugin failures, all Premature_end_of_input. The 6.2.0 release fails the same way.
  • I captured one request protoc sent (16,903 bytes) and fed it to the 6.2.0 plugin. From a file it works. Through a pipe it fails every way I tried: cat request | protoc-gen-ocaml, and a pipe that sends the first 1000 or 2048 bytes and then the rest.
  • A copy of read_all run on that pipe reads 512 of the 16,903 bytes before it stops (OCaml 5.4.0, 5.5.0 and 5.5.1 alike).

Fix

Read until input returns 0. The loop uses only input and Buffer, so it keeps the ocaml >= 4.08.0 bound (In_channel.input_all needs 4.14).

Testing

With this change, on the same machine:

  • dune runtest passes.
  • The rebuilt plugin decodes the captured request piped in whole, and split at 1000 bytes.
  • protoc 34.0 generates code through the rebuilt plugin. Its output differs from the 6.2.0 output only by package_service_name, which is already on main.

🤖 Generated with Claude Code

read_all stopped at the first input call that returned fewer than 1024
bytes. Stdlib.input may return fewer bytes than requested before the end
of input; only 0 means end of file. On macOS a pipe hands the first read
512 bytes, so the CodeGeneratorRequest was cut and decoding failed with
Premature_end_of_input. Read until input returns 0.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
anyang-keepers pushed a commit to jeong-sik/masc that referenced this pull request Sep 26, 2026
…정본으로 pin 해요 (#39217)

* build(opam): pin ocaml-protoc-plugin 6.2.0 with the short-read fix

protoc-gen-ocaml 6.2.0 stops reading its request at the first read shorter
than its 1024-byte buffer. A macOS pipe hands over 512 bytes first, so every
local build of proto/masc_workspace.proto on macOS failed with
Premature_end_of_input; CI on Linux never saw it. The pin is 6.2.0 plus the
one-commit fix from andersfugmann/ocaml-protoc-plugin#60, pinned as 6.2.0 so
the generated code and the lock constraint stay 6.2.0's, and dune-local's pin
guard now tells a switch without it to reinstall.

live_pin_target also reads the whole pin table: an awk that exited at the
match could close the pipe while printf still wrote, and under pipefail that
SIGPIPE ended --check with 141 and no report, which dune-local turned into a
refused build.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* docs(changelog): note the protoc plugin pin

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@andersfugmann

Copy link
Copy Markdown
Owner

Thanks for reporting. The PR looks good. I'll merge once checks have completed.

@andersfugmann andersfugmann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

No unresolved review issues were identified, and all assessments support approval.

Review effort: Lite
Findings: None

What changed in this PR

Fixes plugin request truncation by reading stdin until EOF, handling partial pipe reads correctly.

Changes:

  • Repeatedly reads input until EOF.
  • Reuses buffers while preserving OCaml compatibility.
File Description
src/​plugin/​protoc_gen_ocaml.ml Reads complete protobuf requests from stdin.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@andersfugmann
andersfugmann merged commit 6e3fbe1 into andersfugmann:main Sep 26, 2026
3 checks passed
andersfugmann added a commit to andersfugmann/opam-repository that referenced this pull request Sep 27, 2026
CHANGES:

- Add `package_service_name` to generated service modules, providing the
  complete protobuf service path directly (andersfugmann/ocaml-protoc-plugin#49, thanks @Nymphium)
- Use binary mode for the plugin's standard input and output on Windows
  (andersfugmann/ocaml-protoc-plugin#51, thanks @linsyking)
- Fix JSON decoder initialization when a message references an enum
  nested in another message (andersfugmann/ocaml-protoc-plugin#59, thanks @mbickers)
- Read the complete protoc plugin request instead of treating a short
  input read as end-of-file (andersfugmann/ocaml-protoc-plugin#60, thanks @jeong-sik)
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