From 3dbc8320e08f8bb55325a244fb8b409f3d40cd97 Mon Sep 17 00:00:00 2001 From: Jeroen Rinzema Date: Tue, 25 Oct 2022 14:09:26 +0200 Subject: [PATCH] feat: create issues of todo comments --- .github/workflows/todo.yaml | 20 ++++++++++++++++ auth_test.go | 2 +- command.go | 48 +++++++++++++++++++++++++------------ command_test.go | 2 +- handshake.go | 10 ++++++-- internal/mock/client.go | 6 ++--- options.go | 2 +- row.go | 17 +++++++++++-- wire.go | 2 +- 9 files changed, 83 insertions(+), 26 deletions(-) create mode 100644 .github/workflows/todo.yaml diff --git a/.github/workflows/todo.yaml b/.github/workflows/todo.yaml new file mode 100644 index 0000000..c07434c --- /dev/null +++ b/.github/workflows/todo.yaml @@ -0,0 +1,20 @@ +name: TODOs + +on: + push: + branches: + - main + +jobs: + scan: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Run tdg-github-action + uses: ribtoks/tdg-github-action@master + with: + TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + SHA: ${{ github.sha }} + REF: ${{ github.ref }} + LABEL: "todo" diff --git a/auth_test.go b/auth_test.go index c87ba84..2fac01d 100644 --- a/auth_test.go +++ b/auth_test.go @@ -56,7 +56,7 @@ func TestClearTextPassword(t *testing.T) { input := bytes.NewBuffer([]byte{}) incoming := buffer.NewWriter(input) - // NOTE(Jeroen): we could reuse the server buffered writer to write client messages + // NOTE: we could reuse the server buffered writer to write client messages incoming.Start(types.ServerMessage(types.ClientPassword)) incoming.AddString(expected) incoming.AddNullTerminate() diff --git a/command.go b/command.go index 1471510..54e4ee5 100644 --- a/command.go +++ b/command.go @@ -38,7 +38,9 @@ func NewErrUnkownStatement(name string) error { func (srv *Server) consumeCommands(ctx context.Context, conn net.Conn, reader *buffer.Reader, writer *buffer.Writer) (err error) { srv.logger.Debug("ready for query... starting to consume commands") - // TODO(Jeroen): include a identification value inside the context that + // TODO: Include a value to identify unique connections + // + // include a identification value inside the context that // could be used to identify connections at a later stage. for { @@ -52,7 +54,7 @@ func (srv *Server) consumeCommands(ctx context.Context, conn net.Conn, reader *b return nil } - // NOTE(Jeroen): we could recover from this scenario + // NOTE: we could recover from this scenario if errors.Is(err, buffer.ErrMessageSizeExceeded) { err = srv.handleMessageSizeExceeded(reader, writer, err) if err != nil { @@ -112,7 +114,7 @@ func (srv *Server) handleCommand(ctx context.Context, conn net.Conn, t types.Cli switch t { case types.ClientSync: - // TODO(Jeroen): include the ability to catch sync messages in order to + // TODO: Include the ability to catch sync messages in order to // close the current transaction. // // At completion of each series of extended-query messages, the frontend @@ -136,7 +138,7 @@ func (srv *Server) handleCommand(ctx context.Context, conn net.Conn, t types.Cli case types.ClientParse: return srv.handleParse(ctx, reader, writer) case types.ClientDescribe: - // TODO(Jeroen): the server should return the column types that will be + // TODO: Server should return the column types that will be // returned for the given portal or statement. // // The Describe message (portal variant) specifies the name of an @@ -161,8 +163,10 @@ func (srv *Server) handleCommand(ctx context.Context, conn net.Conn, t types.Cli case types.ClientBind: return srv.handleBind(ctx, reader, writer) case types.ClientFlush: - // TODO(Jeroen): flush all remaining rows inside connection buffer if - // any are remaining. The Flush message does not cause any specific + // TODO: Flush all remaining rows inside connection buffer if + // any are remaining. + // + // The Flush message does not cause any specific // output to be generated, but forces the backend to deliver any data // pending in its output buffers. A Flush must be sent after any // extended-query command except Sync, if the frontend wishes to examine @@ -247,7 +251,7 @@ func (srv *Server) handleParse(ctx context.Context, reader *buffer.Reader, write return err } - // NOTE(Jeroen): the number of parameter data types specified (can be + // NOTE: the number of parameter data types specified (can be // zero). Note that this is not an indication of the number of parameters // that might appear in the query string, only the number that the frontend // wants to prespecify types for. @@ -257,9 +261,11 @@ func (srv *Server) handleParse(ctx context.Context, reader *buffer.Reader, write } for i := uint16(0); i < parameters; i++ { - // TODO(Jeroen): reader.GetUint32() + // TODO: Specifies the object ID of the parameter data type + // // Specifies the object ID of the parameter data type. Placing a zero here // is equivalent to leaving the type unspecified. + // `reader.GetUint32()` } statement, descriptions, err := srv.Parse(ctx, query) @@ -328,7 +334,7 @@ func (srv *Server) handleBind(ctx context.Context, reader *buffer.Reader, writer // reader. The parameters are parsed and returned. // https://www.postgresql.org/docs/14/protocol-message-formats.html func (srv *Server) readParameters(ctx context.Context, reader *buffer.Reader) ([]string, error) { - // NOTE(Jeroen): read the total amount of parameter format codes that will + // NOTE: read the total amount of parameter format codes that will // be send by the client. length, err := reader.GetUint16() if err != nil { @@ -343,16 +349,20 @@ func (srv *Server) readParameters(ctx context.Context, reader *buffer.Reader) ([ return nil, err } - // NOTE(Jeroen): the parameter format codes. Each must presently be zero (text) or one (binary). + // NOTE: the parameter format codes. Each must presently be zero (text) or one (binary). // https://www.postgresql.org/docs/14/protocol-message-formats.html if format != 0 { return nil, errors.New("unsupported binary parameter format, only text formatted parameter types are currently supported") } - // TODO(Jeroen): handle multiple parameter format codes. + // TODO: Handle multiple parameter format codes. + // + // We are currently only supporting string parameters. We have to + // include support for binary parameters in the future. + // https://www.postgresql.org/docs/14/protocol-message-formats.html } - // NOTE(Jeroen): read the total amount of parameter values that will be send + // NOTE: read the total amount of parameter values that will be send // by the client. length, err = reader.GetUint16() if err != nil { @@ -377,7 +387,7 @@ func (srv *Server) readParameters(ctx context.Context, reader *buffer.Reader) ([ parameters[i] = string(value) } - // NOTE(Jeroen): read the total amount of result-column format that will be + // NOTE: Read the total amount of result-column format that will be // send by the client. length, err = reader.GetUint16() if err != nil { @@ -387,7 +397,13 @@ func (srv *Server) readParameters(ctx context.Context, reader *buffer.Reader) ([ srv.logger.Debug("reading result-column format codes", zap.Uint16("length", length)) for i := uint16(0); i < length; i++ { - // TODO(Jeroen): handle incoming result-column format codes + // TODO: Handle incoming result-column format codes + // + // Incoming format codes are currently ignored and should be handled in + // the future. The result-column format codes. Each must presently be + // zero (text) or one (binary). These format codes should be returned + // and handled by the parent function to return the proper column formats. + // https://www.postgresql.org/docs/current/protocol-message-formats.html _, err := reader.GetUint16() if err != nil { return nil, err @@ -407,7 +423,9 @@ func (srv *Server) handleExecute(ctx context.Context, reader *buffer.Reader, wri return err } - // TODO(Jeroen): Maximum number of limit to return, if portal contains a + // TODO: Limit the maximum number of records to be returned. + // + // Maximum number of limit to return, if portal contains a // query that returns limit (ignored otherwise). Zero denotes “no limit”. limit, err := reader.GetUint32() if err != nil { diff --git a/command_test.go b/command_test.go index 9b36200..520cb42 100644 --- a/command_test.go +++ b/command_test.go @@ -32,7 +32,7 @@ func TestMessageSizeExceeded(t *testing.T) { client.Authenticate(t) client.ReadyForQuery(t) - // NOTE(Jeroen): attempt to send a message twice the max buffer size + // NOTE: attempt to send a message twice the max buffer size size := uint32(buffer.DefaultBufferSize * 2) t.Logf("writing message of size: %d", size) diff --git a/handshake.go b/handshake.go index cc7623c..2509f48 100644 --- a/handshake.go +++ b/handshake.go @@ -24,7 +24,13 @@ func (srv *Server) Handshake(conn net.Conn) (_ net.Conn, version types.Version, return conn, version, reader, nil } - // TODO(Jeroen): support GSS encryption + // TODO: support GSS encryption + // + // `psql-wire` currently does not support GSS encrypted connections. The GSS + // authentication API is supported inside the PostgreSQL wire protocol and + // API's should be made available to support these type of connections. + // https://www.postgresql.org/docs/current/gssapi-auth.html + // https://www.postgresql.org/docs/current/protocol-flow.html#id-1.10.6.7.13 conn, reader, version, err = srv.potentialConnUpgrade(conn, reader, version) if err != nil { @@ -152,7 +158,7 @@ func (srv *Server) potentialConnUpgrade(conn net.Conn, reader *buffer.Reader, ve ClientCAs: srv.ClientCAs, } - // NOTE(Jeroen): initialize the TLS connection and construct a new buffered + // NOTE: initialize the TLS connection and construct a new buffered // reader for the constructed TLS connection. conn = tls.Server(conn, &tlsConfig) reader = buffer.NewReader(conn, srv.BufferedMsgSize) diff --git a/internal/mock/client.go b/internal/mock/client.go index 3fdd4de..7e73c0b 100644 --- a/internal/mock/client.go +++ b/internal/mock/client.go @@ -33,7 +33,7 @@ func (client *Client) Handshake(t *testing.T) { version := make([]byte, 4) binary.BigEndian.PutUint32(version, uint32(types.Version30)) - // NOTE(Jeroen): the parameters consist out of keys and values. Each key and + // NOTE: the parameters consist out of keys and values. Each key and // value is terminated using a nul byte and the end of all parameters is // identified using a empty key value. nul := byte(0) @@ -42,7 +42,7 @@ func (client *Client) Handshake(t *testing.T) { end := append([]byte(""), nul) parameters := append(append(key, value...), end...) - // NOTE(Jeroen): we have to define the total message length inside the + // NOTE: we have to define the total message length inside the // header by prefixing a unsigned 32 big-endian int. header := make([]byte, 4) binary.BigEndian.PutUint32(header, uint32(len(version)+len(parameters)+len(header))) @@ -74,7 +74,7 @@ func (client *Client) Authenticate(t *testing.T) { t.Fatal(err) } - // NOTE(Jeroen): a status of 0 indicates that the connection has been authenticated + // NOTE: a status of 0 indicates that the connection has been authenticated if status != 0 { t.Fatalf("unexpected auth status: %d, expected auth ok", status) } diff --git a/options.go b/options.go index 295b51c..ec5a1b4 100644 --- a/options.go +++ b/options.go @@ -68,7 +68,7 @@ func SimpleQuery(fn SimpleQueryFn) OptionFn { return fn(ctx, query, writer, parameters) } - // NOTE(Jeroen): we have to lookup all unique positional parameters + // NOTE: we have to lookup all unique positional parameters // within the given query. We return a zero parameter oid for each // positional parameter indicating that the given parameters could // contain any type. We could safely ignore the err check while diff --git a/row.go b/row.go index 15d7951..0e4cf62 100644 --- a/row.go +++ b/row.go @@ -70,7 +70,20 @@ func (column Column) Define(ctx context.Context, writer *buffer.Writer) { writer.AddInt16(column.AttrNo) writer.AddInt32(int32(column.Oid)) writer.AddInt16(column.Width) - writer.AddInt32(-1) // TODO(Jeroen): type modifiers have not yet been fully implemented. Setting -1 to indicate a undefined value + // TODO: Support type for type modifiers + // + // Some types could be overridden using the type modifier field within a RowDescription. + // Type modifier (see pg_attribute.atttypmod). The meaning of the + // modifier is type-specific. + // Atttypmod records type-specific data supplied at table creation time (for + // example, the maximum length of a varchar column). It is passed to + // type-specific input functions and length coercion functions. The value + // will generally be -1 for types that do not need atttypmod. + // + // https://www.postgresql.org/docs/current/protocol-message-formats.html + // https://www.postgresql.org/docs/current/catalog-pg-attribute.html + + writer.AddInt32(-1) writer.AddInt16(int16(column.Format)) } @@ -103,7 +116,7 @@ func (column Column) Write(ctx context.Context, writer *buffer.Writer, src any) return err } - // NOTE(Jeroen): The length of the column value, in bytes (this count does + // NOTE: The length of the column value, in bytes (this count does // not include itself). Can be zero. As a special case, -1 indicates a NULL // column value. No value bytes follow in the NULL case. length := int32(len(bb)) diff --git a/wire.go b/wire.go index ec7ae17..0a12970 100644 --- a/wire.go +++ b/wire.go @@ -89,7 +89,7 @@ func (srv *Server) Serve(listener net.Listener) error { srv.wg.Add(1) - // NOTE(Jeroen): handle graceful shutdowns + // NOTE: handle graceful shutdowns go func() { defer srv.wg.Done() <-srv.closer