diff --git a/content.go b/content.go index 91c4f44..d22b6c2 100644 --- a/content.go +++ b/content.go @@ -48,24 +48,59 @@ var inlineColourSpaces = map[Name]Name{ // Expanded returns the image's dictionary with the abbreviated keys and colour // space names written out, so it can be read like any image XObject. +// +// A dictionary may say the same thing twice and disagree with itself: /W 20 +// beside /Width 10, /CS /RGB beside /ColorSpace /3chanRGB. The specification +// permits both spellings in an inline image and says nothing about which wins, +// and the public implementations disagree — pdf.js asks for the abbreviation +// and falls back to the written-out name, MuPDF asks the other way round. +// +// The abbreviation wins here. Two reasons: it is the spelling Table 93 gives +// for an inline image, the written-out name being the tolerated alias; and +// safedocs' own fixture for this case marks the abbreviation as the line to +// remove "to see the effect", which is a statement about what the reference +// viewer does. +// +// What is not defensible is the answer changing between runs. Expanding in one +// pass over the map made the winner depend on Go's map iteration order, which +// is deliberately random: the same file drew differently each time it was +// opened, with no way for a caller to notice. Two passes settle it — the +// written-out names first, then the abbreviations over them — and no order +// within either pass can matter, because the names in each are distinct. func (im *InlineImage) Expanded() Dict { out := Dict{} for k, v := range im.Dict { - if long, ok := inlineKeys[k]; ok { - k = long + if _, abbreviated := inlineKeys[k]; abbreviated { + continue } - if k == "ColorSpace" { - if n, ok := ToName(v); ok { - if long, ok := inlineColourSpaces[n]; ok { - v = long - } - } + out[k] = expandColourSpace(k, v) + } + for k, v := range im.Dict { + long, abbreviated := inlineKeys[k] + if !abbreviated { + continue } - out[k] = v + out[long] = expandColourSpace(long, v) } return out } +// expandColourSpace writes out an abbreviated colour space name, which is the +// one value that is abbreviated as well as its key. +func expandColourSpace(k Name, v Object) Object { + if k != "ColorSpace" { + return v + } + n, ok := ToName(v) + if !ok { + return v + } + if long, ok := inlineColourSpaces[n]; ok { + return long + } + return v +} + // A ContentScanner walks a content stream one operation at a time. A stream // with rubbish in it yields the operations around the rubbish rather than // nothing at all, which is what a renderer needs; [ContentScanner.Err] reports diff --git a/content_test.go b/content_test.go index b3200d7..a0964c5 100644 --- a/content_test.go +++ b/content_test.go @@ -297,3 +297,50 @@ func TestOperandThatIsNotAnObject(t *testing.T) { t.Fatalf("got %+v", ops) } } + +// TestInlineImageExpandedIsDeterministic pins the one property that matters +// more than which spelling wins: that the answer is the same every time. +// +// safedocs' Inline_Image_Abbreviations fixture carries seven images that each +// say the same thing twice and disagree — /W 20 beside /Width 10, /CS /RGB +// beside /ColorSpace /3chanRGB. Expanding in one pass over the map let Go's +// randomised iteration order pick the winner, so the same page drew a +// different picture on different runs of the same binary. Two hundred +// expansions of the same dictionary is enough to catch that: against the one +// pass this loop sees both answers within the first few. +func TestInlineImageExpandedIsDeterministic(t *testing.T) { + im := &InlineImage{Dict: Dict{ + "W": Integer(20), + "Width": Integer(10), + "H": Integer(10), + "Height": Integer(40), + "BPC": Integer(8), + "CS": Name("RGB"), + "ColorSpace": Name("3chanRGB"), + "I": Bool(false), + }} + first := im.Expanded() + for i := 0; i < 200; i++ { + got := im.Expanded() + if len(got) != len(first) { + t.Fatalf("expansion %d has %d keys, the first had %d", i, len(got), len(first)) + } + for k, v := range first { + if got[k] != v { + t.Fatalf("expansion %d gave %s = %v, the first gave %v", i, k, got[k], v) + } + } + } + // And the abbreviation is what wins, which is the choice this makes and + // the reason it is written down in Expanded's own comment. + for k, want := range map[Name]Object{ + "Width": Integer(20), + "Height": Integer(10), + "ColorSpace": Name("DeviceRGB"), + "Interpolate": Bool(false), + } { + if first[k] != want { + t.Errorf("%s = %v, want %v", k, first[k], want) + } + } +}