Further improve parsing of dictionaries / names.

With this change, the names are decoded internally, so they be can compared
directly when adding entries to dictionaries. On writing, the names are
encoded if necessary.

Also removed some duplicate code for name encoding / decoding.

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/dict.go

@@ -440,17 +440,16 @@ func (d Dict) indentedString(level int) string {
 	for _, k := range keys {
 
 		v := d[k]
-		key, _ := DecodeName(k)
 
 		if subdict, ok := v.(Dict); ok {
 			dictStr := subdict.indentedString(level + 1)
-			logstr = append(logstr, fmt.Sprintf("%s<%s, %s>\n", tabstr, key, dictStr))
+			logstr = append(logstr, fmt.Sprintf("%s<%s, %s>\n", tabstr, k, dictStr))
 			continue
 		}
 
 		if a, ok := v.(Array); ok {
 			arrStr := a.indentedString(level + 1)
-			logstr = append(logstr, fmt.Sprintf("%s<%s, %s>\n", tabstr, key, arrStr))
+			logstr = append(logstr, fmt.Sprintf("%s<%s, %s>\n", tabstr, k, arrStr))
 			continue
 		}
 
@@ -462,7 +461,7 @@ func (d Dict) indentedString(level int) string {
 			}
 		}
 
-		logstr = append(logstr, fmt.Sprintf("%s<%s, %v>\n", tabstr, key, val))
+		logstr = append(logstr, fmt.Sprintf("%s<%s, %v>\n", tabstr, k, val))
 
 	}
 
@@ -486,63 +485,64 @@ func (d Dict) PDFString() string {
 	for _, k := range keys {
 
 		v := d[k]
+		keyName := EncodeName(k)
 
 		if v == nil {
-			logstr = append(logstr, fmt.Sprintf("/%s null", k))
+			logstr = append(logstr, fmt.Sprintf("/%s null", keyName))
 			continue
 		}
 
 		d, ok := v.(Dict)
 		if ok {
-			logstr = append(logstr, fmt.Sprintf("/%s%s", k, d.PDFString()))
+			logstr = append(logstr, fmt.Sprintf("/%s%s", keyName, d.PDFString()))
 			continue
 		}
 
 		a, ok := v.(Array)
 		if ok {
-			logstr = append(logstr, fmt.Sprintf("/%s%s", k, a.PDFString()))
+			logstr = append(logstr, fmt.Sprintf("/%s%s", keyName, a.PDFString()))
 			continue
 		}
 
 		ir, ok := v.(IndirectRef)
 		if ok {
-			logstr = append(logstr, fmt.Sprintf("/%s %s", k, ir.PDFString()))
+			logstr = append(logstr, fmt.Sprintf("/%s %s", keyName, ir.PDFString()))
 			continue
 		}
 
 		n, ok := v.(Name)
 		if ok {
-			logstr = append(logstr, fmt.Sprintf("/%s%s", k, n.PDFString()))
+			logstr = append(logstr, fmt.Sprintf("/%s%s", keyName, n.PDFString()))
 			continue
 		}
 
 		i, ok := v.(Integer)
 		if ok {
-			logstr = append(logstr, fmt.Sprintf("/%s %s", k, i.PDFString()))
+			logstr = append(logstr, fmt.Sprintf("/%s %s", keyName, i.PDFString()))
 			continue
 		}
 
 		f, ok := v.(Float)
 		if ok {
-			logstr = append(logstr, fmt.Sprintf("/%s %s", k, f.PDFString()))
+			logstr = append(logstr, fmt.Sprintf("/%s %s", keyName, f.PDFString()))
 			continue
 		}
 
 		b, ok := v.(Boolean)
 		if ok {
-			logstr = append(logstr, fmt.Sprintf("/%s %s", k, b.PDFString()))
+			logstr = append(logstr, fmt.Sprintf("/%s %s", keyName, b.PDFString()))
 			continue
 		}
 
 		sl, ok := v.(StringLiteral)
 		if ok {
-			logstr = append(logstr, fmt.Sprintf("/%s%s", k, sl.PDFString()))
+			logstr = append(logstr, fmt.Sprintf("/%s%s", keyName, sl.PDFString()))
 			continue
 		}
 
 		hl, ok := v.(HexLiteral)
 		if ok {
-			logstr = append(logstr, fmt.Sprintf("/%s%s", k, hl.PDFString()))
+			logstr = append(logstr, fmt.Sprintf("/%s%s", keyName, hl.PDFString()))
 			continue
 		}
 

pkg/pdfcpu/types/dict_test.go

@@ -0,0 +1,32 @@
+/*
+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 types
+
+import (
+	"testing"
+)
+
+func TestEncodeDict(t *testing.T) {
+	dict := Dict{
+		"A()": Integer(1),
+	}
+	expected := `<</A#28#29 1>>`
+	s := dict.PDFString()
+	if s != expected {
+		t.Errorf("expected %s for %+v, got %s", expected, dict, s)
+	}
+}