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

Limit signature duration based on max_age_secs. #348

Merged
merged 3 commits into from
Sep 17, 2019

Conversation

twifkak
Copy link
Member

@twifkak twifkak commented Sep 17, 2019

This uses the new metadata field returned from the transformer library,
which is based on the presence of any inline amp-scripts.

@Gregable Note for review to ignore the first commit; it's merely the "diffbase" of #347.

cc @choumx

@@ -477,11 +477,17 @@ func (this *Signer) serveSignedExchange(resp http.ResponseWriter, fetchResp *htt
if err != nil {
util.NewHTTPError(http.StatusInternalServerError, "Error building validity href: ", err).LogAndRespond(resp)
}
// Expires - Date must be <= 604800 seconds, per
// https://tools.ietf.org/html/draft-yasskin-httpbis-origin-signed-exchanges-impl-00#section-3.5.
duration := 7*24*time.Hour
Copy link
Member

Choose a reason for hiding this comment

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

suggest spaces around * for readability.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, ran go fmt on these two files, which fixed this.

// Expires - Date must be <= 604800 seconds, per
// https://tools.ietf.org/html/draft-yasskin-httpbis-origin-signed-exchanges-impl-00#section-3.5.
duration := 7*24*time.Hour
println("max-age=", metadata.MaxAgeSecs)
Copy link
Member

Choose a reason for hiding this comment

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

debug statement?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, removed.

This uses the new metadata field returned from the transformer library,
which is based on the presence of any inline amp-scripts.
@twifkak twifkak merged commit 9b86ec4 into ampproject:master Sep 17, 2019
@twifkak twifkak deleted the limit_duration branch September 17, 2019 16:06
banaag pushed a commit to banaag/amppackager that referenced this pull request Oct 23, 2019
This uses the new metadata field returned from the transformer library,
which is based on the presence of any inline amp-scripts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants