1430713 - Make sure we comply with the latest server-timing spec

Status: RESOLVED WONTFIX

(Core :: Networking: HTTP, enhancement, P3)

Description

[Kershaw Chang [:kershaw]]

The spec says: "User agents MUST ignore extraneous characters found after a server-timing-param-value but before the next server-timing-param and before the end of the current server-timing-metric.".

Our current code doesn't ignore extraneous characters after the current server-timing-metric.
For example, for the case "metric==   \"\"foo;dur=123.4", our code produces {"metric", "123.4", ""}. The correct result should be {"metric", "0", ""}.

Comment 1

[Kershaw Chang [:kershaw]]

Attachment: patch

Summary:
This patch's focus is on the case when we found extraneous characters after the metric-name. If found, we can just stop parsing.
I'd like to use the two examples below to explain this patch.

Example 1: "metric==   \"\"foo;dur=123.4"
In this case, the first ParsedHeaderPair would be {"metric", "=   \"\"foo"}.
We can easily know by checking if the |mValue| is null or not. If not, this means we found '=' after the metric-name.

Example 2: "metric   \"\"foo ;dur=123.4"
For this case, I modified the function |ParseNameAndValue| to return the param name before '"', which is "metric   \"\"foo". And then check if we can find any invalid token character in "metric   \"\"foo". If yes, we can ignore the characters after the invalid character.

Please take a look. Thanks.

Comment 2

[Dragana Damjanovic [:dragana]]

(In reply to Kershaw Chang [:kershaw] from comment #0)
> The spec says: "User agents MUST ignore extraneous characters found after a
> server-timing-param-value but before the next server-timing-param and before
> the end of the current server-timing-metric.".
> 
> Our current code doesn't ignore extraneous characters after the current
> server-timing-metric.
> For example, for the case "metric==   \"\"foo;dur=123.4", our code produces
> {"metric", "123.4", ""}. The correct result should be {"metric", "0", ""}.

i think our code is according to the spec. The test and result ({"metric", "0", ""}) is copied from chrome. We contacted chrome to double check, so that we have the same behavior.


also see https://github.com/w3c/server-timing/issues/51

Comment 3

[Dragana Damjanovic [:dragana]]

(In reply to Kershaw Chang [:kershaw] from comment #1)
> Created attachment 8943277 [details] [diff] [review]
> patch
> 
> Summary:
> This patch's focus is on the case when we found extraneous characters after
> the metric-name. If found, we can just stop parsing.
> I'd like to use the two examples below to explain this patch.
> 
> Example 1: "metric==   \"\"foo;dur=123.4"
> In this case, the first ParsedHeaderPair would be {"metric", "=   \"\"foo"}.
> We can easily know by checking if the |mValue| is null or not. If not, this
> means we found '=' after the metric-name.
> 
> Example 2: "metric   \"\"foo ;dur=123.4"
> For this case, I modified the function |ParseNameAndValue| to return the
> param name before '"', which is "metric   \"\"foo". And then check if we can
> find any invalid token character in "metric   \"\"foo". If yes, we can
> ignore the characters after the invalid character.
> 
> Please take a look. Thanks.

The correct result for example 2 should be {"metric", "123.4", ""}.

Comment 4

[Kershaw Chang [:kershaw]]

Not sure about the current status now. Maybe this bug is already fixed.

Comment 5

[Kershaw Chang [:kershaw]]

This bug probably is WONTFIX.
Please reopen if you think firefox doesn't comply with the spec.