diff --git a/modules/brother/cmd/brother/escl.go b/modules/brother/cmd/brother/escl.go index ff55f407..1f79c348 100644 --- a/modules/brother/cmd/brother/escl.go +++ b/modules/brother/cmd/brother/escl.go @@ -150,9 +150,13 @@ func (c *Client) Scan(ctx context.Context, resolution int, colorMode string) ([] c.deleteJob(job) return nil, err } - if !c.closeJob(ctx, doc) { + closed, extra := c.closeJob(ctx, doc) + if !closed { c.deleteJob(job) } + if extra > 0 { + return nil, fmt.Errorf("the scanner gave more pages than the glass holds: %d", 1+extra) + } return image, nil } @@ -250,31 +254,32 @@ func (c *Client) page(ctx context.Context, doc string) ([]byte, error) { } // maxClosing bounds the NextDocument requests that ask for the end of a job. -const maxClosing = 3 +const maxClosing = 1 // closeJob asks NextDocument until the scanner answers 404, which ends the job; true when it did (or the job is -// gone already). A further page is read and dropped, within the bound. -func (c *Client) closeJob(ctx context.Context, doc string) bool { +// gone already), and how many further pages it sent instead; any is an error for the caller, for a Platen job holds +// one page. It stops at the first further page, and the job is then deleted. +func (c *Client) closeJob(ctx context.Context, doc string) (closed bool, extra int) { ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 15*time.Second) defer cancel() for i := 0; i < maxClosing; i++ { resp, err := c.get(ctx, doc) if err != nil { c.logf("brother: closing the scan job: %v", err) - return false + return false, 0 } readBody(resp, maxImage) switch resp.StatusCode { case http.StatusNotFound, http.StatusGone: - return true + return true, 0 case http.StatusOK: - continue + c.logf("brother: the scanner sent a further page after the glass's one") + return false, 1 } c.logf("brother: closing the scan job: NextDocument answered %s", resp.Status) - return false + return false, 0 } - c.logf("brother: the scanner kept sending pages after %d requests", maxClosing) - return false + return false, 0 } // deleteJob deletes the job, best effort; true when the job is gone. A job that cannot be deleted is remembered, so diff --git a/modules/brother/cmd/brother/escl_test.go b/modules/brother/cmd/brother/escl_test.go index 215fb17e..6418fb9c 100644 --- a/modules/brother/cmd/brother/escl_test.go +++ b/modules/brother/cmd/brother/escl_test.go @@ -203,3 +203,16 @@ func TestABusyScannerWithABrotherJobLeftIsRetriedOnceAfterItCloses(t *testing.T) t.Fatalf("%d posts, want 3 (stuck job, refused, retried)", f.posts) } } + +// 1531: a second page for a Platen job is an error, never a silently incomplete scan, and the job is closed. +func TestMorePagesThanTheGlassHoldsIsAnErrorAndTheJobIsClosed(t *testing.T) { + f := newFakeScanner(t, fixture(t, "scan.jpg")) + f.pages = 2 + image, err := NewClient(f.base()).Scan(context.Background(), 300, "color") + if err == nil || !strings.Contains(err.Error(), "the scanner gave more pages than the glass holds") { + t.Fatalf("image %d bytes, error = %v, want more pages than the glass holds", len(image), err) + } + if f.open || f.deletes != 1 { + t.Fatalf("job open %v, %d DELETEs, want closed by 1 DELETE", f.open, f.deletes) + } +} diff --git a/modules/brother/cmd/brother/helpers_test.go b/modules/brother/cmd/brother/helpers_test.go index 16e29443..1869a718 100644 --- a/modules/brother/cmd/brother/helpers_test.go +++ b/modules/brother/cmd/brother/helpers_test.go @@ -33,6 +33,8 @@ type fakeScanner struct { // or the job is DELETEd; the fake does the same. open bool delivered bool + pages int // pages a job holds beyond the first + served int deletes int gone bool // the first NextDocument answers 410 (the job was cancelled at the device) dropConn bool // the first NextDocument loses its answer @@ -70,7 +72,7 @@ func newFakeScanner(t *testing.T, image []byte) *fakeScanner { http.Error(w, "busy", http.StatusServiceUnavailable) return } - f.open, f.delivered = true, false + f.open, f.delivered, f.served = true, false, 0 // The captured Location names the real scanner's host; the test's own server stands in for it. w.Header().Set("Location", f.URL+"/eSCL/ScanJobs/7320ead5") w.WriteHeader(http.StatusCreated) @@ -95,12 +97,13 @@ func newFakeScanner(t *testing.T, image []byte) *fakeScanner { } return } - if f.delivered || f.noPage { + if f.noPage || (f.delivered && f.served > f.pages) { f.open = false http.NotFound(w, r) return } f.delivered = true + f.served++ w.Header().Set("Content-Type", "image/jpeg") w.Write(f.image) })