Skip to content

Commit b600499

Browse files
1-ashraful-islammostynb
1-ashraful-islam
authored and
mostynb
committed
Reject oversized blob uploads earlier
Implements #875.
1 parent 86599fe commit b600499

File tree

5 files changed

+35
-15
lines changed

5 files changed

+35
-15
lines changed

‎cache/grpcproxy/grpcproxy_test.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"encoding/hex"
88
"fmt"
99
"io"
10+
"math"
1011
"net"
1112
"os"
1213
"path/filepath"
@@ -76,8 +77,7 @@ func newProxy(t *testing.T, dir string, storageMode string) *testProxy {
7677
if err != nil {
7778
t.Fatal(err)
7879
}
79-
proxy := New(clients, storageMode, logger, logger, 100, 100)
80-
p.proxy = proxy
80+
p.proxy = New(clients, storageMode, logger, logger, 100, 100)
8181

8282
return p
8383
}
@@ -223,12 +223,13 @@ func newFixture(t *testing.T, proxy cache.Proxy, storageMode string) *fixture {
223223
8*1024*1024,
224224
disk.WithProxyBackend(proxy),
225225
disk.WithStorageMode(storageMode),
226-
disk.WithAccessLogger(logger))
226+
disk.WithAccessLogger(logger),
227+
disk.WithMaxBlobSize(math.MaxInt64))
227228
if err != nil {
228229
t.Fatal(err)
229230
}
230231

231-
unlimitedMaxCasBlobSize := int64(0)
232+
unlimitedMaxCasBlobSize := int64(math.MaxInt64)
232233

233234
grpcServer := grpc.NewServer()
234235

‎main.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,8 @@ func startHttpServer(c *config.Config, httpServer **http.Server,
249249
checkClientCertForWrites := c.TLSCaFile != ""
250250
validateAC := !c.DisableHTTPACValidation
251251
h := server.NewHTTPCache(diskCache, c.AccessLogger, c.ErrorLogger, validateAC,
252-
c.EnableACKeyInstanceMangling, checkClientCertForReads, checkClientCertForWrites, gitCommit, gitTags)
252+
c.EnableACKeyInstanceMangling, checkClientCertForReads, checkClientCertForWrites, gitCommit, gitTags,
253+
c.MaxBlobSize)
253254

254255
cacheHandler := h.CacheHandler
255256
var ldapAuthenticator authenticator

‎server/grpc_bytestream.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,13 @@ func (s *grpcServer) Write(srv bytestream.ByteStream_WriteServer) error {
408408
return
409409
}
410410

411+
if size > s.maxCasBlobSizeBytes {
412+
recvResult <- status.Errorf(codes.InvalidArgument,
413+
"Blob size %d exceeds maximum allowed size %d",
414+
size, s.maxCasBlobSizeBytes)
415+
return
416+
}
417+
411418
exists, _ := s.cache.Contains(srv.Context(), cache.CAS, hash, size)
412419
if exists {
413420
// Blob already exists, return without writing anything.

‎server/http.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ type httpCache struct {
4848
gitTags string
4949
checkClientCertForReads bool
5050
checkClientCertForWrites bool
51+
maxCasBlobSizeBytes int64
5152
}
5253

5354
type statusPageData struct {
@@ -66,7 +67,7 @@ type statusPageData struct {
6667
// accessLogger will print one line for each HTTP request to stdout.
6768
// errorLogger will print unexpected server errors. Inexistent files and malformed URLs will not
6869
// be reported.
69-
func NewHTTPCache(cache disk.Cache, accessLogger cache.Logger, errorLogger cache.Logger, validateAC bool, mangleACKeys bool, checkClientCertForReads bool, checkClientCertForWrites bool, commit string, gitTags string) HTTPCache {
70+
func NewHTTPCache(cache disk.Cache, accessLogger cache.Logger, errorLogger cache.Logger, validateAC bool, mangleACKeys bool, checkClientCertForReads bool, checkClientCertForWrites bool, commit string, gitTags string, maxCasBlobSizeBytes int64) HTTPCache {
7071

7172
_, _, numItems, _ := cache.Stats()
7273

@@ -80,6 +81,7 @@ func NewHTTPCache(cache disk.Cache, accessLogger cache.Logger, errorLogger cache
8081
mangleACKeys: mangleACKeys,
8182
checkClientCertForReads: checkClientCertForReads,
8283
checkClientCertForWrites: checkClientCertForWrites,
84+
maxCasBlobSizeBytes: maxCasBlobSizeBytes,
8385
}
8486

8587
if commit != "{STABLE_GIT_COMMIT}" {
@@ -321,6 +323,14 @@ func (h *httpCache) CacheHandler(w http.ResponseWriter, r *http.Request) {
321323
return
322324
}
323325

326+
if contentLength > h.maxCasBlobSizeBytes {
327+
msg := fmt.Sprintf("Blob size %d exceeds maximum allowed size %d",
328+
contentLength, h.maxCasBlobSizeBytes)
329+
http.Error(w, msg, http.StatusBadRequest)
330+
h.errorLogger.Printf("PUT %s: %s", path(kind, hash), msg)
331+
return
332+
}
333+
324334
zstdCompressed := false
325335

326336
// Content-Encoding must be one of "identity", "zstd" or not present.

‎server/http_test.go‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"encoding/json"
99
"fmt"
1010
"io"
11+
"math"
1112
"net/http"
1213
"net/http/httptest"
1314
"net/url"
@@ -38,7 +39,7 @@ func TestDownloadFile(t *testing.T) {
3839
if err != nil {
3940
t.Fatal(err)
4041
}
41-
h := NewHTTPCache(c, testutils.NewSilentLogger(), testutils.NewSilentLogger(), true, false, false, false, "", "")
42+
h := NewHTTPCache(c, testutils.NewSilentLogger(), testutils.NewSilentLogger(), true, false, false, false, "", "", math.MaxInt64)
4243

4344
rr := httptest.NewRecorder()
4445
handler := http.HandlerFunc(h.CacheHandler)
@@ -106,7 +107,7 @@ func TestUploadFilesConcurrently(t *testing.T) {
106107
if err != nil {
107108
t.Fatal(err)
108109
}
109-
h := NewHTTPCache(c, testutils.NewSilentLogger(), testutils.NewSilentLogger(), true, false, false, false, "", "")
110+
h := NewHTTPCache(c, testutils.NewSilentLogger(), testutils.NewSilentLogger(), true, false, false, false, "", "", math.MaxInt64)
110111
handler := http.HandlerFunc(h.CacheHandler)
111112

112113
var wg sync.WaitGroup
@@ -170,7 +171,7 @@ func TestUploadSameFileConcurrently(t *testing.T) {
170171
if err != nil {
171172
t.Fatal(err)
172173
}
173-
h := NewHTTPCache(c, testutils.NewSilentLogger(), testutils.NewSilentLogger(), true, false, false, false, "", "")
174+
h := NewHTTPCache(c, testutils.NewSilentLogger(), testutils.NewSilentLogger(), true, false, false, false, "", "", math.MaxInt64)
174175
handler := http.HandlerFunc(h.CacheHandler)
175176

176177
var wg sync.WaitGroup
@@ -211,7 +212,7 @@ func TestUploadCorruptedFile(t *testing.T) {
211212
if err != nil {
212213
t.Fatal(err)
213214
}
214-
h := NewHTTPCache(c, testutils.NewSilentLogger(), testutils.NewSilentLogger(), true, false, false, false, "", "")
215+
h := NewHTTPCache(c, testutils.NewSilentLogger(), testutils.NewSilentLogger(), true, false, false, false, "", "", math.MaxInt64)
215216
rr := httptest.NewRecorder()
216217
handler := http.HandlerFunc(h.CacheHandler)
217218
handler.ServeHTTP(rr, r)
@@ -255,7 +256,7 @@ func TestUploadEmptyActionResult(t *testing.T) {
255256
mangle := false
256257
checkClientCertForReads := false
257258
checkClientCertForWrites := false
258-
h := NewHTTPCache(c, testutils.NewSilentLogger(), testutils.NewSilentLogger(), validate, mangle, checkClientCertForReads, checkClientCertForWrites, "", "")
259+
h := NewHTTPCache(c, testutils.NewSilentLogger(), testutils.NewSilentLogger(), validate, mangle, checkClientCertForReads, checkClientCertForWrites, "", "", math.MaxInt64)
259260
rr := httptest.NewRecorder()
260261
handler := http.HandlerFunc(h.CacheHandler)
261262
handler.ServeHTTP(rr, r)
@@ -317,7 +318,7 @@ func testEmptyBlobAvailable(t *testing.T, method string) {
317318
mangle := false
318319
checkClientCertForReads := false
319320
checkClientCertForWrites := false
320-
h := NewHTTPCache(c, testutils.NewSilentLogger(), testutils.NewSilentLogger(), validate, mangle, checkClientCertForReads, checkClientCertForWrites, "", "")
321+
h := NewHTTPCache(c, testutils.NewSilentLogger(), testutils.NewSilentLogger(), validate, mangle, checkClientCertForReads, checkClientCertForWrites, "", "", math.MaxInt64)
321322
rr := httptest.NewRecorder()
322323
handler := http.HandlerFunc(h.CacheHandler)
323324
handler.ServeHTTP(rr, r)
@@ -340,7 +341,7 @@ func TestStatusPage(t *testing.T) {
340341
if err != nil {
341342
t.Fatal(err)
342343
}
343-
h := NewHTTPCache(c, testutils.NewSilentLogger(), testutils.NewSilentLogger(), true, false, false, false, "", "")
344+
h := NewHTTPCache(c, testutils.NewSilentLogger(), testutils.NewSilentLogger(), true, false, false, false, "", "", math.MaxInt64)
344345
rr := httptest.NewRecorder()
345346
handler := http.HandlerFunc(h.StatusPageHandler)
346347
handler.ServeHTTP(rr, r)
@@ -484,7 +485,7 @@ func TestRemoteReturnsNotFound(t *testing.T) {
484485
t.Fatal(err)
485486
}
486487

487-
h := NewHTTPCache(emptyCache, testutils.NewSilentLogger(), testutils.NewSilentLogger(), true, false, false, false, "", "")
488+
h := NewHTTPCache(emptyCache, testutils.NewSilentLogger(), testutils.NewSilentLogger(), true, false, false, false, "", "", math.MaxInt64)
488489
// create a fake http.Request
489490
_, hash := testutils.RandomDataAndHash(1024)
490491
url, _ := url.Parse(fmt.Sprintf("http://localhost:8080/ac/%s", hash))
@@ -522,7 +523,7 @@ func TestManglingACKeys(t *testing.T) {
522523
t.Fatal(err)
523524
}
524525

525-
h := NewHTTPCache(diskCache, testutils.NewSilentLogger(), testutils.NewSilentLogger(), false, true, false, false, "", "")
526+
h := NewHTTPCache(diskCache, testutils.NewSilentLogger(), testutils.NewSilentLogger(), false, true, false, false, "", "", math.MaxInt64)
526527
// create a fake http.Request
527528
data, hash := testutils.RandomDataAndHash(blobSize)
528529
err = diskCache.Put(context.Background(), cache.RAW, hash, int64(len(data)), bytes.NewReader(data))

0 commit comments

Comments
 (0)