Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1892)

Issue 15520047: code review 15520047: go.crypto/ssh: Implement CertTime to fix an issue with ... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 1 month ago by jmpittman
Modified:
8 years, 1 month ago
Reviewers:
CC:
agl1, dfc, jpsugar, hanwen-google, golang-dev
Visibility:
Public.

Description

go.crypto/ssh: Implement CertTime to properly handle the "infinite" time value ^0, which would become negative when expressed as int64.

Patch Set 1 #

Patch Set 2 : diff -r 32844aa1ae54 https://code.google.com/p/go.crypto #

Patch Set 3 : diff -r 32844aa1ae54 https://code.google.com/p/go.crypto #

Total comments: 4

Patch Set 4 : diff -r 32844aa1ae54 https://code.google.com/p/go.crypto #

Total comments: 6

Patch Set 5 : diff -r 32844aa1ae54 https://code.google.com/p/go.crypto #

Patch Set 6 : diff -r 32844aa1ae54 https://code.google.com/p/go.crypto #

Total comments: 9

Patch Set 7 : diff -r 32844aa1ae54 https://code.google.com/p/go.crypto #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -9 lines) Patch
M ssh/certs.go View 1 2 3 4 5 6 4 chunks +27 lines, -5 lines 0 comments Download
M ssh/keys_test.go View 1 2 3 2 chunks +3 lines, -4 lines 0 comments Download

Messages

Total messages: 11
jmpittman
Hello agl@golang.org, dave@cheney.net (cc: golang-dev@googlegroups.com, hanwen@google.com, jpsugar@google.com), I'd like you to review this change to ...
8 years, 1 month ago (2013-10-22 21:36:11 UTC) #1
jpsugar
https://codereview.appspot.com/15520047/diff/40001/ssh/certs.go File ssh/certs.go (right): https://codereview.appspot.com/15520047/diff/40001/ssh/certs.go#newcode63 ssh/certs.go:63: return ct == 1<<64-1 Doesn't this result in an ...
8 years, 1 month ago (2013-10-22 21:45:35 UTC) #2
jmpittman
https://codereview.appspot.com/15520047/diff/40001/ssh/certs.go File ssh/certs.go (right): https://codereview.appspot.com/15520047/diff/40001/ssh/certs.go#newcode63 ssh/certs.go:63: return ct == 1<<64-1 I do not think it ...
8 years, 1 month ago (2013-10-23 00:19:01 UTC) #3
hanwen-google
what problem is this CL solving? https://codereview.appspot.com/15520047/diff/40002/ssh/certs.go File ssh/certs.go (right): https://codereview.appspot.com/15520047/diff/40002/ssh/certs.go#newcode55 ssh/certs.go:55: // handle time ...
8 years, 1 month ago (2013-10-23 05:03:28 UTC) #4
jmpittman
TLDR... 1. Conversion from uint64 to int64 to time.Time and then back had issues with ...
8 years, 1 month ago (2013-10-23 11:34:48 UTC) #5
hanwen-google
https://codereview.appspot.com/15520047/diff/40002/ssh/certs.go File ssh/certs.go (right): https://codereview.appspot.com/15520047/diff/40002/ssh/certs.go#newcode55 ssh/certs.go:55: // handle time values above what a time.Time can ...
8 years, 1 month ago (2013-10-23 15:32:49 UTC) #6
hanwen-google
Also, can you change the description to be more explicit? Say how you fixed what ...
8 years, 1 month ago (2013-10-23 15:36:44 UTC) #7
jmpittman
https://codereview.appspot.com/15520047/diff/40002/ssh/certs.go File ssh/certs.go (right): https://codereview.appspot.com/15520047/diff/40002/ssh/certs.go#newcode55 ssh/certs.go:55: // handle time values above what a time.Time can ...
8 years, 1 month ago (2013-10-23 16:30:33 UTC) #8
agl1
https://codereview.appspot.com/15520047/diff/160001/ssh/certs.go File ssh/certs.go (right): https://codereview.appspot.com/15520047/diff/160001/ssh/certs.go#newcode39 ssh/certs.go:39: maxUint64 = 1<<64 - 1 for future reference, there's ...
8 years, 1 month ago (2013-10-23 16:38:36 UTC) #9
jmpittman
https://codereview.appspot.com/15520047/diff/160001/ssh/certs.go File ssh/certs.go (right): https://codereview.appspot.com/15520047/diff/160001/ssh/certs.go#newcode39 ssh/certs.go:39: maxUint64 = 1<<64 - 1 That was my reasoning ...
8 years, 1 month ago (2013-10-23 16:41:05 UTC) #10
agl1
8 years, 1 month ago (2013-10-23 16:44:36 UTC) #11
*** Submitted as
https://code.google.com/p/go/source/detail?r=a7997f1dd284&repo=crypto ***

go.crypto/ssh: Implement CertTime to properly handle the "infinite" time
value ^0, which would become negative when expressed as int64.

R=agl, dave, jpsugar, hanwen
CC=golang-dev
https://codereview.appspot.com/15520047

Committer: Adam Langley <agl@golang.org>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b