Skip to content

Commit 6acfc7b

Browse files
apasel422apasel422
committed
Remove redundant defaulting from Save-Impression HTTP parsing
1 parent a1310f0 commit 6acfc7b

File tree

3 files changed

+99
-93
lines changed

3 files changed

+99
-93
lines changed

‎api.bs‎

Lines changed: 34 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1932,7 +1932,10 @@ Save-Impression: histogram-index=3, match-value=2, conversion-sites=("advertiser
19321932

19331933
The following keys are defined, corresponding to the members of
19341934
the {{AttributionImpressionOptions}} dictionary passed to
1935-
<a method for=Attribution>saveImpression()</a>.
1935+
<a method for=Attribution>saveImpression()</a>. Default values for omitted
1936+
optional keys are populated the same way as the corresponding
1937+
{{AttributionImpressionOptions}} field.
1938+
19361939

19371940
<dl dfn-for=save-impression>
19381941
<dt><dfn noexport><code>conversion-sites</code></dfn></dt>
@@ -1941,15 +1944,15 @@ the {{AttributionImpressionOptions}} dictionary passed to
19411944
an [=structured header/inner list=] containing [=structured header/string|strings=].
19421945
Each string value includes a domain name using A-labels only;
19431946
[[RFC5890|Internationalized Domain Names]] therefore need to use [[RFC3492|punycode]].
1944-
This key is optional. If not supplied, an empty set is saved for [=impression/Conversion Sites=].
1947+
This key is optional.
19451948
</dd>
19461949
<dt><dfn noexport><code>conversion-callers</code></dfn></dt>
19471950
<dd>
19481951
Value of <a dict-member for=AttributionImpressionOptions>conversionCallers</a>,
19491952
an [=structured header/inner list=] containing [=structured header/string|strings=].
19501953
Each string value includes a domain name using A-labels only;
19511954
[[RFC5890|Internationalized Domain Names]] therefore need to use [[RFC3492|punycode]].
1952-
This key is optional. If not supplied, an empty set is saved for [=impression/Conversion Callers=].
1955+
This key is optional.
19531956
</dd>
19541957
<dt><dfn noexport><code>histogram-index</code></dfn></dt>
19551958
<dd>
@@ -1965,13 +1968,11 @@ the {{AttributionImpressionOptions}} dictionary passed to
19651968
<dd>
19661969
Value of <a dict-member for=AttributionImpressionOptions>matchValue</a>,
19671970
an [=structured header/integer=] in the [=32-bit unsigned integer=] range. This key is optional.
1968-
If not supplied, a value of 0 is saved for [=impression/Match Value=].
19691971
</dd>
19701972
<dt><dfn noexport><code>lifetime-days</code></dfn></dt>
19711973
<dd>
19721974
Value of <a dict-member for=AttributionImpressionOptions>lifetimeDays</a>,
19731975
a positive [=structured header/integer=]. This key is optional.
1974-
If not supplied, 30 days is saved for [=impression/Lifetime=].
19751976
</dd>
19761977
</dl>
19771978

@@ -1983,35 +1984,36 @@ To <dfn noexport>parse a `Save-Impression` header</dfn> given a [=header value=]
19831984
with <var ignore>input_bytes</var> set to |input| and
19841985
<var ignore>field_type</var> set to "`dictionary`".
19851986
1. If parsing failed, return an error.
1986-
1. If |dict|["<code>[=save-impression/histogram-index=]</code>"] does not [=map/exist=] or
1987-
is not an [=structured header/integer=] in the [=32-bit unsigned integer=] range, return an error.
1988-
1. Let |histogramIndex| be |dict|["<code>[=save-impression/histogram-index=]</code>"].
1989-
1. Let |conversionSites| be |dict|["<code>[=save-impression/conversion-sites=]</code>"]
1990-
[=map/with default=] an empty [=structured header/inner list=].
1991-
1. If |conversionSites| is not an [=structured header/inner list=], or if any of |conversionSites|' [=list/items=] is not a [=structured header/string=], return an error.
1992-
1. Let |conversionCallers| be |dict|["<code>[=save-impression/conversion-callers=]</code>"]
1993-
[=map/with default=] an empty [=structured header/inner list=].
1994-
1. If |conversionCallers| is not an [=structured header/inner list=], or if any of |conversionCallers|' [=list/items=] is not a [=structured header/string=], return an error.
1995-
1. Let |matchValue| be |dict|["<code>[=save-impression/match-value=]</code>"] [=map/with default=] 0.
1996-
1. If |matchValue| is not an [=structured header/integer=] in the [=32-bit unsigned integer=] range, return an error.
1997-
1. Let |lifetimeDays| be |dict|["<code>[=save-impression/lifetime-days=]</code>"] [=map/with default=] 30.
1998-
1. If |lifetimeDays| is not a positive [=structured header/integer=], return an error.
1999-
1. Clamp |lifetimeDays| to the [=32-bit unsigned integer=] range.
2000-
1. Let |priority| be |dict|["<code>[=save-impression/priority=]</code>"] [=map/with default=] 0.
2001-
1. If |priority| is not an [=structured header/integer=] in the [=32-bit signed integer=] range, return an error.
2002-
1. Return a new {{AttributionImpressionOptions}} with the following items:
1987+
1. Let |histogramIndex| be |dict|["<code>[=save-impression/histogram-index=]</code>"] [=map/with default=] `undefined`.
1988+
1. If |histogramIndex| is not an [=structured header/integer=] in the [=32-bit unsigned integer=] range, return an error.
1989+
1. Let |opts| be a new {{AttributionImpressionOptions}} with the following items:
20031990
: {{AttributionImpressionOptions/histogramIndex}}
20041991
:: |histogramIndex|
2005-
: {{AttributionImpressionOptions/matchValue}}
2006-
:: |matchValue|
2007-
: {{AttributionImpressionOptions/conversionSites}}
2008-
:: |conversionSites|
2009-
: {{AttributionImpressionOptions/conversionCallers}}
2010-
:: |conversionCallers|
2011-
: {{AttributionImpressionOptions/lifetimeDays}}
2012-
:: |lifetimeDays|
2013-
: {{AttributionImpressionOptions/priority}}
2014-
:: |priority|
1992+
1. If |dict|["<code>[=save-impression/conversion-sites=]</code>"] [=map/exists=]:
1993+
1. Let |conversionSites| be its [=map/value=].
1994+
1. If |conversionSites| is not an [=structured header/inner list=], or if any of
1995+
|conversionSites|' [=list/items=] is not a [=structured header/string=],
1996+
return an error.
1997+
1. Set |opts|.{{AttributionImpressionOptions/conversionSites}} to |conversionSites|.
1998+
1. If |dict|["<code>[=save-impression/conversion-callers=]</code>"] [=map/exists=]:
1999+
1. Let |conversionCallers| be its [=map/value=].
2000+
1. If |conversionCallers| is not an [=structured header/inner list=], or if any of
2001+
|conversionCallers|' [=list/items=] is not a [=structured header/string=],
2002+
return an error.
2003+
1. Set |opts|.{{AttributionImpressionOptions/conversionCallers}} to |conversionCallers|.
2004+
1. If |dict|["<code>[=save-impression/match-value=]</code>"] [=map/exists=]:
2005+
1. Let |matchValue| be its [=map/value=].
2006+
1. If |matchValue| is not an [=structured header/integer=] in the [=32-bit unsigned integer=] range, return an error.
2007+
1. Set |opts|.{{AttributionImpressionOptions/matchValue}} to |matchValue|.
2008+
1. If |dict|["<code>[=save-impression/lifetime-days=]</code>"] [=map/exists=]:
2009+
1. Let |lifetimeDays| be its [=map/value=].
2010+
1. If |lifetimeDays| is not a positive [=structured header/integer=], return an error.
2011+
1. Set |opts|.{{AttributionImpressionOptions/lifetimeDays}} to |lifetimeDays|.
2012+
1. If |dict|["<code>[=save-impression/priority=]</code>"] [=map/exists=]:
2013+
1. Let |priority| be its [=map/value=].
2014+
1. If |priority| is not an [=structured header/integer=] in the [=32-bit signed integer=] range, return an error.
2015+
1. Set |opts|.{{AttributionImpressionOptions/priority}} to |priority|.
2016+
1. Return |opts|.
20152017

20162018
</div>
20172019

‎impl/src/http.test.ts‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@ runTests([
3434
input: "histogram-index=123",
3535
expected: {
3636
histogramIndex: 123,
37-
matchValue: 0,
38-
conversionSites: [],
39-
conversionCallers: [],
40-
lifetimeDays: 30,
41-
priority: 0,
37+
matchValue: undefined,
38+
conversionSites: undefined,
39+
conversionCallers: undefined,
40+
lifetimeDays: undefined,
41+
priority: undefined,
4242
},
4343
},
4444

@@ -60,11 +60,11 @@ runTests([
6060
input: "histogram-index=1, conversion-sites=(), conversion-callers=()",
6161
expected: {
6262
histogramIndex: 1,
63-
matchValue: 0,
63+
matchValue: undefined,
6464
conversionSites: [],
6565
conversionCallers: [],
66-
lifetimeDays: 30,
67-
priority: 0,
66+
lifetimeDays: undefined,
67+
priority: undefined,
6868
},
6969
},
7070

@@ -78,11 +78,11 @@ runTests([
7878
input: "histogram-index=4294967295",
7979
expected: {
8080
histogramIndex: 4294967295,
81-
matchValue: 0,
82-
conversionSites: [],
83-
conversionCallers: [],
84-
lifetimeDays: 30,
85-
priority: 0,
81+
matchValue: undefined,
82+
conversionSites: undefined,
83+
conversionCallers: undefined,
84+
lifetimeDays: undefined,
85+
priority: undefined,
8686
},
8787
},
8888
{
@@ -120,10 +120,10 @@ runTests([
120120
expected: {
121121
histogramIndex: 1,
122122
matchValue: 4294967295,
123-
conversionSites: [],
124-
conversionCallers: [],
125-
lifetimeDays: 30,
126-
priority: 0,
123+
conversionSites: undefined,
124+
conversionCallers: undefined,
125+
lifetimeDays: undefined,
126+
priority: undefined,
127127
},
128128
},
129129
{
@@ -145,15 +145,15 @@ runTests([
145145
},
146146
{ name: "lifetime-days-zero", input: "lifetime-days=0, histogram-index=1" },
147147
{
148-
name: "valid-lifetime-days-maximal-clamped",
148+
name: "valid-lifetime-days-maximal",
149149
input: "lifetime-days=999999999999999, histogram-index=1",
150150
expected: {
151151
histogramIndex: 1,
152-
matchValue: 0,
153-
conversionSites: [],
154-
conversionCallers: [],
155-
lifetimeDays: 4294967295,
156-
priority: 0,
152+
matchValue: undefined,
153+
conversionSites: undefined,
154+
conversionCallers: undefined,
155+
lifetimeDays: 999999999999999,
156+
priority: undefined,
157157
},
158158
},
159159

@@ -164,10 +164,10 @@ runTests([
164164
input: "priority=2147483647, histogram-index=1",
165165
expected: {
166166
histogramIndex: 1,
167-
matchValue: 0,
168-
conversionSites: [],
169-
conversionCallers: [],
170-
lifetimeDays: 30,
167+
matchValue: undefined,
168+
conversionSites: undefined,
169+
conversionCallers: undefined,
170+
lifetimeDays: undefined,
171171
priority: 2147483647,
172172
},
173173
},
@@ -176,10 +176,10 @@ runTests([
176176
input: "priority=-2147483648, histogram-index=1",
177177
expected: {
178178
histogramIndex: 1,
179-
matchValue: 0,
180-
conversionSites: [],
181-
conversionCallers: [],
182-
lifetimeDays: 30,
179+
matchValue: undefined,
180+
conversionSites: undefined,
181+
conversionCallers: undefined,
182+
lifetimeDays: undefined,
183183
priority: -2147483648,
184184
},
185185
},

‎impl/src/http.ts‎

Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,15 @@ const MAX_UINT32: number = 4294967295;
99
const MIN_INT32: number = -2147483648;
1010
const MAX_INT32: number = 2147483647;
1111

12-
function parseInnerListOfSites(dict: Dictionary, key: string): string[] {
13-
const [values] = dict.get(key) ?? [[]];
12+
function parseInnerListOfSites(
13+
dict: Dictionary,
14+
key: string,
15+
): string[] | undefined {
16+
const [values] = dict.get(key) ?? [undefined];
17+
if (values === undefined) {
18+
return values;
19+
}
20+
1421
if (!Array.isArray(values)) {
1522
throw new TypeError(`${key} must be an inner list`);
1623
}
@@ -42,52 +49,49 @@ export function parseSaveImpressionHeader(
4249
);
4350
}
4451

45-
const conversionSites = parseInnerListOfSites(dict, "conversion-sites");
46-
const conversionCallers = parseInnerListOfSites(dict, "conversion-callers");
52+
const opts: AttributionImpressionOptions = { histogramIndex };
53+
54+
opts.conversionSites = parseInnerListOfSites(dict, "conversion-sites");
55+
opts.conversionCallers = parseInnerListOfSites(dict, "conversion-callers");
4756

48-
const [matchValue] = dict.get("match-value") ?? [0];
57+
const [matchValue] = dict.get("match-value") ?? [undefined];
4958
if (
50-
typeof matchValue !== "number" ||
51-
!Number.isInteger(matchValue) ||
52-
matchValue < 0 ||
53-
matchValue > MAX_UINT32
59+
matchValue !== undefined &&
60+
(typeof matchValue !== "number" ||
61+
!Number.isInteger(matchValue) ||
62+
matchValue < 0 ||
63+
matchValue > MAX_UINT32)
5464
) {
5565
throw new RangeError(
5666
"match-value must be an integer in the 32-bit unsigned range",
5767
);
5868
}
69+
opts.matchValue = matchValue;
5970

60-
let [lifetimeDays] = dict.get("lifetime-days") ?? [30];
71+
const [lifetimeDays] = dict.get("lifetime-days") ?? [undefined];
6172
if (
62-
typeof lifetimeDays !== "number" ||
63-
!Number.isInteger(lifetimeDays) ||
64-
lifetimeDays <= 0
73+
lifetimeDays !== undefined &&
74+
(typeof lifetimeDays !== "number" ||
75+
!Number.isInteger(lifetimeDays) ||
76+
lifetimeDays <= 0)
6577
) {
6678
throw new RangeError("lifetime-days must be a positive integer");
6779
}
80+
opts.lifetimeDays = lifetimeDays;
6881

69-
if (lifetimeDays > MAX_UINT32) {
70-
lifetimeDays = MAX_UINT32;
71-
}
72-
73-
const [priority] = dict.get("priority") ?? [0];
82+
const [priority] = dict.get("priority") ?? [undefined];
7483
if (
75-
typeof priority !== "number" ||
76-
!Number.isInteger(priority) ||
77-
priority < MIN_INT32 ||
78-
priority > MAX_INT32
84+
priority !== undefined &&
85+
(typeof priority !== "number" ||
86+
!Number.isInteger(priority) ||
87+
priority < MIN_INT32 ||
88+
priority > MAX_INT32)
7989
) {
8090
throw new RangeError(
8191
"priority must be an integer in the 32-bit signed range",
8292
);
8393
}
94+
opts.priority = priority;
8495

85-
return {
86-
histogramIndex,
87-
matchValue,
88-
conversionSites,
89-
conversionCallers,
90-
lifetimeDays,
91-
priority,
92-
};
96+
return opts;
9397
}

0 commit comments

Comments
 (0)