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

Issue 56660043: Encode TLS messages

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 3 months ago by ekasper
Modified:
10 years, 3 months ago
Reviewers:
Eran
CC:
ctlog-opensource-review_google.com
Visibility:
Public.

Description

Encode TLS messages

Patch Set 1 #

Patch Set 2 : fix Eran's script #

Total comments: 2

Patch Set 3 : comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+231 lines, -27 lines) Patch
M src/python/ct/client/tls_message.py View 1 2 4 chunks +148 lines, -3 lines 0 comments Download
M src/python/ct/client/tls_message_test.py View 3 chunks +69 lines, -3 lines 0 comments Download
M src/python/ct/client/tools/scan.py View 1 chunk +2 lines, -4 lines 0 comments Download
M src/python/ct/client/tools/verify_single_proof.py View 1 3 chunks +12 lines, -17 lines 0 comments Download

Messages

Total messages: 4
ekasper
10 years, 3 months ago (2014-01-24 16:36:32 UTC) #1
Eran
Overall LGTM, see one comment below. I'd have liked to see the struct package being ...
10 years, 3 months ago (2014-01-27 16:46:40 UTC) #2
ekasper
On 2014/01/27 16:46:40, Eran wrote: > Overall LGTM, see one comment below. > I'd have ...
10 years, 3 months ago (2014-01-28 19:03:55 UTC) #3
ekasper
10 years, 3 months ago (2014-01-28 19:04:05 UTC) #4
https://codereview.appspot.com/56660043/diff/20001/src/python/ct/client/tls_m...
File src/python/ct/client/tls_message.py (right):

https://codereview.appspot.com/56660043/diff/20001/src/python/ct/client/tls_m...
src/python/ct/client/tls_message.py:170: def get_result(self):
On 2014/01/27 16:46:40, Eran wrote:
> Nit: Name get_serialized or serialize or something similar? Or make it
private.

Renamed to get_serialized_result. Since write() is public, this has to be
public, too. Using encode() to serialize a single message probably covers most
uses but I think it's nice to have the manual option around, too.
Sign in to reply to this message.

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