diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 2c7ad82..52933d4 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -24,7 +24,7 @@ jobs: runs-on: ubuntu-latest steps: # Checkout with full history to enable commit listing - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 with: fetch-depth: 0 @@ -65,7 +65,7 @@ jobs: name: Lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Set up Go uses: actions/setup-go@v6 @@ -94,7 +94,7 @@ jobs: arch: [amd64, arm64] steps: # Checkout the specific commit to test - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 with: ref: ${{ matrix.commit }} @@ -116,6 +116,6 @@ jobs: name: Check License Lines runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - uses: kt3k/license_checker@v1.0.6 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0bdd9a3..41d2291 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -23,7 +23,7 @@ jobs: manifest-file: .release-please-manifest.json - if: ${{ steps.release.outputs.release_created }} - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Set up Go if: ${{ steps.release.outputs.release_created }} uses: actions/setup-go@v6 diff --git a/.github/workflows/repo.yml b/.github/workflows/repo.yml index bed3b7e..44fc43c 100644 --- a/.github/workflows/repo.yml +++ b/.github/workflows/repo.yml @@ -14,7 +14,7 @@ jobs: name: Conventional Commits runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 with: fetch-depth: 0 - uses: actions/setup-node@v6 diff --git a/cmds/dutagent/dutagent.go b/cmds/dutagent/dutagent.go index 506b01e..631ee45 100644 --- a/cmds/dutagent/dutagent.go +++ b/cmds/dutagent/dutagent.go @@ -9,25 +9,22 @@ package main import ( "context" - "crypto/tls" "errors" "flag" "fmt" "io" "log" - "net" "net/http" "os" "os/signal" "syscall" + "time" "connectrpc.com/connect" "github.com/BlindspotSoftware/dutctl/internal/buildinfo" "github.com/BlindspotSoftware/dutctl/internal/dutagent" "github.com/BlindspotSoftware/dutctl/pkg/dut" "github.com/BlindspotSoftware/dutctl/protobuf/gen/dutctl/v1/dutctlv1connect" - "golang.org/x/net/http2" - "golang.org/x/net/http2/h2c" "gopkg.in/yaml.v3" pb "github.com/BlindspotSoftware/dutctl/protobuf/gen/dutctl/v1" @@ -155,6 +152,9 @@ func printInitErr(err error) { log.Print(err) } +// readHeaderTimeout bounds how long the server waits to read request headers. +const readHeaderTimeout = 10 * time.Second + // startRPCService starts the RPC service, that ideally listens for incoming // connections forever. It always returns an non-nil error. func (agt *agent) startRPCService() error { @@ -167,12 +167,17 @@ func (agt *agent) startRPCService() error { path, handler := dutctlv1connect.NewDeviceServiceHandler(service) mux.Handle(path, handler) - //nolint:gosec - return http.ListenAndServe( - agt.address, - // Use h2c so we can serve HTTP/2 without TLS. - h2c.NewHandler(mux, &http2.Server{}), - ) + // Serve HTTP/2 without TLS (h2c) + srv := &http.Server{ + Addr: agt.address, + Handler: mux, + ReadHeaderTimeout: readHeaderTimeout, + } + srv.Protocols = new(http.Protocols) + srv.Protocols.SetHTTP1(true) + srv.Protocols.SetUnencryptedHTTP2(true) + + return srv.ListenAndServe() } func (agt *agent) registerWithServer() error { @@ -211,19 +216,17 @@ func spawnClient(agendURL string) dutctlv1connect.RelayServiceClient { // TODO: refactor into pkg and reuse in dutctl and dutserver. func newInsecureClient() *http.Client { + transport := &http.Transport{} + transport.Protocols = new(http.Protocols) + transport.Protocols.SetUnencryptedHTTP2(true) + return &http.Client{ - Transport: &http2.Transport{ - AllowHTTP: true, - DialTLS: func(network, addr string, _ *tls.Config) (net.Conn, error) { - // If you're also using this client for non-h2c traffic, you may want - // to delegate to tls.Dial if the network isn't TCP or the addr isn't - // in an allowlist. - - //nolint:noctx - return net.Dial(network, addr) - }, - // TODO: Don't forget timeouts! - }, + Transport: transport, + // TODO: Don't forget timeouts! http.Client.Timeout must not be used here: + // it bounds the entire exchange including the response body, which would + // abort long-lived streaming RPCs. Instead use per-RPC context deadlines + // on unary calls and/or transport timeouts (DialContext, + // TLSHandshakeTimeout, ResponseHeaderTimeout, IdleConnTimeout). } } diff --git a/cmds/dutctl/dutctl.go b/cmds/dutctl/dutctl.go index 3810743..34d4d01 100644 --- a/cmds/dutctl/dutctl.go +++ b/cmds/dutctl/dutctl.go @@ -7,13 +7,11 @@ package main import ( - "crypto/tls" "errors" "flag" "fmt" "io" "log" - "net" "net/http" "os" @@ -22,7 +20,6 @@ import ( "github.com/BlindspotSoftware/dutctl/internal/output" "github.com/BlindspotSoftware/dutctl/pkg/lock" "github.com/BlindspotSoftware/dutctl/protobuf/gen/dutctl/v1/dutctlv1connect" - "golang.org/x/net/http2" ) const usageAbstract = `dutctl - The client application of the DUT Control system. @@ -128,7 +125,6 @@ type application struct { func (app *application) setupRPCClient() { client := dutctlv1connect.NewDeviceServiceClient( - // Instead of http.DefaultClient, use the HTTP/2 protocol without TLS newInsecureClient(), fmt.Sprintf("http://%s", app.serverAddr), connect.WithGRPC(), @@ -138,19 +134,18 @@ func (app *application) setupRPCClient() { } func newInsecureClient() *http.Client { + // Use the HTTP/2 protocol without TLS (h2c) + transport := &http.Transport{} + transport.Protocols = new(http.Protocols) + transport.Protocols.SetUnencryptedHTTP2(true) + return &http.Client{ - Transport: &http2.Transport{ - AllowHTTP: true, - DialTLS: func(network, addr string, _ *tls.Config) (net.Conn, error) { - // If you're also using this client for non-h2c traffic, you may want - // to delegate to tls.Dial if the network isn't TCP or the addr isn't - // in an allowlist. - - //nolint:noctx - return net.Dial(network, addr) - }, - // Don't forget timeouts! - }, + Transport: transport, + // TODO: Don't forget timeouts! http.Client.Timeout must not be used here: + // it bounds the entire exchange including the response body, which would + // abort long-lived streaming RPCs. Instead use per-RPC context deadlines + // on unary calls and/or transport timeouts (DialContext, + // TLSHandshakeTimeout, ResponseHeaderTimeout, IdleConnTimeout). } } diff --git a/cmds/exp/dutserver/dutserver.go b/cmds/exp/dutserver/dutserver.go index ab9965b..d029b0c 100644 --- a/cmds/exp/dutserver/dutserver.go +++ b/cmds/exp/dutserver/dutserver.go @@ -14,10 +14,9 @@ import ( "os" "os/signal" "syscall" + "time" "github.com/BlindspotSoftware/dutctl/protobuf/gen/dutctl/v1/dutctlv1connect" - "golang.org/x/net/http2" - "golang.org/x/net/http2/h2c" ) const ( @@ -76,6 +75,9 @@ func (svr *server) watchInterrupt() { }() } +// readHeaderTimeout bounds how long the server waits to read request headers. +const readHeaderTimeout = 10 * time.Second + // startRPCService starts the RPC service, that ideally listens for incoming // connections forever. It always returns an non-nil error. func (svr *server) startRPCService() error { @@ -94,12 +96,17 @@ func (svr *server) startRPCService() error { path, handler = dutctlv1connect.NewRelayServiceHandler(service) mux.Handle(path, handler) - //nolint:gosec - return http.ListenAndServe( - svr.address, - // Use h2c so we can serve HTTP/2 without TLS. - h2c.NewHandler(mux, &http2.Server{}), - ) + // Serve HTTP/2 without TLS (h2c) + srv := &http.Server{ + Addr: svr.address, + Handler: mux, + ReadHeaderTimeout: readHeaderTimeout, + } + srv.Protocols = new(http.Protocols) + srv.Protocols.SetHTTP1(true) + srv.Protocols.SetUnencryptedHTTP2(true) + + return srv.ListenAndServe() } // start orchestrates the dutagent execution. diff --git a/cmds/exp/dutserver/rpc.go b/cmds/exp/dutserver/rpc.go index c13cf90..4692684 100644 --- a/cmds/exp/dutserver/rpc.go +++ b/cmds/exp/dutserver/rpc.go @@ -6,13 +6,11 @@ package main import ( "context" - "crypto/tls" "errors" "fmt" "io" "log" "maps" - "net" "net/http" "slices" "sync" @@ -20,7 +18,6 @@ import ( "connectrpc.com/connect" "github.com/BlindspotSoftware/dutctl/pkg/lock" "github.com/BlindspotSoftware/dutctl/protobuf/gen/dutctl/v1/dutctlv1connect" - "golang.org/x/net/http2" pb "github.com/BlindspotSoftware/dutctl/protobuf/gen/dutctl/v1" ) @@ -390,19 +387,18 @@ func spawnClient(agendURL string) dutctlv1connect.DeviceServiceClient { // TODO: refactor into pkg and reuse in dutctl and dutserver. func newInsecureClient() *http.Client { + // Use the HTTP/2 protocol without TLS (h2c). + transport := &http.Transport{} + transport.Protocols = new(http.Protocols) + transport.Protocols.SetUnencryptedHTTP2(true) + return &http.Client{ - Transport: &http2.Transport{ - AllowHTTP: true, - DialTLS: func(network, addr string, _ *tls.Config) (net.Conn, error) { - // If you're also using this client for non-h2c traffic, you may want - // to delegate to tls.Dial if the network isn't TCP or the addr isn't - // in an allowlist. - - //nolint:noctx - return net.Dial(network, addr) - }, - // TODO: Don't forget timeouts! - }, + Transport: transport, + // TODO: Don't forget timeouts! http.Client.Timeout must not be used here: + // it bounds the entire exchange including the response body, which would + // abort long-lived streaming RPCs. Instead use per-RPC context deadlines + // on unary calls and/or transport timeouts (DialContext, + // TLSHandshakeTimeout, ResponseHeaderTimeout, IdleConnTimeout). } } diff --git a/go.mod b/go.mod index ba1890b..113c51b 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,6 @@ require ( github.com/stianeikeland/go-rpio/v4 v4.6.0 github.com/tarm/serial v0.0.0-20180830185346-98f6abe2eb07 golang.org/x/crypto v0.51.0 - golang.org/x/net v0.53.0 google.golang.org/protobuf v1.36.11 gopkg.in/yaml.v3 v3.0.1 ) @@ -32,6 +31,7 @@ require ( github.com/olekukonko/tablewriter v1.0.9 // indirect github.com/rivo/uniseg v0.2.0 // indirect github.com/rogpeppe/go-internal v1.6.1 // indirect + golang.org/x/net v0.53.0 // indirect golang.org/x/sys v0.44.0 // indirect golang.org/x/text v0.37.0 // indirect ) diff --git a/pkg/dut/dut.go b/pkg/dut/dut.go index 2933768..6ff8c3b 100644 --- a/pkg/dut/dut.go +++ b/pkg/dut/dut.go @@ -15,9 +15,9 @@ import ( ) var ( - ErrDeviceNotFound = errors.New("no such device") - ErrCommandNotFound = errors.New("no such command") - ErrNoPassthroughForArgs = errors.New("arguments provided but command has no passthrough module to receive them") + ErrDeviceNotFound = errors.New("no such device") + ErrCommandNotFound = errors.New("no such command") + ErrNoReceiverForArgs = errors.New("arguments provided but command has neither a passthrough module nor declared arguments to receive them") ) // Devlist is a list of devices-under-test. @@ -124,8 +124,11 @@ func (c *Command) countPassthrough() int { // receive their statically configured Args with template references substituted // using runtimeArgs. The returned slice has the same length and ordering as c.Modules. func (c *Command) ModuleArgs(runtimeArgs []string) ([][]string, error) { - if len(runtimeArgs) > 0 && !c.HasPassthrough() { - return nil, ErrNoPassthroughForArgs + // Runtime args may be consumed either by a passthrough module or by + // command-level templating (declared c.Args substituted via ${name}). + // Only reject when neither can receive them. + if len(runtimeArgs) > 0 && !c.HasPassthrough() && len(c.Args) == 0 { + return nil, ErrNoReceiverForArgs } result := make([][]string, len(c.Modules)) diff --git a/pkg/dut/dut_test.go b/pkg/dut/dut_test.go index 918d26a..8260ff1 100644 --- a/pkg/dut/dut_test.go +++ b/pkg/dut/dut_test.go @@ -191,7 +191,7 @@ func TestModuleArgs(t *testing.T) { }}, runtimeArgs: []string{"a"}, want: nil, - err: ErrNoPassthroughForArgs, + err: ErrNoReceiverForArgs, }, { name: "mixed passthrough and non-passthrough", @@ -222,7 +222,7 @@ func TestModuleArgs(t *testing.T) { cmd: Command{}, runtimeArgs: []string{"a"}, want: nil, - err: ErrNoPassthroughForArgs, + err: ErrNoReceiverForArgs, }, { name: "error when runtime args provided but no passthrough module", @@ -231,7 +231,7 @@ func TestModuleArgs(t *testing.T) { }}, runtimeArgs: []string{"a"}, want: nil, - err: ErrNoPassthroughForArgs, + err: ErrNoReceiverForArgs, }, { name: "passthrough module with no runtime args",