Changes for #152 and #143

[nightwatchcyber]

No description provided.

[nightwatchcyber]

@joker314 can you take a look also?

[joker314]

Looks really good! Summary of points contained in my review:

With the redirects, just an issue on wording to make clear whether .well-known should ever not be trusted.

With the ABNF, the optional fields don't appear to be optional anymore, and I have a question about ambiguous parsing and whether they matter.

[joker314]

If there is a circumstance where the top-level path redirects externally, and so does the .well-known path, then I think even though we can't trust the top-level path redirect we can nonetheless continue to trust the .well-known redirect.

This isn't the only reasonable interpretation of the amended text because one might think that a non-compliant redirect is an indicator of malicious activity.

I propose as follows: swap out "MUST NOT follow redirects that" for "MUST NOT follow that redirect if it" (and of course change the rest of the sentence to fit, and do the same for the "SHOULD".

[nightwatchcyber]

done

[joker314]

I like this! But it gets rid of the square brackets, which were made the Canonical and Preferred-Languages directives optional. This can be remedied by replacing the parentheses.

Another issue is that I think contact-field needs to have an "eol" suffixed to it.

Suggested change

	unsigned = *line (can-field eol) *line contact-field *line (lang-field eol) *line
	unsigned = *line [can-field eol] *line (contact-field eol) *line [lang-field eol] *line

I also have a question: if order doesn't matter, then two *lines can appear next to each other. If there are three immaterial lines then that can be distributed as (0,3); or (1,2); or (2,1); or (3,0). There is more than one way to parse a security.txt file, making the grammar ambiguous. I'm not sure if this really matters though, given that a human programmer is going to have to transcribe the ABNF comment into a programming language or write out a lot of BNF, so they will likely be able to prevent any ambiguous parsing.

If we did want to avoid it, though, I think it would be fairly easy: just move the *lines into parentheses so that they have to be in front of a material directive. We'd then need a *line at the end. This might make things a tad more confusing so it depends on whether it's worth it.

A stylistic question: do you think it looks prettier with a space after the semicolon (the one which denotes the start of an ABNF comment?

[nightwatchcyber]

You are correct regarding "[]" vs "()", I am fixing that

re: *line - I think we are relaying on a human programmer anyway because of the order comment, so I would rather leave it the way it is

Regarding the style, space makes sense - I will add it

Thank you again

[joker314]

That was fast! The ABNF looks good, but there's a missing set of parentheses. I think there's a missing indefinite article in the paragraph bit of the PR.

[joker314]

The "contact-field eol" ought to be in parenthesis to ensure that there is always a line break after the Contact directive.

Suggested change

	unsigned = *line [can-field eol] *line contact-field eol *line [lang-field eol] *line
	unsigned = *line [can-field eol] *line (contact-field eol) *line [lang-field eol] *line

---

I thought I'd quickly also show what disambiguating the line would look like (though I agree a human programmer could probably figure it out regardless):

unsigned-begin         =  [*line can-field eol] (*line contact-field eol) [*line lang-field eol] ; order unimportant
unsigned               =  unsigned-begin *line

I don't think it's that much more confusing now that I see it's just moving the *lines to be inside the brackets. Though this is not too important, as discussed above.

[nightwatchcyber]

done

[joker314]

Maybe

Suggested change

	but SHOULD follow such redirect within the same domain name
	but SHOULD follow such a redirect within the same domain name

or

Suggested change

	but SHOULD follow such redirect within the same domain name
	but SHOULD follow that redirect within the same domain name

but looks good!

[nightwatchcyber]

Done