Further improve parsing of dictionaries / names

pkg/pdfcpu/model/parse.go

@@ -18,7 +18,6 @@ package model
 
 import (
 	"context"
-	"encoding/hex"
 	"strconv"
 	"strings"
 	"unicode"
@@ -461,30 +460,13 @@ func parseHexLiteral(line *string) (types.Object, error) {
 	return types.HexLiteral(*hexStr), nil
 }
 
-func validateNameHexSequence(s string) error {
-	for i := 0; i < len(s); {
-		c := s[i]
-		if c != '#' {
-			i++
-			continue
-		}
-
-		// # detected, next 2 chars have to exist.
-		if len(s) < i+3 {
-			return errNameObjectCorrupt
-		}
-
-		s1 := s[i+1 : i+3]
-
-		// And they have to be hex characters.
-		_, err := hex.DecodeString(s1)
-		if err != nil {
-			return errNameObjectCorrupt
-		}
-
-		i += 3
+func decodeNameHexSequence(s string) (string, error) {
+	decoded, err := types.DecodeName(s)
+	if err != nil {
+		return "", errNameObjectCorrupt
 	}
-	return nil
+
+	return decoded, nil
 }
 
 func parseName(line *string) (*types.Name, error) {
@@ -518,8 +500,8 @@ func parseName(line *string) (*types.Name, error) {
 		l = l[:eok]
 	}
 
-	// Validate optional #xx sequences
-	err := validateNameHexSequence(l)
+	// Decode optional #xx sequences
+	l, err := decodeNameHexSequence(l)
 	if err != nil {
 		return nil, err
 	}
@@ -528,48 +510,30 @@ func parseName(line *string) (*types.Name, error) {
 	return &nameObj, nil
 }
 
-func insertKey(d types.Dict, key string, val types.Object, usesHexCodes bool) (bool, error) {
-	var duplicateKeyErr bool
-
-	if !usesHexCodes {
-		if strings.IndexByte(key, '#') < 0 {
-			// Avoid expensive "DecodeName".
-			if _, found := d[key]; !found {
-				d[key] = val
-			} else {
-				duplicateKeyErr = true
-			}
-		} else {
-			duplicateKeyErr = d.Insert(key, val)
-			usesHexCodes = true
-		}
+func insertKey(d types.Dict, key string, val types.Object) error {
+	if _, found := d[key]; !found {
+		d[key] = val
 	} else {
-		duplicateKeyErr = d.Insert(key, val)
-	}
-
-	if duplicateKeyErr {
 		// for now we digest duplicate keys.
 		// TODO
 		// if !validationRelaxed {
-		// 	return false, errDictionaryDuplicateKey
+		// 	return errDictionaryDuplicateKey
 		// }
 		// if log.CLIEnabled() {
 		// 	log.CLI.Printf("ParseDict: digesting duplicate key\n")
 		// }
-		_ = duplicateKeyErr
 	}
 
 	if log.ParseEnabled() {
 		log.Parse.Printf("ParseDict: dict[%s]=%v\n", key, val)
 	}
 
-	return usesHexCodes, nil
+	return nil
 }
 
 func processDictKeys(c context.Context, line *string, relaxed bool) (types.Dict, error) {
 	l := *line
 	var eol bool
-	var usesHexCodes bool
 	d := types.NewDict()
 
 	for !strings.HasPrefix(l, ">>") {
@@ -612,13 +576,9 @@ func processDictKeys(c context.Context, line *string, relaxed bool) (types.Dict,
 		// Specifying the null object as the value of a dictionary entry (7.3.7, "Dictionary Objects")
 		// shall be equivalent to omitting the entry entirely.
 		if val != nil {
-			detectedHexCodes, err := insertKey(d, string(*keyName), val, usesHexCodes)
-			if err != nil {
+			if err := insertKey(d, string(*keyName), val); err != nil {
 				return nil, err
 			}
-			if !usesHexCodes && detectedHexCodes {
-				usesHexCodes = true
-			}
 		}
 
 		// We are positioned on the char behind the last parsed dict value.

pkg/pdfcpu/model/parse_dict_test.go

@@ -150,6 +150,7 @@ func doTestParseDictWithComments(t *testing.T) {
 func doTestLargeDicts(t *testing.T) {
 	var sb strings.Builder
 	sb.WriteString("<<")
+	sb.WriteString("/Key#28#29 (Value)")
 	for i := 0; i < 50000; i++ {
 		sb.WriteString(fmt.Sprintf("/Key%d (Value)", i))
 	}

pkg/pdfcpu/model/parse_test.go

@@ -0,0 +1,68 @@
+/*
+Copyright 2024 The pdfcpu Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package model
+
+import (
+	"testing"
+)
+
+func TestDecodeNameHexInvalid(t *testing.T) {
+	testcases := []string{
+		"#",
+		"#A",
+		"#a",
+		"#G0",
+		"#00",
+		"Fo\x00",
+	}
+	for _, tc := range testcases {
+		if decoded, err := decodeNameHexSequence(tc); err == nil {
+			t.Errorf("expected error decoding %s, got %s", tc, decoded)
+		}
+	}
+}
+
+func TestDecodeNameHexValid(t *testing.T) {
+	testcases := []struct {
+		Input    string
+		Expected string
+	}{
+		{"", ""},
+		{"Foo", "Foo"},
+		{"A#23", "A#"},
+		// Examples from "7.3.5 Name Objects"
+		{"Name1", "Name1"},
+		{"ASomewhatLongerName", "ASomewhatLongerName"},
+		{"A;Name_With-Various***Characters?", "A;Name_With-Various***Characters?"},
+		{"1.2", "1.2"},
+		{"$$", "$$"},
+		{"@pattern", "@pattern"},
+		{".notdef", ".notdef"},
+		{"Lime#20Green", "Lime Green"},
+		{"paired#28#29parentheses", "paired()parentheses"},
+		{"The_Key_of_F#23_Minor", "The_Key_of_F#_Minor"},
+		{"A#42", "AB"},
+	}
+	for _, tc := range testcases {
+		decoded, err := decodeNameHexSequence(tc.Input)
+		if err != nil {
+			t.Errorf("decoding %s failed: %s", tc.Input, err)
+		} else if decoded != tc.Expected {
+			t.Errorf("expected %s when decoding %s, got %s", tc.Expected, tc.Input, decoded)
+		}
+	}
+}

pkg/pdfcpu/types/array.go

@@ -115,28 +115,25 @@ func (a Array) indentedString(level int) string {
 			sepstr = " "
 		}
 
-		if subdict, ok := entry.(Dict); ok {
-			dictstr := subdict.indentedString(level + 1)
+		switch entry := entry.(type) {
+		case Dict:
+			dictstr := entry.indentedString(level + 1)
 			logstr = append(logstr, fmt.Sprintf("\n%[1]s%[2]s\n%[1]s", tabstr, dictstr))
 			first = true
-			continue
-		}
-
-		if array, ok := entry.(Array); ok {
-			arrstr := array.indentedString(level + 1)
+		case Array:
+			arrstr := entry.indentedString(level + 1)
 			logstr = append(logstr, fmt.Sprintf("%s%s", sepstr, arrstr))
-			continue
-		}
-
-		v := "null"
-		if entry != nil {
-			v = entry.String()
-			if n, ok := entry.(Name); ok {
-				v, _ = DecodeName(string(n))
+		default:
+			v := "null"
+			if entry != nil {
+				v = entry.String()
+				if n, ok := entry.(Name); ok {
+					v, _ = DecodeName(string(n))
+				}
 			}
-		}
 
-		logstr = append(logstr, fmt.Sprintf("%s%v", sepstr, v))
+			logstr = append(logstr, fmt.Sprintf("%s%v", sepstr, v))
+		}
 	}
 
 	logstr = append(logstr, "]")
@@ -165,66 +162,32 @@ func (a Array) PDFString() string {
 			sepstr = " "
 		}
 
-		if entry == nil {
+		switch entry := entry.(type) {
+		case nil:
 			logstr = append(logstr, fmt.Sprintf("%snull", sepstr))
+		case Dict:
+			logstr = append(logstr, entry.PDFString())
+		case Array:
+			logstr = append(logstr, entry.PDFString())
 			continue
-		}
-
-		d, ok := entry.(Dict)
-		if ok {
-			logstr = append(logstr, d.PDFString())
-			continue
-		}
-
-		a, ok := entry.(Array)
-		if ok {
-			logstr = append(logstr, a.PDFString())
-			continue
-		}
-
-		ir, ok := entry.(IndirectRef)
-		if ok {
-			logstr = append(logstr, fmt.Sprintf("%s%s", sepstr, ir.PDFString()))
-			continue
-		}
-
-		n, ok := entry.(Name)
-		if ok {
-			logstr = append(logstr, n.PDFString())
-			continue
-		}
-
-		i, ok := entry.(Integer)
-		if ok {
-			logstr = append(logstr, fmt.Sprintf("%s%s", sepstr, i.PDFString()))
-			continue
-		}
-
-		f, ok := entry.(Float)
-		if ok {
-			logstr = append(logstr, fmt.Sprintf("%s%s", sepstr, f.PDFString()))
-			continue
-		}
-
-		b, ok := entry.(Boolean)
-		if ok {
-			logstr = append(logstr, fmt.Sprintf("%s%s", sepstr, b.PDFString()))
-			continue
-		}
-		sl, ok := entry.(StringLiteral)
-		if ok {
-			logstr = append(logstr, fmt.Sprintf("%s%s", sepstr, sl.PDFString()))
-			continue
-		}
-
-		hl, ok := entry.(HexLiteral)
-		if ok {
-			logstr = append(logstr, fmt.Sprintf("%s%s", sepstr, hl.PDFString()))
-			continue
-		}
-
-		if log.InfoEnabled() {
-			log.Info.Fatalf("PDFArray.PDFString(): entry of unknown object type: %[1]T %[1]v\n", entry)
+		case IndirectRef:
+			logstr = append(logstr, fmt.Sprintf("%s%s", sepstr, entry.PDFString()))
+		case Name:
+			logstr = append(logstr, entry.PDFString())
+		case Integer:
+			logstr = append(logstr, fmt.Sprintf("%s%s", sepstr, entry.PDFString()))
+		case Float:
+			logstr = append(logstr, fmt.Sprintf("%s%s", sepstr, entry.PDFString()))
+		case Boolean:
+			logstr = append(logstr, fmt.Sprintf("%s%s", sepstr, entry.PDFString()))
+		case StringLiteral:
+			logstr = append(logstr, fmt.Sprintf("%s%s", sepstr, entry.PDFString()))
+		case HexLiteral:
+			logstr = append(logstr, fmt.Sprintf("%s%s", sepstr, entry.PDFString()))
+		default:
+			if log.InfoEnabled() {
+				log.Info.Fatalf("PDFArray.PDFString(): entry of unknown object type: %[1]T %[1]v\n", entry)
+			}
 		}
 	}