From a5999b7b81b9dc875cc635e1b089d768ddd41a8c Mon Sep 17 00:00:00 2001
From: George Gkirtsou <ggirtsou@gmail.com>
Date: Sun, 9 Apr 2017 21:12:39 +0100
Subject: [PATCH] os: more descriptive error for File.ReadAt and File.WriteAt
 with negative offset.

The existing implementation does not provide a useful error message
if a negative offset is passed in File.ReadAt or File.WriteAt. This
change is to return descriptive errors. An error of type *PathError
is returned to keep it consistent with rest of the code.

There is no need to add an exported error variable since it's used only
in one file.

Fixes #19031

Change-Id: Ib94cab0afae8c5fe4dd97ed2887018a09b9f4538
Reviewed-on: https://go-review.googlesource.com/39136
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
---
 src/os/file.go    | 11 +++++++++++
 src/os/os_test.go | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/src/os/file.go b/src/os/file.go
index 047217881f..d61124b338 100644
--- a/src/os/file.go
+++ b/src/os/file.go
@@ -37,6 +37,7 @@
 package os
 
 import (
+	"errors"
 	"io"
 	"syscall"
 )
@@ -117,6 +118,11 @@ func (f *File) ReadAt(b []byte, off int64) (n int, err error) {
 	if err := f.checkValid("read"); err != nil {
 		return 0, err
 	}
+
+	if off < 0 {
+		return 0, &PathError{"readat", f.name, errors.New("negative offset")}
+	}
+
 	for len(b) > 0 {
 		m, e := f.pread(b, off)
 		if e != nil {
@@ -164,6 +170,11 @@ func (f *File) WriteAt(b []byte, off int64) (n int, err error) {
 	if err := f.checkValid("write"); err != nil {
 		return 0, err
 	}
+
+	if off < 0 {
+		return 0, &PathError{"writeat", f.name, errors.New("negative offset")}
+	}
+
 	for len(b) > 0 {
 		m, e := f.pwrite(b, off)
 		if e != nil {
diff --git a/src/os/os_test.go b/src/os/os_test.go
index ee9e0cf767..fcfcc43620 100644
--- a/src/os/os_test.go
+++ b/src/os/os_test.go
@@ -1451,6 +1451,26 @@ func TestReadAtOffset(t *testing.T) {
 	}
 }
 
+// Verify that ReadAt doesn't allow negative offset.
+func TestReadAtNegativeOffset(t *testing.T) {
+	f := newFile("TestReadAtNegativeOffset", t)
+	defer Remove(f.Name())
+	defer f.Close()
+
+	const data = "hello, world\n"
+	io.WriteString(f, data)
+
+	f.Seek(0, 0)
+	b := make([]byte, 5)
+
+	n, err := f.ReadAt(b, -10)
+
+	const wantsub = "negative offset"
+	if !strings.Contains(fmt.Sprint(err), wantsub) || n != 0 {
+		t.Errorf("ReadAt(-10) = %v, %v; want 0, ...%q...", n, err, wantsub)
+	}
+}
+
 func TestWriteAt(t *testing.T) {
 	f := newFile("TestWriteAt", t)
 	defer Remove(f.Name())
@@ -1473,6 +1493,20 @@ func TestWriteAt(t *testing.T) {
 	}
 }
 
+// Verify that WriteAt doesn't allow negative offset.
+func TestWriteAtNegativeOffset(t *testing.T) {
+	f := newFile("TestWriteAtNegativeOffset", t)
+	defer Remove(f.Name())
+	defer f.Close()
+
+	n, err := f.WriteAt([]byte("WORLD"), -10)
+
+	const wantsub = "negative offset"
+	if !strings.Contains(fmt.Sprint(err), wantsub) || n != 0 {
+		t.Errorf("WriteAt(-10) = %v, %v; want 0, ...%q...", n, err, wantsub)
+	}
+}
+
 func writeFile(t *testing.T, fname string, flag int, text string) string {
 	f, err := OpenFile(fname, flag, 0666)
 	if err != nil {
-- 
2.48.1