Sessions and Connections #1
Conversation
There was a problem hiding this comment.
@akarl: If you see yourself providing more changes (regarding my comments) in the near future I'm all OK with merging this. If you think this will be stale for a week or two I can leave this PR as is and we merge when we've provided more coded addressing my comments. What's your opinion?
| fmt.Println("!!!", err.Error()) | ||
| } | ||
|
|
||
| /* |
There was a problem hiding this comment.
Wouldn't hur to keep this comment somewhere until we've figured out a way to handle commands and their namespaces.
examples/simple-server/main.go
Outdated
| MaxSessionLength: 10 * time.Minute, | ||
| Addr: ":4701", | ||
| Handler: RouteMessage, | ||
| Greeting: Greet, |
There was a problem hiding this comment.
Nice idea to have this as a separate field instead of a handler!
examples/simple-server/main.go
Outdated
| if len(rawCerts) != 1 { | ||
| return errors.New("dind't find one single client ceritficate") | ||
| } | ||
| func Greet(s *eppserver.Session) ([]byte, error) { |
There was a problem hiding this comment.
This won't work so I don't think it's a good idea to keep this as a default implementation of the greeter.
examples/simple-server/main.go
Outdated
| } | ||
|
|
||
| _, ok := c.PublicKey.(*rsa.PublicKey) | ||
| for _, xpath := range xpaths { | ||
| if xpath == "/epp/command/login" { |
There was a problem hiding this comment.
This must be handled before even accepting the connection so this should not be a part of the router/message handler.
| @@ -0,0 +1,69 @@ | |||
| package eppserver | |||
| // ReadMessage reads one full message from r. | ||
| func ReadMessage(conn net.Conn) ([]byte, error) { | ||
| // https://tools.ietf.org/html/rfc5734#section-4 | ||
| var totalSize uint32 = 0 |
There was a problem hiding this comment.
I haven't provided it yet but my default golangci-lint configuration doesn't like inits with default values.
| @@ -11,7 +11,7 @@ func (s *Server) GreetResponse() ws.EppType { | |||
| return ws.EppType{ | |||
| Greeting: &ws.GreetingType{ | |||
| SvID: ws.SIDType{ | |||
| Id: ws.ClIDType(s.ServerID), | |||
| Id: ws.ClIDType("asdb"), | |||
There was a problem hiding this comment.
I know this is WIP and all but this just looks bad, maybe a better default temporary value? :)
| func (s *Server) processConnection(conn *tls.Conn) { | ||
| s.wg.Add(1) | ||
| func (s *Server) startSession(conn net.Conn, tlsConfig *tls.Config) { | ||
| // Initialize tls. |
| idleTimeout := time.After(s.IdleTimeout) | ||
|
|
||
| for { | ||
| select { |
There was a problem hiding this comment.
This select needs a case for when the client is disconnected (can we tell by looking at conn?) and also a way to inject custom rules like max allowed session etc. Or should we just make max allowed a configurable parameter?
| enc := xml.NewEncoder(&xBuf) | ||
| enc.Indent("", " ") | ||
|
|
||
| if err := enc.EncodeElement(data, xml.StartElement{ |
There was a problem hiding this comment.
Maybe not drop this just yet if we want to test XML validity with a proper EPP library? I think this is OK to keep for a short while.
akarl
force-pushed
the
session-connection
branch
from
9bfeb26
to
9866391
Compare
No description provided.