diff --git a/pkg/archive/archive_zip.go b/pkg/archive/archive_zip.go index 1f5590a2..7462d441 100644 --- a/pkg/archive/archive_zip.go +++ b/pkg/archive/archive_zip.go @@ -320,8 +320,45 @@ func (e *gozipArchiveEntry) ReadCompressedGzip() ([]byte, error) { type gozipArchive struct { zip *zip.Reader closer func() error - cachedEntries sync.Map minimizeReads bool + + // nameIndex maps each entry's cleaned path to its *zip.File, giving O(1) + // lookups instead of a linear scan. It's built once, lazily, on the first + // Entry call that misses the wrapper cache. + indexOnce sync.Once + nameIndex map[string]*zip.File + + // cachedEntries holds lazily-created *gozipArchiveEntry wrappers, keyed by + // cleaned path. Wrappers are shared so per-entry read state (e.g. the + // deflate random-access index) persists across reads of the same entry. + cachedEntries sync.Map +} + +// buildIndex populates nameIndex from the archive's central directory. First +// occurrence of a cleaned path wins, matching the previous linear-scan lookup. +func (a *gozipArchive) buildIndex() { + nameIndex := make(map[string]*zip.File, len(a.zip.File)) + for _, f := range a.zip.File { + cp := path.Clean(f.Name) + if _, ok := nameIndex[cp]; !ok { + nameIndex[cp] = f + } + } + a.nameIndex = nameIndex +} + +// wrap returns the shared entry wrapper for f, creating it on first access. +// LoadOrStore guarantees a single wrapper per entry even under concurrent Get. +func (a *gozipArchive) wrap(cleanPath string, f *zip.File) *gozipArchiveEntry { + if e, ok := a.cachedEntries.Load(cleanPath); ok { + return e.(*gozipArchiveEntry) + } + entry := &gozipArchiveEntry{ + file: f, + minimizeReads: a.minimizeReads, + } + actual, _ := a.cachedEntries.LoadOrStore(cleanPath, entry) + return actual.(*gozipArchiveEntry) } // Close implements Archive @@ -336,16 +373,7 @@ func (a *gozipArchive) Entries() []Entry { if f.FileInfo().IsDir() { continue } - - aentry, ok := a.cachedEntries.Load(f.Name) - if !ok { - aentry = &gozipArchiveEntry{ - file: f, - minimizeReads: a.minimizeReads, - } - a.cachedEntries.Store(f.Name, aentry) - } - entries = append(entries, aentry.(Entry)) + entries = append(entries, a.wrap(path.Clean(f.Name), f)) } return entries } @@ -357,24 +385,17 @@ func (a *gozipArchive) Entry(p string) (Entry, error) { } cpath := path.Clean(p) - // Check for entry in cache - aentry, ok := a.cachedEntries.Load(cpath) - if ok { // Found entry in cache + // Check for an already-wrapped entry first. + if aentry, ok := a.cachedEntries.Load(cpath); ok { return aentry.(Entry), nil } - for _, f := range a.zip.File { - fp := path.Clean(f.Name) - if fp == cpath { - aentry := &gozipArchiveEntry{ - file: f, - minimizeReads: a.minimizeReads, - } - a.cachedEntries.Store(fp, aentry) // Put entry in cache - return aentry, nil - } + a.indexOnce.Do(a.buildIndex) + f, ok := a.nameIndex[cpath] + if !ok { + return nil, fs.ErrNotExist } - return nil, fs.ErrNotExist + return a.wrap(cpath, f), nil } func NewGoZIPArchive(zip *zip.Reader, closer func() error, minimizeReads bool) Archive { diff --git a/pkg/parser/epub/parser.go b/pkg/parser/epub/parser.go index fa66031a..e22046d1 100644 --- a/pkg/parser/epub/parser.go +++ b/pkg/parser/epub/parser.go @@ -3,6 +3,7 @@ package epub import ( "context" + "github.com/antchfx/xmlquery" "github.com/pkg/errors" "github.com/readium/go-toolkit/pkg/asset" "github.com/readium/go-toolkit/pkg/content/iterator" @@ -57,12 +58,12 @@ func (p Parser) Parse(ctx context.Context, asset asset.PublicationAsset, f fetch // themselves through other well-known files (rights.xml / sinf.xml). // TODO: surface the publication-level scheme on the manifest itself so // consumers can detect protection even when encryption.xml is absent. - scheme, err := protection.IdentifyEPUBProtection(ctx, f) + scheme, encryptionDoc, err := protection.IdentifyEPUBProtection(ctx, f) if err != nil { return nil, errors.Wrap(err, "failed identifying EPUB protection scheme") } - encryptionData, err := parseEncryptionData(ctx, f, scheme.URI()) + encryptionData, err := parseEncryptionData(ctx, f, scheme.URI(), encryptionDoc) if err != nil { return nil, errors.Wrap(err, "failed parsing encryption data") } @@ -94,12 +95,19 @@ func (p Parser) Parse(ctx context.Context, asset asset.PublicationAsset, f fetch // each entry with the supplied DRM scheme URI (typically obtained by calling // [protection.IdentifyEPUBProtection] at a higher level). A missing // encryption.xml is normal and returns (nil, nil). -func parseEncryptionData(ctx context.Context, f fetcher.Fetcher, scheme string) (map[string]manifest.Encryption, error) { - n, rerr := fetcher.ReadResourceAsXML(ctx, f.Get(ctx, manifest.Link{Href: manifest.MustNewHREFFromString("META-INF/encryption.xml", false)})) - if rerr != nil { - return nil, nil +// +// When doc is non-nil it is used directly, avoiding a redundant read+parse of +// encryption.xml — [protection.IdentifyEPUBProtection] returns the document it +// already parsed for exactly this purpose. +func parseEncryptionData(ctx context.Context, f fetcher.Fetcher, scheme string, doc *xmlquery.Node) (map[string]manifest.Encryption, error) { + if doc == nil { + var rerr *fetcher.ResourceError + doc, rerr = fetcher.ReadResourceAsXML(ctx, f.Get(ctx, manifest.Link{Href: manifest.MustNewHREFFromString("META-INF/encryption.xml", false)})) + if rerr != nil { + return nil, nil + } } - return ParseEncryption(n, scheme), nil + return ParseEncryption(doc, scheme), nil } func parseNavigationData(ctx context.Context, packageDocument PackageDocument, f fetcher.Fetcher) (ret map[string]manifest.LinkList) { diff --git a/pkg/parser/epub/parser_bench_test.go b/pkg/parser/epub/parser_bench_test.go new file mode 100644 index 00000000..bb235fb0 --- /dev/null +++ b/pkg/parser/epub/parser_bench_test.go @@ -0,0 +1,43 @@ +package epub + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/readium/go-toolkit/pkg/fetcher" + "github.com/stretchr/testify/require" +) + +func BenchmarkParser(b *testing.B) { + pubsDir := "../../../publications" + files, err := os.ReadDir(pubsDir) + if err != nil { + b.Skip("publications directory not found") + } + + for _, file := range files { + if file.IsDir() || filepath.Ext(file.Name()) != ".epub" { + continue + } + + b.Run(file.Name(), func(b *testing.B) { + path := filepath.Join(pubsDir, file.Name()) + + parser := NewParser(nil) + asset := fakeEPUBAsset{name: file.Name()} + for i := 0; i < b.N; i++ { + // We recreate fetcher in each iteration because in real world we parse from fresh fetcher. + f, err := fetcher.NewArchiveFetcherFromPath(context.Background(), path) + require.NoError(b, err) + func() { + defer f.Close() + builder, err := parser.Parse(context.Background(), asset, f) + require.NoError(b, err) + require.NotNil(b, builder) + }() + } + }) + } +} diff --git a/pkg/parser/epub/parser_navdoc.go b/pkg/parser/epub/parser_navdoc.go index a2c74339..18bfe6b6 100644 --- a/pkg/parser/epub/parser_navdoc.go +++ b/pkg/parser/epub/parser_navdoc.go @@ -92,21 +92,26 @@ func parseLiElement(li *xmlquery.Node, filePath url.URL) (link *manifest.Link) { } var title string if first.Data != "ol" { - title = strings.TrimSpace(muchSpaceSuchWowMatcher.ReplaceAllString(first.InnerText(), " ")) + title = collapseWhitespace(first.InnerText()) } rawHref := first.SelectAttr("href") - href := url.MustURLFromString("#") + var href url.URL if first.Data == "a" && rawHref != "" { - s, err := url.FromEPUBHref(rawHref) - if err == nil { + if s, err := url.FromEPUBHref(rawHref); err == nil { href = filePath.Resolve(s) } } children := parseOlElement(xmlquery.QuerySelector(li, xpNavOL), filePath) - if len(children) == 0 && (href.String() == "" || title == "") { + // A nil href here is the lazy stand-in for the old "#" default, whose + // String() is "" — so a missing or empty-resolved href drops the entry + // (unless it has children), exactly as before. + if len(children) == 0 && (href == nil || href.String() == "" || title == "") { return nil } + if href == nil { + href = url.MustURLFromString("#") + } return &manifest.Link{ Title: title, Href: manifest.NewHREF(href), diff --git a/pkg/parser/epub/parser_ncx.go b/pkg/parser/epub/parser_ncx.go index d4b63e73..3ff50a6f 100644 --- a/pkg/parser/epub/parser_ncx.go +++ b/pkg/parser/epub/parser_ncx.go @@ -1,8 +1,6 @@ package epub import ( - "strings" - "github.com/antchfx/xmlquery" "github.com/readium/go-toolkit/pkg/manifest" "github.com/readium/go-toolkit/pkg/util/url" @@ -92,7 +90,7 @@ func extractTitle(element *xmlquery.Node) string { if tel == nil { return "" } - return strings.TrimSpace(muchSpaceSuchWowMatcher.ReplaceAllString(tel.InnerText(), " ")) + return collapseWhitespace(tel.InnerText()) } func extractHref(element *xmlquery.Node, filePath url.URL) url.URL { diff --git a/pkg/parser/epub/parser_packagedoc.go b/pkg/parser/epub/parser_packagedoc.go index 6c6414a5..01200338 100644 --- a/pkg/parser/epub/parser_packagedoc.go +++ b/pkg/parser/epub/parser_packagedoc.go @@ -178,7 +178,7 @@ func ParseItemRef(element *xmlquery.Node, prefixMap map[string]string) *ItemRef pp := parseProperties(element.SelectAttr("properties")) properties := make([]string, 0, len(pp)) - for _, prop := range parseProperties(element.SelectAttr("properties")) { + for _, prop := range pp { if prop == "" { continue } diff --git a/pkg/parser/epub/parser_test.go b/pkg/parser/epub/parser_test.go index 94a93373..cffb419f 100644 --- a/pkg/parser/epub/parser_test.go +++ b/pkg/parser/epub/parser_test.go @@ -107,10 +107,10 @@ func TestParseEncryptionDataScheme(t *testing.T) { t.Run(tt.name, func(t *testing.T) { f := openProtectionFixture(t, tt.file) - scheme, err := protection.IdentifyEPUBProtection(t.Context(), f) + scheme, encryptionDoc, err := protection.IdentifyEPUBProtection(t.Context(), f) require.NoError(t, err) - enc, err := parseEncryptionData(t.Context(), f, scheme.URI()) + enc, err := parseEncryptionData(t.Context(), f, scheme.URI(), encryptionDoc) require.NoError(t, err) if !tt.expectEntries { assert.Empty(t, enc) diff --git a/pkg/parser/epub/property_data_type.go b/pkg/parser/epub/property_data_type.go index e750ff26..5a6bbe3e 100644 --- a/pkg/parser/epub/property_data_type.go +++ b/pkg/parser/epub/property_data_type.go @@ -78,15 +78,34 @@ func parsePrefixes(prefixes string) map[string]string { return p } -var muchSpaceSuchWowMatcher = regexp.MustCompile(`\s+`) - func parseProperties(raw string) []string { - vals := muchSpaceSuchWowMatcher.Split(raw, -1) - s := make([]string, 0, len(vals)) - for _, v := range vals { - if v != "" { - s = append(s, v) + return strings.Fields(raw) +} + +// collapseWhitespace trims leading/trailing ASCII whitespace and replaces every +// internal run of it with a single space. It reproduces the previous +// regexp.ReplaceAllString(`\s+`, " ") + TrimSpace behavior — deliberately ASCII +// only (the same set RE2's \s matches), so non-ASCII spaces such as the U+3000 +// ideographic space common in CJK titles are preserved. Scanning byte-wise is +// safe because UTF-8 continuation bytes are always >= 0x80 and never collide +// with ASCII whitespace. +func collapseWhitespace(s string) string { + var b strings.Builder + b.Grow(len(s)) + pendingSpace := false + for i := 0; i < len(s); i++ { + switch s[i] { + case ' ', '\t', '\n', '\f', '\r': + if b.Len() > 0 { + pendingSpace = true + } + default: + if pendingSpace { + b.WriteByte(' ') + pendingSpace = false + } + b.WriteByte(s[i]) } } - return s + return b.String() } diff --git a/pkg/protection/epub.go b/pkg/protection/epub.go index 78d5d212..d44bd117 100644 --- a/pkg/protection/epub.go +++ b/pkg/protection/epub.go @@ -9,7 +9,6 @@ import ( "github.com/pkg/errors" "github.com/readium/go-toolkit/pkg/fetcher" "github.com/readium/go-toolkit/pkg/manifest" - "github.com/readium/go-toolkit/pkg/util/url" ) // Well-known XML namespaces used by EPUB DRM containers. @@ -52,21 +51,30 @@ func mustCompileNS(expr string) *xpath.Expr { } // IdentifyEPUBProtection inspects the well-known DRM metadata files inside an -// EPUB container and returns the detected protection [Scheme]. Returns [NoDRM] -// when no protection metadata is present. -func IdentifyEPUBProtection(ctx context.Context, f fetcher.Fetcher) (Scheme, error) { - links, err := f.Links(ctx) - if err != nil { - return NoDRM, err - } - - hasLink := func(path string) (*manifest.Link, bool) { - u, uerr := url.URLFromString(path) - if uerr != nil { - return nil, false +// EPUB container and returns the detected protection [Scheme]. The second return +// value is the parsed encryption.xml document when available (otherwise nil). +// Returns [NoDRM] when no protection metadata is present. +func IdentifyEPUBProtection(ctx context.Context, f fetcher.Fetcher) (Scheme, *xmlquery.Node, error) { + // hasLink probes for the presence of a resource. It returns a non-nil link + // when the resource exists, (nil, nil) when it is genuinely absent + // (NotFound), and a non-nil error when the resource exists but can't be read + // (e.g. Forbidden, Offline, Timeout) so detection can surface the failure + // instead of silently falling through to NoDRM. + hasLink := func(path string) (*manifest.Link, error) { + href, err := manifest.NewHREFFromString(path, false) + if err != nil { + return nil, err + } + link := manifest.Link{Href: href} + res := f.Get(ctx, link) + defer res.Close() + if _, lerr := res.Length(ctx); lerr != nil { + if lerr.Code == fetcher.CodeNotFound { + return nil, nil + } + return nil, errors.Wrap(lerr.Cause, "unable to probe "+path) } - l := links.FirstWithHref(u) - return l, l != nil + return &link, nil } readXML := func(link *manifest.Link) (*xmlquery.Node, error) { @@ -81,62 +89,73 @@ func IdentifyEPUBProtection(ctx context.Context, f fetcher.Fetcher) (Scheme, err } // LCP: presence of the license file is the strongest signal. - if _, ok := hasLink(pathLCPLicense); ok { - return LCP, nil + if link, err := hasLink(pathLCPLicense); err != nil { + return NoDRM, nil, err + } else if link != nil { + return LCP, nil, nil } // Apple FairPlay: META-INF/sinf.xml containing . - if link, ok := hasLink(pathFairplaySinf); ok { + if link, err := hasLink(pathFairplaySinf); err != nil { + return NoDRM, nil, err + } else if link != nil { doc, derr := readXML(link) if derr != nil { - return NoDRM, derr + return NoDRM, nil, derr } if doc != nil && xmlquery.QuerySelector(doc, xpFairplaySinf) != nil { - return Fairplay, nil + return Fairplay, nil, nil } } // Adobe ADEPT (and Barnes & Noble): META-INF/rights.xml with . - if link, ok := hasLink(pathAdeptRights); ok { + if link, err := hasLink(pathAdeptRights); err != nil { + return NoDRM, nil, err + } else if link != nil { doc, derr := readXML(link) if derr != nil { - return NoDRM, derr + return NoDRM, nil, derr } if doc != nil { if op := xmlquery.QuerySelector(doc, xpAdeptOperator); op != nil { if strings.Contains(strings.ToLower(op.InnerText()), barnesAndNobleTag) { - return BarnesAndNoble, nil + return BarnesAndNoble, nil, nil } - return Adept, nil + return Adept, nil, nil } } } // Kobo: rights.xml at the container root containing . - if link, ok := hasLink(pathKoboRights); ok { + if link, err := hasLink(pathKoboRights); err != nil { + return NoDRM, nil, err + } else if link != nil { doc, derr := readXML(link) if derr != nil { - return NoDRM, derr + return NoDRM, nil, derr } if doc != nil && xmlquery.QuerySelector(doc, xpKdrm) != nil { - return Kobo, nil + return Kobo, nil, nil } } // Fall back to META-INF/encryption.xml: it may reveal LCP via the - // retrieval method, or indicate generic/unknown encryption otherwise. - if link, ok := hasLink(pathEncryption); ok { + // retrieval method, or indicate generic/unknown encryption otherwise. The + // parsed document is handed back so the caller can avoid re-parsing it. + if link, err := hasLink(pathEncryption); err != nil { + return NoDRM, nil, err + } else if link != nil { doc, derr := readXML(link) if derr != nil { - return NoDRM, derr + return NoDRM, nil, derr } if doc != nil { if xmlquery.QuerySelector(doc, xpLCPRetrieval) != nil { - return LCP, nil + return LCP, doc, nil } - return Generic, nil + return Generic, doc, nil } } - return NoDRM, nil + return NoDRM, nil, nil } diff --git a/pkg/protection/epub_test.go b/pkg/protection/epub_test.go index 30da86ea..b42c4917 100644 --- a/pkg/protection/epub_test.go +++ b/pkg/protection/epub_test.go @@ -26,7 +26,7 @@ func TestIdentifyEPUBProtection(t *testing.T) { require.NoError(t, err) defer f.Close() - got, err := IdentifyEPUBProtection(t.Context(), f) + got, _, err := IdentifyEPUBProtection(t.Context(), f) require.NoError(t, err) assert.Equal(t, tt.want, got, "unexpected scheme for %s", tt.file) }) @@ -34,7 +34,7 @@ func TestIdentifyEPUBProtection(t *testing.T) { } func TestIdentifyEPUBProtectionNoDRM(t *testing.T) { - got, err := IdentifyEPUBProtection(t.Context(), fetcher.EmptyFetcher{}) + got, _, err := IdentifyEPUBProtection(t.Context(), fetcher.EmptyFetcher{}) require.NoError(t, err) assert.Equal(t, NoDRM, got) }