Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

# token HTTP-trailer { <HTTP-trailer> } #7

Open
raiph opened this issue Jun 22, 2020 · 2 comments
Open

# token HTTP-trailer { <HTTP-trailer> } #7

raiph opened this issue Jun 22, 2020 · 2 comments

Comments

@raiph
Copy link

raiph commented Jun 22, 2020

Posting this seemed like the lightest weight way to document, or at least leave a link tidbit, that covered what I found when I researched why you had the # token HTTP-trailer { <HTTP-trailer> } in your grammar, and to offer a related PR if you'd like one.

(I don't know http but had reason to browse this package. I found it disconcerting to see that commented out line. I guessed you had a good reason for it, and thought it somewhat unlikely that it was just something you hadn't gotten around to finishing. A few minutes later I got to the link I share below. I thought it quite plausible others might visit the issues to reassure themselves this package is "solid", and that also led me to feel that posting an issue was a reasonable thing to do.)

So, here is what I'll draw attention to; it distills (a random someone's summary of) the recent standing of http trailers, and puts it in the broader context of the SO it's in.

I'm thinking that one of the following three responses to me posting this issue would make sense (and of course you might have some other response). One is to just close this without comment. Later readers might see this issue and understand there's nothing to worry about, and all will be well. Another would be to close with whatever commentary you wish to add. And a third would be to want to add a # comment in the grammar, above the # token HTTP-trailer { <HTTP-trailer> } comment, or perhaps a mod to this package's README, presumably along the lines of:

Re: # token HTTP-trailer { <HTTP-trailer> }: See (link to the overall SO containing the comment I linked above)

I'd be happy to do a PR if you wish, after reading your guidance on whether to add a code comment in the grammar or prose in the README, or some other action.

@raiph
Copy link
Author

raiph commented Jun 22, 2020

Digging a little further, here's another link that might be appropriate instead, or as well, httpwg/http-core#18, at least to leave here for posterity, possibly also for inclusion in the README or as a code comment as outlined above.

@ugexe
Copy link
Owner

ugexe commented Jun 22, 2020

That comment was originally added here (this code was originally in zef). It kinda seems like I didn't get around to it.

I'd probably try adding a test case that includes trailers and tweaking something like
token HTTP-message { <start-line> [<header-field> <.CRLF>]* <.CRLF> <message-body>? } to
token HTTP-message { <start-line> [<header-field> <.CRLF>]* <.CRLF> <message-body>? <trailer-part> }
Of course the above likely needs an additional token(s), CRLF?, between those additions so it knows when the message-body ends and the trailers start... then again maybe chunked messages complicate the above. Its worth noting the existing trailer-part token.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants