Skip to content

Populate Date header if none supplied#2

Merged
azdagron merged 2 commits into
spacemonkeygo:masterfrom
writeas:patch-date
Jul 1, 2018
Merged

Populate Date header if none supplied#2
azdagron merged 2 commits into
spacemonkeygo:masterfrom
writeas:patch-date

Conversation

@thebaer

@thebaer thebaer commented Jul 1, 2018

Copy link
Copy Markdown
Contributor

By default, the Date header is included in the list of signed headers, though it might not exist on the request. This patch ensures the header is always there.

By default the Date header is included in the list of signed headers,
though it might not even exist on the request. This ensures the header
is added if it isn't already there.
Comment thread common.go Outdated
values = append(values, fmt.Sprintf("%s: %s", h, req.Host))
case "date":
if req.Header.Get(h) == "" {
req.Header.Set(h, time.Now().Format(time.RFC1123))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a well-formed Date header according to the HTTP RFC. HTTP dates are in RFC1123 format, except they are always represented in UTC time. Another caveat is that the timezone indication is not "UTC", but "GMT".

I'd suggest something like:

req.Header.Set(h, time.Now().UTC().Format(http.TimeFormat))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback! This is fixed now.

@azdagron azdagron left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks for the contribution.

@azdagron azdagron merged commit c76789e into spacemonkeygo:master Jul 1, 2018
@thebaer thebaer deleted the patch-date branch July 1, 2018 15:50
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

Successfully merging this pull request may close these issues.

2 participants