Skip to content

Commit 36959d3

Browse files
alexpevzneralexpevzner
committed
Accept textWithLanguage for many printer attrs where text is expected.
For many printer attributes ipp-usb erroneously expected that value will be the bare string, while standard allows textWithLanguage as well. It broke announcing of the "ty", "product", "usb_MFG", "usb_MDL", and "usb_CMD" TXT records for Brother MFC-L8390CDW, and, probably, some other similar devices (see #109 for details). Now fixed and test-covered.
1 parent 850b3e2 commit 36959d3

2 files changed

Lines changed: 358 additions & 5 deletions

File tree

‎ipp.go‎

Lines changed: 63 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -468,13 +468,69 @@ func (attrs ippAttrs) strBrackets(name string) string {
468468

469469
// Get attribute's []string value by attribute name
470470
func (attrs ippAttrs) getStrings(name string) []string {
471-
vals := attrs.getAttr(goipp.TypeString, name)
472-
strs := make([]string, len(vals))
471+
// Obtain attribute values.
472+
vals := attrs[name]
473+
if vals == nil {
474+
return nil
475+
}
476+
477+
// Classify available strings by language
478+
langsInOrder := make([]string, 0, len(vals))
479+
stringsByLang := make(map[string][]string, len(vals))
480+
stringsWithoutLang := make([]string, 0, len(vals))
481+
473482
for i := range vals {
474-
strs[i] = string(vals[i].(goipp.String))
483+
switch val := vals[i].V.(type) {
484+
case goipp.String:
485+
stringsWithoutLang = append(stringsWithoutLang,
486+
string(val))
487+
case goipp.TextWithLang:
488+
lang := strings.ToLower(val.Lang)
489+
490+
strings := stringsByLang[lang]
491+
stringsByLang[lang] = append(strings, val.Text)
492+
493+
if strings == nil {
494+
langsInOrder = append(langsInOrder, lang)
495+
}
496+
}
497+
}
498+
499+
// If we have goipp.String values, just use them
500+
if len(stringsWithoutLang) != 0 {
501+
return stringsWithoutLang
502+
}
503+
504+
// Choose the most appropriate language
505+
if len(langsInOrder) == 0 {
506+
// No strings, return nil
507+
return nil
475508
}
476509

477-
return strs
510+
if len(langsInOrder) == 1 {
511+
// We have only one language, use it
512+
return stringsByLang[langsInOrder[0]]
513+
}
514+
515+
if strings := stringsByLang["en"]; strings != nil {
516+
// Use "en" if available
517+
return strings
518+
}
519+
520+
if strings := stringsByLang["en-us"]; strings != nil {
521+
// Use "en-us"
522+
return strings
523+
}
524+
525+
for _, lang := range langsInOrder {
526+
// Use the first variant of "en-" in the list.
527+
if strings.HasPrefix(lang, "en") {
528+
return stringsByLang[lang]
529+
}
530+
}
531+
532+
// Fallback to the first language in the list
533+
return stringsByLang[langsInOrder[0]]
478534
}
479535

480536
// Get boolean attribute. Returns "F" or "T" if attribute is found,
@@ -497,7 +553,9 @@ func (attrs ippAttrs) getAttr(t goipp.Type, name string) []goipp.Value {
497553
if ok && v[0].V.Type() == t {
498554
var vals []goipp.Value
499555
for i := range v {
500-
vals = append(vals, v[i].V)
556+
if v[i].V.Type() == t {
557+
vals = append(vals, v[i].V)
558+
}
501559
}
502560
return vals
503561
}

‎ipp_test.go‎

Lines changed: 295 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
package main
1010

1111
import (
12+
"reflect"
1213
"testing"
1314

1415
"github.com/OpenPrinting/goipp"
@@ -92,7 +93,301 @@ func TestNewIppAttrs(t *testing.T) {
9293
f.SetIndent(0)
9394

9495
t.Errorf("%s", f.String())
96+
}
97+
}
98+
}
9599

100+
// TestIppAttrsGetStrings tests ippAttrs.getStrings function
101+
func TestIppAttrsGetStrings(t *testing.T) {
102+
type testData struct {
103+
attrs goipp.Attributes // Attributes in the set
104+
name string // Requested name
105+
out []string // Expected ippAttrs.getStrings output
106+
}
107+
108+
tests := []testData{
109+
{
110+
// Empty set returns nil
111+
attrs: nil,
112+
name: "printer-make-and-model",
113+
out: nil,
114+
},
115+
116+
{
117+
// Invalid attribute type, nil is expected
118+
attrs: goipp.Attributes{
119+
goipp.MakeAttr("printer-make-and-model",
120+
goipp.TagInteger,
121+
goipp.Integer(5)),
122+
},
123+
name: "printer-make-and-model",
124+
out: nil,
125+
},
126+
127+
{
128+
// Single value of type goipp.String
129+
attrs: goipp.Attributes{
130+
goipp.MakeAttr("printer-make-and-model",
131+
goipp.TagText,
132+
goipp.String("test printer")),
133+
},
134+
name: "printer-make-and-model",
135+
out: []string{"test printer"},
136+
},
137+
138+
{
139+
// Single value of type goipp.TextWithLang
140+
attrs: goipp.Attributes{
141+
goipp.MakeAttr("printer-make-and-model",
142+
goipp.TagTextLang,
143+
goipp.TextWithLang{
144+
Lang: "en",
145+
Text: "test printer",
146+
},
147+
),
148+
},
149+
name: "printer-make-and-model",
150+
out: []string{"test printer"},
151+
},
152+
153+
{
154+
// Mix of goipp.String/goipp.TextWithLang
155+
// goipp.String wins
156+
attrs: goipp.Attributes{
157+
goipp.MakeAttr("printer-make-and-model",
158+
goipp.TagTextLang,
159+
goipp.TextWithLang{
160+
Lang: "en",
161+
Text: "test printer - en",
162+
},
163+
goipp.String("test printer"),
164+
),
165+
},
166+
name: "printer-make-and-model",
167+
out: []string{"test printer"},
168+
},
169+
170+
{
171+
// Mix of goipp.String/goipp.TextWithLang, reordered
172+
// goipp.String wins
173+
attrs: goipp.Attributes{
174+
goipp.MakeAttr("printer-make-and-model",
175+
goipp.TagTextLang,
176+
goipp.String("test printer"),
177+
goipp.TextWithLang{
178+
Lang: "en",
179+
Text: "test printer - en",
180+
},
181+
),
182+
},
183+
name: "printer-make-and-model",
184+
out: []string{"test printer"},
185+
},
186+
187+
{
188+
// "en" vs "ru". "en" wins
189+
attrs: goipp.Attributes{
190+
goipp.MakeAttr("printer-make-and-model",
191+
goipp.TagTextLang,
192+
goipp.TextWithLang{
193+
Lang: "en",
194+
Text: "test printer",
195+
},
196+
goipp.TextWithLang{
197+
Lang: "ru",
198+
Text: "тестовый принтер",
199+
},
200+
),
201+
},
202+
name: "printer-make-and-model",
203+
out: []string{"test printer"},
204+
},
205+
206+
{
207+
// "en" vs "ru", in the different order. "en" wins
208+
attrs: goipp.Attributes{
209+
goipp.MakeAttr("printer-make-and-model",
210+
goipp.TagTextLang,
211+
goipp.TextWithLang{
212+
Lang: "ru",
213+
Text: "тестовый принтер",
214+
},
215+
goipp.TextWithLang{
216+
Lang: "en",
217+
Text: "test printer",
218+
},
219+
),
220+
},
221+
name: "printer-make-and-model",
222+
out: []string{"test printer"},
223+
},
224+
225+
{
226+
// "EN" vs "RU" (in uppercase). "EN" wins
227+
attrs: goipp.Attributes{
228+
goipp.MakeAttr("printer-make-and-model",
229+
goipp.TagTextLang,
230+
goipp.TextWithLang{
231+
Lang: "EN",
232+
Text: "test printer",
233+
},
234+
goipp.TextWithLang{
235+
Lang: "RU",
236+
Text: "тестовый принтер",
237+
},
238+
),
239+
},
240+
name: "printer-make-and-model",
241+
out: []string{"test printer"},
242+
},
243+
244+
{
245+
// "EN" vs "RU", in the different order. "EN" wins
246+
attrs: goipp.Attributes{
247+
goipp.MakeAttr("printer-make-and-model",
248+
goipp.TagTextLang,
249+
goipp.TextWithLang{
250+
Lang: "RU",
251+
Text: "тестовый принтер",
252+
},
253+
goipp.TextWithLang{
254+
Lang: "EN",
255+
Text: "test printer",
256+
},
257+
),
258+
},
259+
name: "printer-make-and-model",
260+
out: []string{"test printer"},
261+
},
262+
263+
{
264+
// "en" vs "en-US". "en" wins
265+
attrs: goipp.Attributes{
266+
goipp.MakeAttr("printer-make-and-model",
267+
goipp.TagTextLang,
268+
goipp.TextWithLang{
269+
Lang: "en",
270+
Text: "test printer - en",
271+
},
272+
goipp.TextWithLang{
273+
Lang: "en-US",
274+
Text: "test printer - en-US",
275+
},
276+
),
277+
},
278+
name: "printer-make-and-model",
279+
out: []string{"test printer - en"},
280+
},
281+
282+
{
283+
// "en-US" vs "en-XXX". "en-US" wins
284+
attrs: goipp.Attributes{
285+
goipp.MakeAttr("printer-make-and-model",
286+
goipp.TagTextLang,
287+
goipp.TextWithLang{
288+
Lang: "en-US",
289+
Text: "test printer - US",
290+
},
291+
goipp.TextWithLang{
292+
Lang: "en-XXX",
293+
Text: "test printer - XXX",
294+
},
295+
),
296+
},
297+
name: "printer-make-and-model",
298+
out: []string{"test printer - US"},
299+
},
300+
301+
{
302+
// "en-XXX" vs "ru". "en-XXX" wins
303+
attrs: goipp.Attributes{
304+
goipp.MakeAttr("printer-make-and-model",
305+
goipp.TagTextLang,
306+
goipp.TextWithLang{
307+
Lang: "ru",
308+
Text: "тестовый принтер",
309+
},
310+
goipp.TextWithLang{
311+
Lang: "en-XXX",
312+
Text: "test printer",
313+
},
314+
),
315+
},
316+
name: "printer-make-and-model",
317+
out: []string{"test printer"},
318+
},
319+
320+
{
321+
// "de" vs "it". First occurrence wins
322+
attrs: goipp.Attributes{
323+
goipp.MakeAttr("printer-make-and-model",
324+
goipp.TagTextLang,
325+
goipp.TextWithLang{
326+
Lang: "de",
327+
Text: "test printer - de",
328+
},
329+
goipp.TextWithLang{
330+
Lang: "it",
331+
Text: "test printer - it",
332+
},
333+
),
334+
},
335+
name: "printer-make-and-model",
336+
out: []string{"test printer - de"},
337+
},
338+
339+
{
340+
// "de" vs "it" in reverse order.
341+
// First occurrence wins
342+
attrs: goipp.Attributes{
343+
goipp.MakeAttr("printer-make-and-model",
344+
goipp.TagTextLang,
345+
goipp.TextWithLang{
346+
Lang: "it",
347+
Text: "test printer - it",
348+
},
349+
goipp.TextWithLang{
350+
Lang: "de",
351+
Text: "test printer - de",
352+
},
353+
),
354+
},
355+
name: "printer-make-and-model",
356+
out: []string{"test printer - it"},
357+
},
358+
359+
{
360+
// Multiple values
361+
attrs: goipp.Attributes{
362+
goipp.MakeAttr("kind",
363+
goipp.TagKeyword,
364+
goipp.String("document"),
365+
goipp.String("envelope"),
366+
),
367+
},
368+
name: "kind",
369+
out: []string{"document", "envelope"},
370+
},
371+
}
372+
373+
for _, test := range tests {
374+
attrs := newIppAttrs(test.attrs)
375+
out := attrs.getStrings(test.name)
376+
377+
if !reflect.DeepEqual(test.out, out) {
378+
f := goipp.NewFormatter()
379+
f.Printf("ippAttrs.getStrings test failed:")
380+
381+
f.Printf("input:")
382+
f.SetIndent(4)
383+
f.FmtAttributes(test.attrs)
384+
f.SetIndent(0)
385+
386+
f.Printf("query: %s", test.name)
387+
f.Printf("expected: %v", test.out)
388+
f.Printf("presemt: %v", out)
389+
390+
t.Errorf("%s", f.String())
96391
}
97392
}
98393
}

0 commit comments

Comments
 (0)