Skip to content

Commit ea9573c

Browse files
sdssds
authored
Preserve standard input bytes when forwarding hooks (sds#925)
Overcommit forwards ad hoc hook input using `puts`, which adds a newline to an empty stream or to an unterminated record. For a post-rewrite consumer such as git-branchless, an empty stream becomes a blank record and fails with `Invalid rewrite line`. Read hook stdin and write the supplied subprocess input in binary mode. This preserves empty input, record terminators, and arbitrary bytes on Windows as well as Unix, while still closing stdin so the child sees EOF. Fixes sds#869. Reproduced locally through an installed post-rewrite CustomScript using a small parser with git-branchless's empty-stream/blank-record behavior. Before the fix, empty input fails with `Invalid rewrite line`, and unterminated input gains a newline. The fixture verifies arguments and exact bytes for empty, unterminated, LF, multiline, and CRLF input. Subprocess tests also cover binary data; existing expectations that assumed an added newline now expect the actual supplied input. Validation: full local RSpec suite (1,738 examples); RuboCop on all changed Ruby files. Full Linux/Windows CI runs on the PR. Also verified against git-branchless 0.11.1 downloaded into `/tmp` and initialized only in a disposable repository: forwarding an empty stream exits 0; forwarding the injected blank line exits 1 with `Invalid rewrite line`. No system installation or existing repository configuration was changed. The first Windows CI run exposed stdin CRLF conversion before forwarding; HookContext now reads the original stream in binary mode as well. The CRLF integration case remains identical on every platform.
1 parent aa7cd10 commit ea9573c

5 files changed

Lines changed: 62 additions & 4 deletions

File tree

‎lib/overcommit/hook_context/base.rb‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,10 @@ def input_string
9999
return @input_string if defined?(@input_string)
100100

101101
@input_mutex.synchronize do
102-
@input_string = @input.read unless defined?(@input_string)
102+
unless defined?(@input_string)
103+
@input.binmode if @input.respond_to?(:binmode)
104+
@input_string = @input.read
105+
end
103106
end
104107

105108
@input_string

‎lib/overcommit/subprocess.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ def spawn(args, options = {})
3535
process.start
3636
if options[:input]
3737
begin
38-
process.io.stdin.puts(options[:input])
38+
process.io.stdin.binmode
39+
process.io.stdin.write(options[:input])
3940
rescue StandardError
4041
# Silently ignore if the standard input stream of the spawned
4142
# process is closed before we get a chance to write to it. This

‎spec/integration/custom_script_spec.rb‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
# frozen_string_literal: true
2+
3+
require 'spec_helper'
4+
require 'json'
5+
6+
describe 'post-rewrite CustomScript input' do
7+
around do |example|
8+
repo do
9+
echo({ 'PostRewrite' => {
10+
'ALL' => { 'enabled' => false },
11+
'CustomScript' => {
12+
'enabled' => true,
13+
'command' => [Gem.ruby, 'record-rewrite.rb']
14+
}
15+
} }.to_yaml, '.overcommit.yml')
16+
echo(<<~RUBY, 'record-rewrite.rb')
17+
require 'json'
18+
STDIN.binmode
19+
input = STDIN.read
20+
# Like git-branchless, accept an empty stream but reject a blank record.
21+
input.each_line do |line|
22+
abort 'Invalid rewrite line' unless line.strip.match?(/\\A[0-9a-f]{40} [0-9a-f]{40}\\z/)
23+
end
24+
File.write('received.json', JSON.generate(args: ARGV, bytes: input.bytes))
25+
RUBY
26+
install_overcommit_hooks
27+
example.run
28+
end
29+
end
30+
31+
pair = "#{'a' * 40} #{'b' * 40}"
32+
['', pair, "#{pair}\n", "#{pair}\n#{pair}\n", "#{pair}\r\n"].each do |input|
33+
it "forwards the rewrite type and exact input bytes for #{input.inspect}" do
34+
result = Overcommit::Subprocess.spawn(
35+
[Gem.ruby, '.git/hooks/post-rewrite', 'rebase'], input: input,
36+
)
37+
38+
expect(result.status).to eq(0), result.stdout + result.stderr
39+
JSON.parse(File.read('received.json')).should == { 'args' => ['rebase'],
40+
'bytes' => input.bytes }
41+
end
42+
end
43+
end

‎spec/overcommit/subprocess_spec.rb‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,18 @@ def received_argv(result)
5151

5252
it 'passes the input to the standard input stream of the process' do
5353
subject.should be_success
54-
subject.stdout.chomp.should == 'from-stdin'
54+
subject.stdout.should == 'from-stdin'
55+
end
56+
end
57+
58+
['', 'no newline', "one line\n", "two\nlines\n", "binary\0\r\n\xFF".b].each do |input|
59+
it "preserves input bytes for #{input.inspect}" do
60+
result = described_class.spawn(
61+
[ruby, '-e', 'STDIN.binmode; STDOUT.print STDIN.read.bytes.join(",")'],
62+
input: input,
63+
)
64+
result.should be_success
65+
result.stdout.should == input.bytes.join(',')
5566
end
5667
end
5768

‎spec/overcommit/utils_spec.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@
321321
subject { described_class.execute(arguments, input: input) }
322322

323323
it 'passes the input to the standard input stream' do
324-
subject.stdout.should == "Hello world\n"
324+
subject.stdout.should == 'Hello world'
325325
end
326326
end
327327

0 commit comments

Comments
 (0)