Minor reorganization of HTTP-parsing spec/code

api.bs

@@ -1923,13 +1923,19 @@ if the user has opted out of collection of diagnostic data.
 set on a response requesting that the user agent invoke the
 <a method for=Attribution>saveImpression()</a> API.
 
-<pre class=example id=ex-save-impression-header>
-Save-Impression: conversion-sites=("advertiser.example"), conversion-callers=("intermediary.example"), histogram-index=2, match-value=12, lifetime-days=7
-</pre>
+<div class=example id=ex-save-impression-header>
+This is the HTTP equivalent of <a href=#ex-save-impression>the JavaScript `saveImpression` example</a>:
+<xmp highlight=http>
+Save-Impression: histogram-index=3, match-value=2, conversion-sites=("advertiser.example"), lifetime-days=7
+</xmp>
+</div>
 
 The following keys are defined, corresponding to the members of
 the {{AttributionImpressionOptions}} dictionary passed to
-<a method for=Attribution>saveImpression()</a>.
+<a method for=Attribution>saveImpression()</a>. Default values for omitted
+optional keys are treated the same way as the corresponding
+{{AttributionImpressionOptions}} field.
+
 
 <dl dfn-for=save-impression>
   <dt><dfn noexport><code>conversion-sites</code></dfn></dt>
@@ -1938,15 +1944,15 @@ the {{AttributionImpressionOptions}} dictionary passed to
     an [=structured header/inner list=] containing [=structured header/string|strings=].
     Each string value includes a domain name using A-labels only;
     [[RFC5890|Internationalized Domain Names]] therefore need to use [[RFC3492|punycode]].
-    This key is optional. If not supplied, an empty set is saved for [=impression/Conversion Sites=].
+    This key is optional.
   </dd>
   <dt><dfn noexport><code>conversion-callers</code></dfn></dt>
   <dd>
     Value of <a dict-member for=AttributionImpressionOptions>conversionCallers</a>,
     an [=structured header/inner list=] containing [=structured header/string|strings=].
     Each string value includes a domain name using A-labels only;
     [[RFC5890|Internationalized Domain Names]] therefore need to use [[RFC3492|punycode]].
-    This key is optional. If not supplied, an empty set is saved for [=impression/Conversion Callers=].
+    This key is optional.
   </dd>
   <dt><dfn noexport><code>histogram-index</code></dfn></dt>
   <dd>
@@ -1962,13 +1968,11 @@ the {{AttributionImpressionOptions}} dictionary passed to
   <dd>
     Value of <a dict-member for=AttributionImpressionOptions>matchValue</a>,
     an [=structured header/integer=] in the [=32-bit unsigned integer=] range. This key is optional.
-    If not supplied, a value of 0 is saved for [=impression/Match Value=].
   </dd>
   <dt><dfn noexport><code>lifetime-days</code></dfn></dt>
   <dd>
     Value of <a dict-member for=AttributionImpressionOptions>lifetimeDays</a>,
     a positive [=structured header/integer=]. This key is optional.
-    If not supplied, 30 days is saved for [=impression/Lifetime=].
   </dd>
 </dl>
 
@@ -1980,35 +1984,36 @@ To <dfn noexport>parse a `Save-Impression` header</dfn> given a [=header value=]
     with <var ignore>input_bytes</var> set to |input| and
     <var ignore>field_type</var> set to "`dictionary`".
 1.  If parsing failed, return an error.
-1.  If |dict|["<code>[=save-impression/histogram-index=]</code>"] does not [=map/exist=] or
-    is not an [=structured header/integer=] in the [=32-bit unsigned integer=] range, return an error.
-1.  Let |histogramIndex| be |dict|["<code>[=save-impression/histogram-index=]</code>"].
-1.  Let |conversionSites| be |dict|["<code>[=save-impression/conversion-sites=]</code>"]
-    [=map/with default=] an empty [=structured header/inner list=].
-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.
-1.  Let |conversionCallers| be |dict|["<code>[=save-impression/conversion-callers=]</code>"]
-    [=map/with default=] an empty [=structured header/inner list=].
-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.
-1.  Let |matchValue| be |dict|["<code>[=save-impression/match-value=]</code>"] [=map/with default=] 0.
-1.  If |matchValue| is not an [=structured header/integer=] in the [=32-bit unsigned integer=] range, return an error.
-1.  Let |lifetimeDays| be |dict|["<code>[=save-impression/lifetime-days=]</code>"] [=map/with default=] 30.
-1.  If |lifetimeDays| is not a positive [=structured header/integer=], return an error.
-1.  Clamp |lifetimeDays| to the [=32-bit unsigned integer=] range.
-1.  Let |priority| be |dict|["<code>[=save-impression/priority=]</code>"] [=map/with default=] 0.
-1.  If |priority| is not an [=structured header/integer=] in the [=32-bit signed integer=] range, return an error.
-1.  Return a new {{AttributionImpressionOptions}} with the following items:
+1.  Let |histogramIndex| be |dict|["<code>[=save-impression/histogram-index=]</code>"] [=map/with default=] `undefined`.
+1.  If |histogramIndex| is not an [=structured header/integer=] in the [=32-bit unsigned integer=] range, return an error.
+1.  Let |opts| be a new {{AttributionImpressionOptions}} with the following items:
      : {{AttributionImpressionOptions/histogramIndex}}
      :: |histogramIndex|
-     : {{AttributionImpressionOptions/matchValue}}
-     :: |matchValue|
-     : {{AttributionImpressionOptions/conversionSites}}
-     :: |conversionSites|
-     : {{AttributionImpressionOptions/conversionCallers}}
-     :: |conversionCallers|
-     : {{AttributionImpressionOptions/lifetimeDays}}
-     :: |lifetimeDays|
-     : {{AttributionImpressionOptions/priority}}
-     :: |priority|
+1.  If |dict|["<code>[=save-impression/conversion-sites=]</code>"] [=map/exists=]:
+     1.  Let |conversionSites| be its [=map/value=].
+     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.
+     1.  Set |opts|.{{AttributionImpressionOptions/conversionSites}} to |conversionSites|.
+1.  If |dict|["<code>[=save-impression/conversion-callers=]</code>"] [=map/exists=]:
+     1.  Let |conversionCallers| be its [=map/value=].
+     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.
+     1.  Set |opts|.{{AttributionImpressionOptions/conversionCallers}} to |conversionCallers|.
+1.  If |dict|["<code>[=save-impression/match-value=]</code>"] [=map/exists=]:
+     1.  Let |matchValue| be its [=map/value=].
+     1.  If |matchValue| is not an [=structured header/integer=] in the [=32-bit unsigned integer=] range, return an error.
+     1.  Set |opts|.{{AttributionImpressionOptions/matchValue}} to |matchValue|.
+1.  If |dict|["<code>[=save-impression/lifetime-days=]</code>"] [=map/exists=]:
+     1.  Let |lifetimeDays| be its [=map/value=].
+     1.  If |lifetimeDays| is not a positive [=structured header/integer=], return an error.
+     1.  Set |opts|.{{AttributionImpressionOptions/lifetimeDays}} to |lifetimeDays|.
+1.  If |dict|["<code>[=save-impression/priority=]</code>"] [=map/exists=]:
+     1.  Let |priority| be its [=map/value=].
+     1.  If |priority| is not an [=structured header/integer=] in the [=32-bit signed integer=] range, return an error.
+     1.  Set |opts|.{{AttributionImpressionOptions/priority}} to |priority|.
+1.  Return |opts|.
 
 </div>
 

impl/src/http.test.ts

@@ -34,11 +34,11 @@ runTests([
     input: "histogram-index=123",
     expected: {
       histogramIndex: 123,
-      matchValue: 0,
-      conversionSites: [],
-      conversionCallers: [],
-      lifetimeDays: 30,
-      priority: 0,
+      matchValue: undefined,
+      conversionSites: undefined,
+      conversionCallers: undefined,
+      lifetimeDays: undefined,
+      priority: undefined,
     },
   },
 
@@ -60,11 +60,11 @@ runTests([
     input: "histogram-index=1, conversion-sites=(), conversion-callers=()",
     expected: {
       histogramIndex: 1,
-      matchValue: 0,
+      matchValue: undefined,
       conversionSites: [],
       conversionCallers: [],
-      lifetimeDays: 30,
-      priority: 0,
+      lifetimeDays: undefined,
+      priority: undefined,
     },
   },
 
@@ -78,11 +78,11 @@ runTests([
     input: "histogram-index=4294967295",
     expected: {
       histogramIndex: 4294967295,
-      matchValue: 0,
-      conversionSites: [],
-      conversionCallers: [],
-      lifetimeDays: 30,
-      priority: 0,
+      matchValue: undefined,
+      conversionSites: undefined,
+      conversionCallers: undefined,
+      lifetimeDays: undefined,
+      priority: undefined,
     },
   },
   {
@@ -120,10 +120,10 @@ runTests([
     expected: {
       histogramIndex: 1,
       matchValue: 4294967295,
-      conversionSites: [],
-      conversionCallers: [],
-      lifetimeDays: 30,
-      priority: 0,
+      conversionSites: undefined,
+      conversionCallers: undefined,
+      lifetimeDays: undefined,
+      priority: undefined,
     },
   },
   {
@@ -145,15 +145,15 @@ runTests([
   },
   { name: "lifetime-days-zero", input: "lifetime-days=0, histogram-index=1" },
   {
-    name: "valid-lifetime-days-maximal-clamped",
+    name: "valid-lifetime-days-maximal",
     input: "lifetime-days=999999999999999, histogram-index=1",
     expected: {
       histogramIndex: 1,
-      matchValue: 0,
-      conversionSites: [],
-      conversionCallers: [],
-      lifetimeDays: 4294967295,
-      priority: 0,
+      matchValue: undefined,
+      conversionSites: undefined,
+      conversionCallers: undefined,
+      lifetimeDays: 999999999999999,
+      priority: undefined,
     },
   },
 
@@ -164,10 +164,10 @@ runTests([
     input: "priority=2147483647, histogram-index=1",
     expected: {
       histogramIndex: 1,
-      matchValue: 0,
-      conversionSites: [],
-      conversionCallers: [],
-      lifetimeDays: 30,
+      matchValue: undefined,
+      conversionSites: undefined,
+      conversionCallers: undefined,
+      lifetimeDays: undefined,
       priority: 2147483647,
     },
   },
@@ -176,10 +176,10 @@ runTests([
     input: "priority=-2147483648, histogram-index=1",
     expected: {
       histogramIndex: 1,
-      matchValue: 0,
-      conversionSites: [],
-      conversionCallers: [],
-      lifetimeDays: 30,
+      matchValue: undefined,
+      conversionSites: undefined,
+      conversionCallers: undefined,
+      lifetimeDays: undefined,
       priority: -2147483648,
     },
   },

impl/src/http.ts

@@ -1,6 +1,6 @@
 import type { AttributionImpressionOptions } from "./index";
 
-import type { Dictionary } from "structured-headers";
+import type { BareItem, Dictionary, Item } from "structured-headers";
 
 import { parseDictionary } from "structured-headers";
 
@@ -9,8 +9,49 @@ const MAX_UINT32: number = 4294967295;
 const MIN_INT32: number = -2147483648;
 const MAX_INT32: number = 2147483647;
 
-function parseInnerListOfSites(dict: Dictionary, key: string): string[] {
-  const [values] = dict.get(key) ?? [[]];
+function get(dict: Dictionary, key: string): BareItem | Item[] | undefined {
+  const [value] = dict.get(key) ?? [undefined];
+  return value;
+}
+
+function getInteger(dict: Dictionary, key: string): number | undefined {
+  const value = get(dict, key);
+  if (value === undefined) {
+    return value;
+  }
+
+  if (typeof value !== "number" || !Number.isInteger(value)) {
+    throw new TypeError(`${key} must be an integer`);
+  }
+
+  return value;
+}
+
+function get32BitUnsignedInteger(
+  dict: Dictionary,
+  key: string,
+): number | undefined {
+  const value = getInteger(dict, key);
+  if (value === undefined) {
+    return value;
+  }
+
+  if (value < 0 || value > MAX_UINT32) {
+    throw new RangeError(`${key} must be in the 32-bit unsigned range`);
+  }
+
+  return value;
+}
+
+function parseInnerListOfSites(
+  dict: Dictionary,
+  key: string,
+): string[] | undefined {
+  const values = get(dict, key);
+  if (values === undefined) {
+    return values;
+  }
+
   if (!Array.isArray(values)) {
     throw new TypeError(`${key} must be an inner list`);
   }
@@ -30,64 +71,30 @@ export function parseSaveImpressionHeader(
 ): AttributionImpressionOptions {
   const dict = parseDictionary(input);
 
-  const [histogramIndex] = dict.get("histogram-index") ?? [undefined];
-  if (
-    typeof histogramIndex !== "number" ||
-    !Number.isInteger(histogramIndex) ||
-    histogramIndex < 0 ||
-    histogramIndex > MAX_UINT32
-  ) {
-    throw new RangeError(
-      "histogram-index must be an integer in the 32-bit unsigned range",
-    );
+  const histogramIndex = get32BitUnsignedInteger(dict, "histogram-index");
+  if (histogramIndex === undefined) {
+    throw new TypeError("histogram-index is required");
   }
 
-  const conversionSites = parseInnerListOfSites(dict, "conversion-sites");
-  const conversionCallers = parseInnerListOfSites(dict, "conversion-callers");
+  const opts: AttributionImpressionOptions = { histogramIndex };
 
-  const [matchValue] = dict.get("match-value") ?? [0];
-  if (
-    typeof matchValue !== "number" ||
-    !Number.isInteger(matchValue) ||
-    matchValue < 0 ||
-    matchValue > MAX_UINT32
-  ) {
-    throw new RangeError(
-      "match-value must be an integer in the 32-bit unsigned range",
-    );
-  }
+  opts.conversionSites = parseInnerListOfSites(dict, "conversion-sites");
+  opts.conversionCallers = parseInnerListOfSites(dict, "conversion-callers");
 
-  let [lifetimeDays] = dict.get("lifetime-days") ?? [30];
-  if (
-    typeof lifetimeDays !== "number" ||
-    !Number.isInteger(lifetimeDays) ||
-    lifetimeDays <= 0
-  ) {
-    throw new RangeError("lifetime-days must be a positive integer");
-  }
+  opts.matchValue = get32BitUnsignedInteger(dict, "match-value");
 
-  if (lifetimeDays > MAX_UINT32) {
-    lifetimeDays = MAX_UINT32;
+  opts.lifetimeDays = getInteger(dict, "lifetime-days");
+  if (opts.lifetimeDays !== undefined && opts.lifetimeDays <= 0) {
+    throw new RangeError("lifetime-days must be positive");
   }
 
-  const [priority] = dict.get("priority") ?? [0];
+  opts.priority = getInteger(dict, "priority");
   if (
-    typeof priority !== "number" ||
-    !Number.isInteger(priority) ||
-    priority < MIN_INT32 ||
-    priority > MAX_INT32
+    opts.priority !== undefined &&
+    (opts.priority < MIN_INT32 || opts.priority > MAX_INT32)
   ) {
-    throw new RangeError(
-      "priority must be an integer in the 32-bit signed range",
-    );
+    throw new RangeError("priority must be in the 32-bit signed range");
   }
 
-  return {
-    histogramIndex,
-    matchValue,
-    conversionSites,
-    conversionCallers,
-    lifetimeDays,
-    priority,
-  };
+  return opts;
 }