|
1 From c0f5de1ad275cf9a2d7f572be76e7b10fcd3e48f Mon Sep 17 00:00:00 2001 |
|
2 From: Joe Tsai <[email protected]> |
|
3 Date: Tue, 16 Aug 2016 16:03:00 -0700 |
|
4 Subject: [PATCH 03/38] [release-branch.go1.7] compress/flate: make |
|
5 huffmanBitWriter errors persistent |
|
6 |
|
7 For persistent error handling, the methods of huffmanBitWriter have to be |
|
8 consistent about how they check errors. It must either consistently |
|
9 check error *before* every operation OR immediately *after* every |
|
10 operation. Since most of the current logic uses the previous approach, |
|
11 we apply the same style of error checking to writeBits and all calls |
|
12 to Write such that they only operate if w.err is already nil going |
|
13 into them. |
|
14 |
|
15 The error handling approach is brittle and easily broken by future commits to |
|
16 the code. In the near future, we should switch the logic to use panic at the |
|
17 lowest levels and a recover at the edge of the public API to ensure |
|
18 that errors are always persistent. |
|
19 |
|
20 Fixes #16749 |
|
21 |
|
22 Change-Id: Ie1d83e4ed8842f6911a31e23311cd3cbf38abe8c |
|
23 Reviewed-on: https://go-review.googlesource.com/27200 |
|
24 Reviewed-by: Matthew Dempsky <[email protected]> |
|
25 Reviewed-by: Brad Fitzpatrick <[email protected]> |
|
26 Reviewed-on: https://go-review.googlesource.com/28634 |
|
27 Reviewed-by: Joe Tsai <[email protected]> |
|
28 Run-TryBot: Brad Fitzpatrick <[email protected]> |
|
29 --- |
|
30 src/compress/flate/deflate.go | 2 +- |
|
31 src/compress/flate/deflate_test.go | 50 ++++++++++++++++++++++++++++++++ |
|
32 src/compress/flate/huffman_bit_writer.go | 40 ++++++++++++++++--------- |
|
33 3 files changed, 78 insertions(+), 14 deletions(-) |
|
34 |
|
35 diff --git a/src/compress/flate/deflate.go b/src/compress/flate/deflate.go |
|
36 index 3e4dc7b..9f53d51 100644 |
|
37 --- a/src/compress/flate/deflate.go |
|
38 +++ b/src/compress/flate/deflate.go |
|
39 @@ -724,7 +724,7 @@ func (w *Writer) Close() error { |
|
40 // the result of NewWriter or NewWriterDict called with dst |
|
41 // and w's level and dictionary. |
|
42 func (w *Writer) Reset(dst io.Writer) { |
|
43 - if dw, ok := w.d.w.w.(*dictWriter); ok { |
|
44 + if dw, ok := w.d.w.writer.(*dictWriter); ok { |
|
45 // w was created with NewWriterDict |
|
46 dw.w = dst |
|
47 w.d.reset(dw) |
|
48 diff --git a/src/compress/flate/deflate_test.go b/src/compress/flate/deflate_test.go |
|
49 index 27a3b38..3322c40 100644 |
|
50 --- a/src/compress/flate/deflate_test.go |
|
51 +++ b/src/compress/flate/deflate_test.go |
|
52 @@ -6,6 +6,7 @@ package flate |
|
53 |
|
54 import ( |
|
55 "bytes" |
|
56 + "errors" |
|
57 "fmt" |
|
58 "internal/testenv" |
|
59 "io" |
|
60 @@ -631,3 +632,52 @@ func TestBestSpeed(t *testing.T) { |
|
61 } |
|
62 } |
|
63 } |
|
64 + |
|
65 +var errIO = errors.New("IO error") |
|
66 + |
|
67 +// failWriter fails with errIO exactly at the nth call to Write. |
|
68 +type failWriter struct{ n int } |
|
69 + |
|
70 +func (w *failWriter) Write(b []byte) (int, error) { |
|
71 + w.n-- |
|
72 + if w.n == -1 { |
|
73 + return 0, errIO |
|
74 + } |
|
75 + return len(b), nil |
|
76 +} |
|
77 + |
|
78 +func TestWriterPersistentError(t *testing.T) { |
|
79 + d, err := ioutil.ReadFile("../testdata/Mark.Twain-Tom.Sawyer.txt") |
|
80 + if err != nil { |
|
81 + t.Fatalf("ReadFile: %v", err) |
|
82 + } |
|
83 + d = d[:10000] // Keep this test short |
|
84 + |
|
85 + zw, err := NewWriter(nil, DefaultCompression) |
|
86 + if err != nil { |
|
87 + t.Fatalf("NewWriter: %v", err) |
|
88 + } |
|
89 + |
|
90 + // Sweep over the threshold at which an error is returned. |
|
91 + // The variable i makes it such that the ith call to failWriter.Write will |
|
92 + // return errIO. Since failWriter errors are not persistent, we must ensure |
|
93 + // that flate.Writer errors are persistent. |
|
94 + for i := 0; i < 1000; i++ { |
|
95 + fw := &failWriter{i} |
|
96 + zw.Reset(fw) |
|
97 + |
|
98 + _, werr := zw.Write(d) |
|
99 + cerr := zw.Close() |
|
100 + if werr != errIO && werr != nil { |
|
101 + t.Errorf("test %d, mismatching Write error: got %v, want %v", i, werr, errIO) |
|
102 + } |
|
103 + if cerr != errIO && fw.n < 0 { |
|
104 + t.Errorf("test %d, mismatching Close error: got %v, want %v", i, cerr, errIO) |
|
105 + } |
|
106 + if fw.n >= 0 { |
|
107 + // At this point, the failure threshold was sufficiently high enough |
|
108 + // that we wrote the whole stream without any errors. |
|
109 + return |
|
110 + } |
|
111 + } |
|
112 +} |
|
113 diff --git a/src/compress/flate/huffman_bit_writer.go b/src/compress/flate/huffman_bit_writer.go |
|
114 index c4adef9..d8b5a3e 100644 |
|
115 --- a/src/compress/flate/huffman_bit_writer.go |
|
116 +++ b/src/compress/flate/huffman_bit_writer.go |
|
117 @@ -77,7 +77,11 @@ var offsetBase = []uint32{ |
|
118 var codegenOrder = []uint32{16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15} |
|
119 |
|
120 type huffmanBitWriter struct { |
|
121 - w io.Writer |
|
122 + // writer is the underlying writer. |
|
123 + // Do not use it directly; use the write method, which ensures |
|
124 + // that Write errors are sticky. |
|
125 + writer io.Writer |
|
126 + |
|
127 // Data waiting to be written is bytes[0:nbytes] |
|
128 // and then the low nbits of bits. |
|
129 bits uint64 |
|
130 @@ -96,7 +100,7 @@ type huffmanBitWriter struct { |
|
131 |
|
132 func newHuffmanBitWriter(w io.Writer) *huffmanBitWriter { |
|
133 return &huffmanBitWriter{ |
|
134 - w: w, |
|
135 + writer: w, |
|
136 literalFreq: make([]int32, maxNumLit), |
|
137 offsetFreq: make([]int32, offsetCodeCount), |
|
138 codegen: make([]uint8, maxNumLit+offsetCodeCount+1), |
|
139 @@ -107,7 +111,7 @@ func newHuffmanBitWriter(w io.Writer) *huffmanBitWriter { |
|
140 } |
|
141 |
|
142 func (w *huffmanBitWriter) reset(writer io.Writer) { |
|
143 - w.w = writer |
|
144 + w.writer = writer |
|
145 w.bits, w.nbits, w.nbytes, w.err = 0, 0, 0, nil |
|
146 w.bytes = [bufferSize]byte{} |
|
147 } |
|
148 @@ -129,11 +133,21 @@ func (w *huffmanBitWriter) flush() { |
|
149 n++ |
|
150 } |
|
151 w.bits = 0 |
|
152 - _, w.err = w.w.Write(w.bytes[:n]) |
|
153 + w.write(w.bytes[:n]) |
|
154 w.nbytes = 0 |
|
155 } |
|
156 |
|
157 +func (w *huffmanBitWriter) write(b []byte) { |
|
158 + if w.err != nil { |
|
159 + return |
|
160 + } |
|
161 + _, w.err = w.writer.Write(b) |
|
162 +} |
|
163 + |
|
164 func (w *huffmanBitWriter) writeBits(b int32, nb uint) { |
|
165 + if w.err != nil { |
|
166 + return |
|
167 + } |
|
168 w.bits |= uint64(b) << w.nbits |
|
169 w.nbits += nb |
|
170 if w.nbits >= 48 { |
|
171 @@ -150,7 +164,7 @@ func (w *huffmanBitWriter) writeBits(b int32, nb uint) { |
|
172 bytes[5] = byte(bits >> 40) |
|
173 n += 6 |
|
174 if n >= bufferFlushSize { |
|
175 - _, w.err = w.w.Write(w.bytes[:n]) |
|
176 + w.write(w.bytes[:n]) |
|
177 n = 0 |
|
178 } |
|
179 w.nbytes = n |
|
180 @@ -173,13 +187,10 @@ func (w *huffmanBitWriter) writeBytes(bytes []byte) { |
|
181 n++ |
|
182 } |
|
183 if n != 0 { |
|
184 - _, w.err = w.w.Write(w.bytes[:n]) |
|
185 - if w.err != nil { |
|
186 - return |
|
187 - } |
|
188 + w.write(w.bytes[:n]) |
|
189 } |
|
190 w.nbytes = 0 |
|
191 - _, w.err = w.w.Write(bytes) |
|
192 + w.write(bytes) |
|
193 } |
|
194 |
|
195 // RFC 1951 3.2.7 specifies a special run-length encoding for specifying |
|
196 @@ -341,7 +352,7 @@ func (w *huffmanBitWriter) writeCode(c hcode) { |
|
197 bytes[5] = byte(bits >> 40) |
|
198 n += 6 |
|
199 if n >= bufferFlushSize { |
|
200 - _, w.err = w.w.Write(w.bytes[:n]) |
|
201 + w.write(w.bytes[:n]) |
|
202 n = 0 |
|
203 } |
|
204 w.nbytes = n |
|
205 @@ -572,6 +583,9 @@ func (w *huffmanBitWriter) indexTokens(tokens []token) (numLiterals, numOffsets |
|
206 // writeTokens writes a slice of tokens to the output. |
|
207 // codes for literal and offset encoding must be supplied. |
|
208 func (w *huffmanBitWriter) writeTokens(tokens []token, leCodes, oeCodes []hcode) { |
|
209 + if w.err != nil { |
|
210 + return |
|
211 + } |
|
212 for _, t := range tokens { |
|
213 if t < matchType { |
|
214 w.writeCode(leCodes[t.literal()]) |
|
215 @@ -676,9 +690,9 @@ func (w *huffmanBitWriter) writeBlockHuff(eof bool, input []byte) { |
|
216 if n < bufferFlushSize { |
|
217 continue |
|
218 } |
|
219 - _, w.err = w.w.Write(w.bytes[:n]) |
|
220 + w.write(w.bytes[:n]) |
|
221 if w.err != nil { |
|
222 - return |
|
223 + return // Return early in the event of write failures |
|
224 } |
|
225 n = 0 |
|
226 } |
|
227 -- |
|
228 2.7.4 |
|
229 |