GaloisInc / daedalus

The Daedalus data description language
BSD 3-Clause "New" or "Revised" License
63 stars 11 forks source link

Remove quoted content-length #327

Closed kenballus closed 1 year ago

kenballus commented 1 year ago

When I try to feed the generated examples to my parsers, they complain because the Content-Length values are quoted. HTTP.ddl says this:

      -- NOTE: the specification permits all field values to be
      -- either tokens or quoted strings. This handles both. The
      -- specification also states that a Content-Length field is
      -- valid if it is a comma-separated list of numbers, all of
      -- which take the same value. We also handle that case here.

The relevant portion of the RFC says this:

Many fields (such as Content-Type, defined in Section 8.3) use a common syntax for parameters that allows both unquoted (token) and quoted (quoted-string) syntax for a parameter value (Section 5.6.6). Use of common syntax allows recipients to reuse existing parser components. When allowing both forms, the meaning of a parameter value ought to be the same whether it was received as a token or a quoted string.

As far as I can tell, this isn't a universal statement about all field-values; it's just saying that some headers, including Content-Type, can have either quoted or unquoted values. My understanding is that Content-Length can't have a quoted value, so this PR removes a couple of MaybeQuoteds.

yav commented 1 year ago

@kenballus the spec is not very clear, but I think I agree with you is that the intention is that there's no quotes here. My guess is that the quotes are allowed in places where the grammar says token, and in this case they explicitly say a bunch of digits.

It's not hugely important, but could you edit the PR to remove the parens around the PositiveNum64, as this would be the idiomatic way to write it, so: SepBy1 PositiveNum { ... }