From cf397436334426bf9c7baf5f566f2a322baf4c52 Mon Sep 17 00:00:00 2001 From: Quim Muntal Diaz Date: Thu, 7 Feb 2019 11:39:25 +0100 Subject: [PATCH] improve random number generator for writer --- relationship.go | 6 ++---- writer.go | 8 ++++--- writer_test.go | 57 +++++++++++++++++-------------------------------- 3 files changed, 27 insertions(+), 44 deletions(-) diff --git a/relationship.go b/relationship.go index 3575e35..030979c 100644 --- a/relationship.go +++ b/relationship.go @@ -6,7 +6,6 @@ import ( "math/rand" "net/url" "strings" - "time" ) // TargetMode is an enumerable for the different target modes. @@ -48,15 +47,14 @@ type relationshipXML struct { Mode string `xml:"TargetMode,attr,omitempty"` } -func (r *Relationship) ensureID() { +func (r *Relationship) ensureID(rnd *rand.Rand) { if r.ID != "" { return } b := make([]byte, 8) - rd := rand.New(rand.NewSource(time.Now().UnixNano())) for i := range b { - b[i] = charBytes[rd.Intn(len(charBytes))] + b[i] = charBytes[rnd.Intn(len(charBytes))] } r.ID = string(b) } diff --git a/writer.go b/writer.go index 355d4c2..04e5909 100644 --- a/writer.go +++ b/writer.go @@ -5,6 +5,7 @@ import ( "compress/flate" "fmt" "io" + "math/rand" "path/filepath" "time" ) @@ -32,11 +33,12 @@ type Writer struct { p *pkg w *zip.Writer last *Part + rnd *rand.Rand } // NewWriter returns a new Writer writing an OPC file to w. func NewWriter(w io.Writer) *Writer { - return &Writer{p: newPackage(), w: zip.NewWriter(w)} + return &Writer{p: newPackage(), w: zip.NewWriter(w), rnd: rand.New(rand.NewSource(time.Now().UnixNano()))} } // Flush flushes any buffered data to the underlying writer. @@ -123,7 +125,7 @@ func (w *Writer) createOwnRelationships() error { return nil } for _, r := range w.Relationships { - r.ensureID() + r.ensureID(w.rnd) } if err := validateRelationships("/", w.Relationships); err != nil { return err @@ -140,7 +142,7 @@ func (w *Writer) createLastPartRelationships() error { return nil } for _, r := range w.last.Relationships { - r.ensureID() + r.ensureID(w.rnd) } if err := validateRelationships(w.last.Name, w.last.Relationships); err != nil { return err diff --git a/writer_test.go b/writer_test.go index 6c8eff2..a62eb14 100644 --- a/writer_test.go +++ b/writer_test.go @@ -3,31 +3,10 @@ package opc import ( "archive/zip" "bytes" - "reflect" + "math/rand" "testing" ) -func TestNewWriter(t *testing.T) { - tests := []struct { - name string - want *Writer - wantW string - }{ - {"base", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{})}, ""}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - w := &bytes.Buffer{} - if got := NewWriter(w); !reflect.DeepEqual(got, tt.want) { - t.Errorf("NewWriter() = %v, want %v", got, tt.want) - } - if gotW := w.String(); gotW != tt.wantW { - t.Errorf("NewWriter() = %v, want %v", gotW, tt.wantW) - } - }) - } -} - func TestWriter_Flush(t *testing.T) { tests := []struct { name string @@ -45,6 +24,10 @@ func TestWriter_Flush(t *testing.T) { } } +func fakeRand() *rand.Rand { + return rand.New(rand.NewSource(42)) +} + func TestWriter_Close(t *testing.T) { p := newPackage() p.contentTypes.add("/a.xml", "a/b") @@ -61,15 +44,15 @@ func TestWriter_Close(t *testing.T) { wantErr bool }{ {"base", NewWriter(&bytes.Buffer{}), false}, - {"invalidContentType", &Writer{p: pC, w: zip.NewWriter(&bytes.Buffer{})}, true}, - {"withCt", &Writer{p: p, w: zip.NewWriter(&bytes.Buffer{})}, false}, - {"invalidPartRel", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), last: &Part{Name: "/b.xml", Relationships: []*Relationship{{}}}}, true}, - {"invalidOwnRel", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Relationships: []*Relationship{{}}}, true}, - {"withDuplicatedCoreProps", &Writer{p: pCore, w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}}, true}, - {"withDuplicatedRels", &Writer{p: pRel, w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}}, true}, - {"withDuplicatedRels", &Writer{p: pRel, w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}}, true}, - {"withCoreProps", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}}, false}, - {"withCorePropsWithName", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song", PartName: "/props.xml"}}, false}, + {"invalidContentType", &Writer{p: pC, w: zip.NewWriter(&bytes.Buffer{}), rnd: fakeRand()}, true}, + {"withCt", &Writer{p: p, w: zip.NewWriter(&bytes.Buffer{}), rnd: fakeRand()}, false}, + {"invalidPartRel", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), last: &Part{Name: "/b.xml", Relationships: []*Relationship{{}}}, rnd: fakeRand()}, true}, + {"invalidOwnRel", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Relationships: []*Relationship{{}}, rnd: fakeRand()}, true}, + {"withDuplicatedCoreProps", &Writer{p: pCore, w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}, rnd: fakeRand()}, true}, + {"withDuplicatedRels", &Writer{p: pRel, w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}, rnd: fakeRand()}, true}, + {"withDuplicatedRels", &Writer{p: pRel, w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}, rnd: fakeRand()}, true}, + {"withCoreProps", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song"}, rnd: fakeRand()}, false}, + {"withCorePropsWithName", &Writer{p: newPackage(), w: zip.NewWriter(&bytes.Buffer{}), Properties: CoreProperties{Title: "Song", PartName: "/props.xml"}, rnd: fakeRand()}, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -169,8 +152,8 @@ func TestWriter_CreatePart(t *testing.T) { }{ {"fhErr", NewWriter(&bytes.Buffer{}), args{&Part{"/a.xml", "a/b", nil}, -3}, true}, {"nameErr", NewWriter(&bytes.Buffer{}), args{&Part{"a.xml", "a/b", nil}, CompressionNone}, true}, - {"failRel", &Writer{w: zip.NewWriter(nil), last: &Part{Name: "/b.xml", Relationships: []*Relationship{{}}}}, args{&Part{"/a.xml", "a/b", nil}, CompressionNone}, true}, - {"failRel2", &Writer{p: pRel, w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{rel}}}, args{&Part{"/b.xml", "a/b", nil}, CompressionNone}, true}, + {"failRel", &Writer{w: zip.NewWriter(nil), last: &Part{Name: "/b.xml", Relationships: []*Relationship{{}}}, rnd: fakeRand()}, args{&Part{"/a.xml", "a/b", nil}, CompressionNone}, true}, + {"failRel2", &Writer{p: pRel, w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{rel}}, rnd: fakeRand()}, args{&Part{"/b.xml", "a/b", nil}, CompressionNone}, true}, {"base", w, args{&Part{"/a.xml", "a/b", nil}, CompressionNone}, false}, {"multipleDiffName", w, args{&Part{"/b.xml", "a/b", nil}, CompressionNone}, false}, {"multipleDiffContentType", w, args{&Part{"/c.xml", "c/d", nil}, CompressionNone}, false}, @@ -199,11 +182,11 @@ func TestWriter_createLastPartRelationships(t *testing.T) { w *Writer wantErr bool }{ - {"base", &Writer{p: newPackage(), w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{rel}}}, false}, - {"base2", &Writer{p: newPackage(), w: zip.NewWriter(nil), last: &Part{Name: "/b/a.xml", Relationships: []*Relationship{rel}}}, false}, + {"base", &Writer{p: newPackage(), w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{rel}}, rnd: fakeRand()}, false}, + {"base2", &Writer{p: newPackage(), w: zip.NewWriter(nil), last: &Part{Name: "/b/a.xml", Relationships: []*Relationship{rel}}, rnd: fakeRand()}, false}, {"hasSome", w, false}, - {"duplicated", &Writer{w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{rel, rel}}}, true}, - {"invalidRelation", &Writer{w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{{}}}}, true}, + {"duplicated", &Writer{w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{rel, rel}}, rnd: fakeRand()}, true}, + {"invalidRelation", &Writer{w: zip.NewWriter(nil), last: &Part{Name: "/a.xml", Relationships: []*Relationship{{}}}, rnd: fakeRand()}, true}, {"empty", NewWriter(&bytes.Buffer{}), false}, } for _, tt := range tests {