file: Appending new segments

Adds the ability to append new segments right before the SOS segment.
Uses xio.Splice to handle that actual file manipulation which works on a
temp copy of the file to avoid corrupting the original before moving it
back.

Author: neilpa

file.go

@@ -1,9 +1,13 @@
 package jfif
 
 import (
+	"bytes"
 	"errors"
 	"io"
+	"math"
 	"os"
+
+	xio "neilpa.me/go-x/io"
 )
 
 var (
@@ -13,24 +17,84 @@ var (
 	// ErrOversizePayload means there's not enough space to update
 	// the segment data in-place.
 	ErrOversizePayload = errors.New("Oversize payload")
+
+	// ErrOversizeSegment means segment data was to large to append.
+	ErrOversizeSegment = errors.New("Oversize segment")
 )
 
+// Patch is used to insert new JFIF segments just before the SOS segment.
+type Patch struct { // TODO better name or use segments directly ignoring offset
+	// Marker is the type of segment
+	Marker Marker
+	// Data are the segment bytes that will be appended. Max size is 0xFFFF-2
+	Data []byte
+}
+
+// Append new JFIF segments to the file at path.
+//
+// Notes:
+//	- Under the hood this creates a temp-copy of the original file so
+//		that it can safely insert the new segments in the middle of the
+//		file. This avoids potential for corrupting data if an error is
+//		hit in the middle of the update. At the end the original path
+//		is replaced with a single os.Rename operation.
+//
+// TODO: Higher-level version of this that could be smarter for XMP data
+// TODO: Return the updated pointer data?
+func Append(path string, patches ...Patch) error {
+	// Prep the buffer for writing
+	var buf bytes.Buffer
+	for _, p := range patches {
+		seg := Segment{}
+		seg.Marker = p.Marker
+		seg.Offset = -1
+
+		l := len(seg.Data) + 2
+		if l > math.MaxUint16 {
+			return ErrOversizeSegment
+		}
+		seg.Length = uint16(l) // TODO not right for oversize segments
+		// TODO: what about an embedded Data where the first two bytes are the length
+		seg.Data = p.Data
+
+		// TODO Would be nice to avoid yet-another-copy of data and plumb
+		// through a custom reader that calculated the size
+		if err := EncodeSegment(&buf, seg); err != nil {
+			return err
+		}
+	}
+
+	f, err := os.Open(path)
+	ptrs, err := ScanSegments(f)
+	if err != nil {
+		return err
+	}
+	last := ptrs[len(ptrs)-1]
+
+	return xio.SpliceFile(f, buf.Bytes(), last.Offset)
+}
+
 // File is used to perform in-place updates to JFIF segments to a backing
 // file on-disk.
 type File struct {
 	// f is the underlying file on disk.
 	f *os.File
 
-	// refs are the intially scanned segment pointers.
-	refs []SegmentP
+	// refs are the minimally scanned segment pointers.
+	refs []Pointer
 }
 
 // Edit opens and scans segments from a JFIF file. This should be
 // used to replace segments in-place without having to re-write the
 // full file. Note that this will fail on attempts to write segments
-// that would expend beyond the current bounds. Otherise, "short-segments"
-// retain the desired size but there are 0xFF fill bytes used for padding
-// until the next segment.
+// that would expand beyond the current bounds.
+//
+// TODO: Otherise, "short-segments" retain the desired size but there
+// are 0xFF fill bytes used for padding until the next segment.
+//
+// TODO: This may not be all that valuable verse doing a proper splice.
+// in a copied version of the file and replacing over top of it. This
+// can lead to file corruption if not careful...
 func Edit(path string) (*File, error) {
 	f, err := os.OpenFile(path, os.O_RDWR, 0)
 	if err != nil {
@@ -57,8 +121,8 @@ func (f *File) Close() (err error) {
 }
 
 // Query finds existing segments that matches the given marker
-func (f *File) Query(m Marker) ([]SegmentP, error) {
-	refs := make([]SegmentP, 0)
+func (f *File) Query(m Marker) ([]Pointer, error) {
+	refs := make([]Pointer, 0)
 	for _, r := range f.refs {
 		if r.Marker == m {
 			refs = append(refs, r)
@@ -67,10 +131,68 @@ func (f *File) Query(m Marker) ([]SegmentP, error) {
 	return refs, nil
 }
 
+// Add new segments at the end of the JFIF header, before
+// the first SOI marker.
+//
+// Notes:
+//	* This is an expensive operation. It requires shifting all of
+//		image bytes on disk to make space.
+//	* Offset and Length are ignored on the incoming segments. They
+//		are simply calcluted from the provided Data in order.
+//
+// TODO What about a multi-segment that could have splitting/chunking behavior?
+//
+// TODO: This interface doesn't quite work since we need to close the file
+//	descriptor and do the move...
+func (f *File) Add(segs ...Segment) error {
+	// TODO Probably want an xio primitive to insert-in-middle operation
+	if len(f.refs) < 3 {
+		return errors.New("todo: Not enough file segments to start")
+	}
+
+	last := f.refs[len(f.refs)-1]
+
+	// Calculate how much extra space we need.
+	size := int64(0)
+	insert := last.Offset
+	for i, s := range segs {
+		s.Offset = insert + size
+		if len(s.Data) > 0 {
+			l := len(s.Data) + 2
+			if l > 0xffff { // TODO double-check actual max segment size
+				return ErrOversizeSegment
+			}
+			s.Length = uint16(l)
+		} else {
+			s.Length = 0
+		}
+		size += s.DiskSize()
+		segs[i] = s
+	}
+
+	// TODO Apply bookkeeping for the `last` SOS marker offset
+
+	return nil
+}
+
+// Sync writes any updated contents of the segment back to disk
+//
+// TODO: Note that at some point this may "do the right thing" when
+// further downstream allocations need ot happen.
+func (f *File) Sync(s Segment) error {
+	return f.Update(s.Pointer, s.Data)
+}
+
 // Update replaces the payload for the given segment ref. Returns an
 // error if it's too large or doesn't match a known segment in this
 // file.
-func (f *File) Update(r SegmentP, buf []byte) error {
+//
+// Note:
+//	- This updates the file in-place so all of the general warnings
+//		apply w.r.t. potential file corruption. This should be limited
+//		to files that have already been copied and are intended to
+//		be edited directly.
+func (f *File) Update(r Pointer, buf []byte) error {
 	var i int
 	for ; i < len(f.refs); i++ {
 		if f.refs[i] == r {
@@ -92,14 +214,15 @@ func (f *File) Update(r SegmentP, buf []byte) error {
 	}
 
 	// Encode the updated segment to disk
-	// TODO Need to make sure to update our SegmentP copy
+	// TODO Need to make sure to update our Pointer copy
 	_, err := f.f.Seek(r.Offset, io.SeekStart)
 	if err != nil {
 		return err
 	}
 
 	seg := Segment{ // TODO Can I avoid all the "+/- 2's" everywhere
-		SegmentP{r.Offset, r.Marker, uint16(len(buf) + 2)}, buf,
+		Pointer{r.Offset, r.Marker, uint16(len(buf) + 2)},
+		buf,
 	}
 	err = EncodeSegment(f.f, seg)
 	if err != nil {
@@ -110,6 +233,6 @@ func (f *File) Update(r SegmentP, buf []byte) error {
 	}
 
 	// Update our in-memory location
-	f.refs[i] = seg.SegmentP
+	f.refs[i] = seg.Pointer
 	return nil
 }

file_test.go

@@ -2,6 +2,7 @@ package jfif
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
@@ -15,14 +16,14 @@ func TestFileQuery(t *testing.T) { // TODO
 func TestFileUpdate(t *testing.T) {
 	var tests = []struct {
 		name string
-		ref  SegmentP
+		ref  Pointer
 		buf  []byte
 
 		golden string
 	}{
 		{
 			"min",
-			SegmentP{Offset: 2, Marker: DQT, Length: 67},
+			Pointer{Offset: 2, Marker: DQT, Length: 67},
 			[]byte{0, // Pq and Tq bytes
 				// Arbitrary DQT table for testing
 				16, 11, 10, 16, 24, 40, 51, 61,
@@ -86,3 +87,44 @@ func TestFileUpdate(t *testing.T) {
 		})
 	}
 }
+
+func TestFileAdd(t *testing.T) {
+
+	var tests = []struct {
+		name string
+		ref  Pointer
+		buf  []byte
+
+		golden string
+	}{
+		{
+			"min",
+			Pointer{Offset: 2, Marker: DQT, Length: 67},
+			[]byte{0, // Pq and Tq bytes
+				// Arbitrary DQT table for testing
+				16, 11, 10, 16, 24, 40, 51, 61,
+				12, 12, 14, 19, 26, 58, 60, 55,
+				14, 13, 16, 24, 40, 57, 69, 56,
+				14, 17, 22, 29, 51, 87, 80, 62,
+				18, 22, 37, 56, 68, 109, 103, 77,
+				24, 35, 55, 64, 81, 104, 113, 92,
+				49, 64, 78, 87, 103, 121, 120, 101,
+				72, 92, 95, 98, 112, 100, 103, 99,
+			},
+			"min.dqt.jpg",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			temp, err := ioutil.TempFile(os.TempDir(), "jfif-test-add-"+tt.name)
+			if err != nil {
+				t.Fatal(err)
+			}
+			path := temp.Name()
+			//defer os.Remove(path)
+			defer temp.Close()
+
+			fmt.Println("TODO: TestFileAdd:", path)
+		})
+	}
+}

go.mod

@@ -2,4 +2,4 @@ module neilpa.me/go-jfif
 
 go 1.14
 
-require neilpa.me/go-x v0.2.0
+require neilpa.me/go-x v0.2.1-0.20200507232743-5243b9624d5e

go.sum

@@ -1,2 +1,4 @@
 neilpa.me/go-x v0.2.0 h1:GbLRDtAZ9MgVrwrIe3jWnHF2W40LCFA9Ng/aDbd9GVs=
 neilpa.me/go-x v0.2.0/go.mod h1:aIemU+pQYLLV3dygXotHKF7SantXe5HzZR6VIjzY/4g=
+neilpa.me/go-x v0.2.1-0.20200507232743-5243b9624d5e h1:+O171A50t9HKAT8lpmTeTVscpTNYUvwDlPvOAOpgq/E=
+neilpa.me/go-x v0.2.1-0.20200507232743-5243b9624d5e/go.mod h1:aIemU+pQYLLV3dygXotHKF7SantXe5HzZR6VIjzY/4g=