diff --git a/livekit/sip.go b/livekit/sip.go index 454bf00ae..679ca4380 100644 --- a/livekit/sip.go +++ b/livekit/sip.go @@ -308,16 +308,17 @@ func (p *SIPOutboundTrunkInfo) AsTrunkInfo() *SIPTrunkInfo { } // validateHeaders makes sure header names/keys and values are per SIP specifications -func validateHeaders(headers map[string]string) error { +func validateHeaders(headers map[string]string) []error { + var errs []error for headerName, headerValue := range headers { if err := ValidateHeaderName(headerName, true); err != nil { - return fmt.Errorf("invalid header name: %w", err) - } - if err := ValidateHeaderValue(headerName, headerValue); err != nil { - return fmt.Errorf("invalid header value for %s: %w", headerName, err) + errs = append(errs, fmt.Errorf("invalid header name: %w", err)) + continue } + valueErrs := ValidateHeaderValue(headerName, headerValue) + errs = append(errs, valueErrs...) } - return nil + return errs } // validateHeaderNames Makes sure the values of the given map correspond to valid SIP header names @@ -360,38 +361,32 @@ func (p *SIPTrunkInfo) Validate() error { return nil } -func (p *CreateSIPOutboundTrunkRequest) Validate() error { +func (p *CreateSIPOutboundTrunkRequest) Validate() []error { if p.Trunk == nil { - return errors.New("missing trunk") + return []error{errors.New("missing trunk")} } if p.Trunk.SipTrunkId != "" { - return errors.New("trunk id must not be set") - } - if err := p.Trunk.Validate(); err != nil { - return err + return []error{errors.New("trunk id must not be set")} } - return nil + return p.Trunk.Validate() } -func (p *CreateSIPInboundTrunkRequest) Validate() error { +func (p *CreateSIPInboundTrunkRequest) Validate() []error { if p.Trunk == nil { - return errors.New("missing trunk") + return []error{errors.New("missing trunk")} } if p.Trunk.SipTrunkId != "" { - return errors.New("trunk id must not be set") - } - if err := p.Trunk.Validate(); err != nil { - return err + return []error{errors.New("trunk id must not be set")} } - return nil + return p.Trunk.Validate() } -func (p *UpdateSIPOutboundTrunkRequest) Validate() error { +func (p *UpdateSIPOutboundTrunkRequest) Validate() []error { if p.SipTrunkId == "" { - return errors.New("trunk id must be set") + return []error{errors.New("trunk id must be set")} } if p.Action == nil { - return errors.New("missing or unsupported update action") + return []error{errors.New("missing or unsupported update action")} } switch a := p.Action.(type) { default: @@ -399,27 +394,27 @@ func (p *UpdateSIPOutboundTrunkRequest) Validate() error { case *UpdateSIPOutboundTrunkRequest_Replace: info := a.Replace if info == nil { - return errors.New("missing trunk") + return []error{errors.New("missing trunk")} } if info.SipTrunkId != "" && info.SipTrunkId != p.SipTrunkId { - return errors.New("trunk id in the info must be empty or match the id in the update") + return []error{errors.New("trunk id in the info must be empty or match the id in the update")} } return info.Validate() case *UpdateSIPOutboundTrunkRequest_Update: diff := a.Update if diff == nil { - return errors.New("missing trunk update") + return []error{errors.New("missing trunk update")} } - return diff.Validate() + return []error{diff.Validate()} } } -func (p *UpdateSIPInboundTrunkRequest) Validate() error { +func (p *UpdateSIPInboundTrunkRequest) Validate() []error { if p.SipTrunkId == "" { - return errors.New("trunk id must be set") + return []error{errors.New("trunk id must be set")} } if p.Action == nil { - return errors.New("missing or unsupported update action") + return []error{errors.New("missing or unsupported update action")} } switch a := p.Action.(type) { default: @@ -427,39 +422,39 @@ func (p *UpdateSIPInboundTrunkRequest) Validate() error { case *UpdateSIPInboundTrunkRequest_Replace: info := a.Replace if info == nil { - return errors.New("missing trunk") + return []error{errors.New("missing trunk")} } if info.SipTrunkId != "" && info.SipTrunkId != p.SipTrunkId { - return errors.New("trunk id in the info must be empty or match the id in the update") + return []error{errors.New("trunk id in the info must be empty or match the id in the update")} } return info.Validate() case *UpdateSIPInboundTrunkRequest_Update: diff := a.Update if diff == nil { - return errors.New("missing trunk update") + return []error{errors.New("missing trunk update")} } - return diff.Validate() + return []error{diff.Validate()} } } -func (p *SIPInboundTrunkInfo) Validate() error { +func (p *SIPInboundTrunkInfo) Validate() []error { hasAuth := p.AuthUsername != "" || p.AuthPassword != "" hasCIDR := len(p.AllowedAddresses) != 0 hasNumbers := len(p.Numbers) != 0 // TODO: remove this condition, it doesn't really help with security if !hasAuth && !hasCIDR && !hasNumbers { - return psrpc.NewErrorf(psrpc.InvalidArgument, "for security, one of the fields must be set: AuthUsername+AuthPassword, AllowedAddresses or Numbers") + return []error{psrpc.NewErrorf(psrpc.InvalidArgument, "for security, one of the fields must be set: AuthUsername+AuthPassword, AllowedAddresses or Numbers")} } if err := p.Media.Validate(); err != nil { - return err + return []error{err} } - if err := validateHeaders(p.Headers); err != nil { - return err + if errs := validateHeaders(p.Headers); len(errs) > 0 { + return errs } if err := validateAttributesToHeaders(p.AttributesToHeaders); err != nil { - return err + return []error{err} } if err := validateHeaderToAttributes(p.HeadersToAttributes); err != nil { - return err + return []error{err} } return nil } @@ -480,9 +475,9 @@ func (p *SIPInboundTrunkUpdate) Validate() error { return nil } -func (p *SIPInboundTrunkUpdate) Apply(info *SIPInboundTrunkInfo) error { +func (p *SIPInboundTrunkUpdate) Apply(info *SIPInboundTrunkInfo) []error { if err := p.Validate(); err != nil { - return err + return []error{err} } applyListUpdate(&info.Numbers, p.Numbers) applyListUpdate(&info.AllowedAddresses, p.AllowedAddresses) @@ -499,7 +494,7 @@ func (p *SIPInboundTrunkUpdate) Apply(info *SIPInboundTrunkInfo) error { type UpdateSIPOutboundTrunkRequestAction interface { isUpdateSIPOutboundTrunkRequest_Action - Apply(info *SIPOutboundTrunkInfo) (*SIPOutboundTrunkInfo, error) + Apply(info *SIPOutboundTrunkInfo) (*SIPOutboundTrunkInfo, []error) } var ( @@ -507,56 +502,56 @@ var ( _ UpdateSIPOutboundTrunkRequestAction = (*UpdateSIPOutboundTrunkRequest_Update)(nil) ) -func (p *UpdateSIPOutboundTrunkRequest_Replace) Apply(info *SIPOutboundTrunkInfo) (*SIPOutboundTrunkInfo, error) { +func (p *UpdateSIPOutboundTrunkRequest_Replace) Apply(info *SIPOutboundTrunkInfo) (*SIPOutboundTrunkInfo, []error) { val := proto.CloneOf(p.Replace) if val == nil { - return nil, errors.New("missing trunk") + return nil, []error{errors.New("missing trunk")} } if info.SipTrunkId != "" { val.SipTrunkId = info.SipTrunkId } - if err := val.Validate(); err != nil { - return nil, err + if errs := val.Validate(); len(errs) > 0 { + return nil, errs } return val, nil } -func (p *UpdateSIPOutboundTrunkRequest_Update) Apply(info *SIPOutboundTrunkInfo) (*SIPOutboundTrunkInfo, error) { +func (p *UpdateSIPOutboundTrunkRequest_Update) Apply(info *SIPOutboundTrunkInfo) (*SIPOutboundTrunkInfo, []error) { diff := p.Update if diff == nil { - return nil, errors.New("missing trunk update") + return nil, []error{errors.New("missing trunk update")} } val := proto.CloneOf(info) - if err := diff.Apply(val); err != nil { - return nil, err + if errs := diff.Apply(val); len(errs) > 0 { + return nil, errs } return val, nil } -func (p *SIPOutboundTrunkInfo) Validate() error { +func (p *SIPOutboundTrunkInfo) Validate() []error { if len(p.Numbers) == 0 { - return errors.New("no trunk numbers specified") + return []error{errors.New("no trunk numbers specified")} } if p.Address == "" { - return errors.New("no outbound address specified") + return []error{errors.New("no outbound address specified")} } if err := validateHostnameFormat(p.Address, "trunk address"); err != nil { - return err + return []error{err} } if err := validateHostnameFormat(p.FromHost, "from_host"); err != nil { - return err + return []error{err} } - if err := validateHeaders(p.Headers); err != nil { - return err + if errs := validateHeaders(p.Headers); len(errs) > 0 { + return errs } if err := validateAttributesToHeaders(p.AttributesToHeaders); err != nil { - return err + return []error{err} } if err := validateHeaderToAttributes(p.HeadersToAttributes); err != nil { - return err + return []error{err} } if err := p.Media.Validate(); err != nil { - return err + return []error{err} } return nil } @@ -590,9 +585,9 @@ func (p *SIPOutboundTrunkUpdate) Validate() error { return nil } -func (p *SIPOutboundTrunkUpdate) Apply(info *SIPOutboundTrunkInfo) error { +func (p *SIPOutboundTrunkUpdate) Apply(info *SIPOutboundTrunkInfo) []error { if err := p.Validate(); err != nil { - return err + return []error{err} } applyUpdate(&info.Address, p.Address) applyUpdate(&info.Transport, p.Transport) @@ -610,7 +605,7 @@ func (p *SIPOutboundTrunkUpdate) Apply(info *SIPOutboundTrunkInfo) error { type UpdateSIPInboundTrunkRequestAction interface { isUpdateSIPInboundTrunkRequest_Action - Apply(info *SIPInboundTrunkInfo) (*SIPInboundTrunkInfo, error) + Apply(info *SIPInboundTrunkInfo) (*SIPInboundTrunkInfo, []error) } var ( @@ -618,24 +613,24 @@ var ( _ UpdateSIPInboundTrunkRequestAction = (*UpdateSIPInboundTrunkRequest_Update)(nil) ) -func (p *UpdateSIPInboundTrunkRequest_Replace) Apply(info *SIPInboundTrunkInfo) (*SIPInboundTrunkInfo, error) { +func (p *UpdateSIPInboundTrunkRequest_Replace) Apply(info *SIPInboundTrunkInfo) (*SIPInboundTrunkInfo, []error) { val := proto.CloneOf(p.Replace) if val == nil { - return nil, errors.New("missing trunk") + return nil, []error{errors.New("missing trunk")} } if info.SipTrunkId != "" { val.SipTrunkId = info.SipTrunkId } - if err := val.Validate(); err != nil { - return nil, err + if errs := val.Validate(); len(errs) > 0 { + return nil, errs } return val, nil } -func (p *UpdateSIPInboundTrunkRequest_Update) Apply(info *SIPInboundTrunkInfo) (*SIPInboundTrunkInfo, error) { +func (p *UpdateSIPInboundTrunkRequest_Update) Apply(info *SIPInboundTrunkInfo) (*SIPInboundTrunkInfo, []error) { diff := p.Update if diff == nil { - return nil, errors.New("missing trunk update") + return nil, []error{errors.New("missing trunk update")} } val := proto.CloneOf(info) if err := diff.Apply(val); err != nil { @@ -773,42 +768,42 @@ func (p *UpdateSIPDispatchRuleRequest_Update) Apply(info *SIPDispatchRuleInfo) ( return val, nil } -func (p *CreateSIPParticipantRequest) Validate() error { +func (p *CreateSIPParticipantRequest) Validate() []error { if p.SipTrunkId == "" && p.Trunk == nil && p.SipNumber == "" { - return errors.New("missing sip trunk id and sip number") + return []error{errors.New("missing sip trunk id and sip number")} } if p.Trunk != nil { if err := p.Trunk.Validate(); err != nil { - return err + return []error{err} } } if p.SipCallTo == "" { - return errors.New("missing sip callee number") + return []error{errors.New("missing sip callee number")} } else if strings.Contains(p.SipCallTo, "@") { - return errors.New("SipCallTo should be a phone number or SIP user, not a full SIP URI") + return []error{errors.New("SipCallTo should be a phone number or SIP user, not a full SIP URI")} } if p.RoomName == "" { - return errors.New("missing room name") + return []error{errors.New("missing room name")} } // Validate display_name if provided if p.DisplayName != nil { if len(*p.DisplayName) > 128 { - return errors.New("DisplayName too long (max 128 characters)") + return []error{errors.New("DisplayName too long (max 128 characters)")} } if _, err := strconv.Unquote("\"" + *p.DisplayName + "\""); err != nil { - return fmt.Errorf("DisplayName must be a valid quoted string: %w", err) + return []error{fmt.Errorf("DisplayName must be a valid quoted string: %w", err)} } } // Validate destination if provided if err := p.Destination.Validate(); err != nil { - return err + return []error{err} } - if err := validateHeaders(p.Headers); err != nil { - return err + if errs := validateHeaders(p.Headers); len(errs) > 0 { + return errs } if err := p.Media.Validate(); err != nil { - return err + return []error{err} } return nil } @@ -853,15 +848,15 @@ func (d *Destination) Validate() error { return nil } -func (p *TransferSIPParticipantRequest) Validate() error { +func (p *TransferSIPParticipantRequest) Validate() []error { if p.RoomName == "" { - return errors.New("missing room name") + return []error{errors.New("missing room name")} } if p.ParticipantIdentity == "" { - return errors.New("missing participant identity") + return []error{errors.New("missing participant identity")} } if p.TransferTo == "" { - return errors.New("missing transfer to") + return []error{errors.New("missing transfer to")} } // Validate TransferTo URI format and ensure RFC compliance @@ -876,7 +871,7 @@ func (p *TransferSIPParticipantRequest) Validate() error { if !strings.HasPrefix(innerURI, "sip:") && !strings.HasPrefix(innerURI, "sips:") && !strings.HasPrefix(innerURI, "tel:") { // In theory the Refer-To header can receive the full name-addr. // This can make this check inaccurate, but we want to limit to just SIP and TEL URIs. - return errors.New("transfer_to must be a valid SIP(s) or TEL URI (sip:, sips: or tel:)") + return []error{errors.New("transfer_to must be a valid SIP(s) or TEL URI (sip:, sips: or tel:)")} } if strings.HasPrefix(innerURI, "sip:") || strings.HasPrefix(innerURI, "sips:") { @@ -890,8 +885,8 @@ func (p *TransferSIPParticipantRequest) Validate() error { p.TransferTo = innerURI } - if err := validateHeaders(p.Headers); err != nil { - return err + if errs := validateHeaders(p.Headers); len(errs) > 0 { + return errs } return nil diff --git a/livekit/sip_test.go b/livekit/sip_test.go index 59f0b96dc..4e7bf5bb0 100644 --- a/livekit/sip_test.go +++ b/livekit/sip_test.go @@ -45,13 +45,43 @@ func TestSIPTrunkAs(t *testing.T) { }) } -func TestSIPValidate(t *testing.T) { - type validateable interface { - Validate() error +// Helper to handle the fact taht some of the Validate methods return `error` +// and some return `[]error`. +func validate(t *testing.T, req any) []error { + t.Helper() + + var errs []error + switch v := req.(type) { + case *SIPInboundTrunkInfo: + errs = v.Validate() + case *SIPOutboundTrunkInfo: + errs = v.Validate() + case *SIPMediaConfig: + if err := v.Validate(); err != nil { + errs = []error{v.Validate()} + } + case *CreateSIPDispatchRuleRequest: + if err := v.Validate(); err != nil { + errs = []error{v.Validate()} + } + case *UpdateSIPDispatchRuleRequest: + if err := v.Validate(); err != nil { + errs = []error{v.Validate()} + } + case *CreateSIPParticipantRequest: + errs = v.Validate() + default: + t.Fatalf("unexpected request type: %T", v) } + + return errs +} + +func TestSIPValidate(t *testing.T) { + type validateTestCase struct { name string - req validateable + req any exp bool } cases := map[string][]validateTestCase{ @@ -327,8 +357,8 @@ func TestSIPValidate(t *testing.T) { t.Run(name, func(t *testing.T) { for _, c := range class { t.Run(c.name, func(t *testing.T) { - err := c.req.Validate() - require.Equal(t, c.exp, err == nil, "error: %v", err) + errs := validate(t, c.req) + require.Equal(t, c.exp, len(errs) == 0, "errors: %v", errs) }) } }) @@ -701,11 +731,11 @@ func TestTransferSIPParticipantRequestValidate(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { - err := c.req.Validate() + errs := c.req.Validate() if c.expectError { - require.Error(t, err, "Expected validation to fail") + require.NotEmpty(t, errs, "Expected validation to fail") } else { - require.NoError(t, err, "Expected validation to pass") + require.Empty(t, errs, "Expected validation to pass") require.Equal(t, c.expectedURI, c.req.TransferTo, "TransferTo should be RFC-compliant after validation") } }) @@ -732,8 +762,8 @@ func TestInboundTrunkUpdate(t *testing.T) { Update: u, }, } - out, err := upd.Action.(UpdateSIPInboundTrunkRequestAction).Apply(r) - require.NoError(t, err) + out, errs := upd.Action.(UpdateSIPInboundTrunkRequestAction).Apply(r) + require.Empty(t, errs) require.True(t, r != out) return out } @@ -795,8 +825,8 @@ func TestInboundTrunkUpdate(t *testing.T) { }, } - out, err := upd2.Action.(UpdateSIPInboundTrunkRequestAction).Apply(r) - require.NoError(t, err) + out, errs := upd2.Action.(UpdateSIPInboundTrunkRequestAction).Apply(r) + require.Empty(t, errs) require.True(t, r != out) require.True(t, r2 != out) prototest.Equals(t, r2, out) @@ -809,8 +839,8 @@ func TestOutboundTrunkUpdate(t *testing.T) { Update: u, }, } - out, err := upd.Action.(UpdateSIPOutboundTrunkRequestAction).Apply(r) - require.NoError(t, err) + out, errs := upd.Action.(UpdateSIPOutboundTrunkRequestAction).Apply(r) + require.Empty(t, errs) require.True(t, r != out) return out } @@ -878,8 +908,8 @@ func TestOutboundTrunkUpdate(t *testing.T) { }, } - out, err := upd2.Action.(UpdateSIPOutboundTrunkRequestAction).Apply(r) - require.NoError(t, err) + out, errs := upd2.Action.(UpdateSIPOutboundTrunkRequestAction).Apply(r) + require.Empty(t, errs) require.True(t, r != out) require.True(t, r2 != out) prototest.Equals(t, r2, out) diff --git a/livekit/sip_validation.go b/livekit/sip_validation.go index a30df1c92..0799a8568 100644 --- a/livekit/sip_validation.go +++ b/livekit/sip_validation.go @@ -19,9 +19,18 @@ import ( fmt "fmt" "strconv" "strings" + + "github.com/livekit/protocol/logger" ) // RFC 3261 compliant validation functions for SIP headers and messages +// +// Some of the Validate* functions return error slices comprised of all errors +// encountered during validation. Of these errors, some might soft errors, +// that is, errors which reflect real validation failures but should probably +// be ignored by the caller using the LogSoftErrors helper, e.g.: +// +// err := LogSoftErrors(ValidateHeaderValue(headerName, headerValue)) type allowedCharacters struct { ascii [128]bool @@ -212,10 +221,45 @@ var nameAddrHeaders = map[string]bool{ "referred-by": true, // RFC 3892 Section 3 } -var softFailureReporter func(err error) +// SoftValidationErr is an error that should not prevent further processing. +var SoftValidationErr = errors.New("soft validation failure") + +type errorFn func(error) + +// perHardSoftErrors invokes the supplied callbacks against errors. +func perHardSoftErrors(errs []error, hardErrFn, softErrFn errorFn) { + for _, err := range errs { + if errors.Is(err, SoftValidationErr) { + softErrFn(err) + continue + } + hardErrFn(err) + } +} + +// LogSoftErrors returns a new error comprised of all hard errors in the given +// slice and logs a warning for each soft error. +func LogSoftErrors(logger logger.Logger, errs []error) error { + var hardErrs []error + perHardSoftErrors(errs, func(err error) { + hardErrs = append(hardErrs, err) + }, func(err error) { + logger.Warnw("soft validation error", err) + }) + return errors.Join(hardErrs...) +} -func SetSoftFailureReporter(reporter func(err error)) { - softFailureReporter = reporter +// JoinErrors returns a new error comprised of all hard erros in the given slice. +func JoinErrors(errs []error) error { + var hardErrs []error + perHardSoftErrors(errs, func(err error) { + hardErrs = append(hardErrs, err) + }, func(err error) {}) + return errors.Join(hardErrs...) +} + +func newSoftValidationErr(err error) error { + return fmt.Errorf("%w: %w", SoftValidationErr, err) } // ValidateHeaderName validates a SIP header name per RFC 3261 Section 25.1 @@ -244,33 +288,30 @@ func ValidateHeaderName(name string, restrictNames bool) error { } // ValidateHeaderValue validates a SIP header value per RFC 3261 Section 25.1 -func ValidateHeaderValue(name, value string) error { +func ValidateHeaderValue(name, value string) []error { if value == "" { return nil } if len(value) > 1024 { - return fmt.Errorf("header %s: value too long (max 1024 characters)", name) + return []error{fmt.Errorf("header %s: value too long (max 1024 characters)", name)} } // Basic character validation - printable ASCII. We're stricter than the spec here - no UTF-8 for now if err := headerValuesCharacters.Validate(value); err != nil { - return fmt.Errorf("header %s: value: %w", name, err) + return []error{fmt.Errorf("header %s: value: %w", name, err)} } // Convert to lowercase for case-insensitive comparison + var errs []error lowerName := strings.ToLower(name) if _, exists := nameAddrHeaders[lowerName]; exists { if err := validateNameAddrHeader(value); err != nil { - err = fmt.Errorf("header %s: value: %w", name, err) - // For now do not actually error out on header values, just note failure. - if cb := softFailureReporter; cb != nil { - cb(err) - } + errs = append(errs, fmt.Errorf("header %s: value: %w", name, newSoftValidationErr(err))) } } - return nil + return errs } // findAngleBrackets efficiently finds angle brackets in a single scan diff --git a/livekit/sip_validation_test.go b/livekit/sip_validation_test.go index 266b5409f..58fcb444a 100644 --- a/livekit/sip_validation_test.go +++ b/livekit/sip_validation_test.go @@ -15,9 +15,16 @@ package livekit import ( + "errors" "fmt" "strings" "testing" + + "github.com/livekit/protocol/logger" + "github.com/livekit/protocol/logger/testutil" + "github.com/livekit/protocol/logger/zaputil" + "github.com/stretchr/testify/require" + "go.uber.org/zap/zapcore" ) // Valid Header Test Cases @@ -276,3 +283,51 @@ func TestFrobiddenSipHeaderNames(t *testing.T) { }) } } + +func TestLogSoftErrors(t *testing.T) { + ws := &testutil.BufferedWriteSyncer{} + + l, err := logger.NewZapLogger(&logger.Config{}, logger.WithTap(zaputil.NewWriteEnabler(ws, zapcore.DebugLevel))) + require.NoError(t, err) + + errs := []error{ + errors.New("hard error 1"), + newSoftValidationErr(fmt.Errorf("soft error 1")), + errors.New("hard error 2"), + newSoftValidationErr(fmt.Errorf("soft error 2")), + errors.New("hard error 3"), + } + + err = LogSoftErrors(l, errs) + + // Verify that the helper consolidates errors. + require.Error(t, err) + require.ErrorContains(t, err, "hard error 1") + require.ErrorContains(t, err, "hard error 2") + require.ErrorContains(t, err, "hard error 3") + require.False(t, strings.Contains(err.Error(), "soft error 1")) + require.False(t, strings.Contains(err.Error(), "soft error 2")) + + // Verify that the helper logs soft errors. + logBuffer := ws.Buffer.String() + require.Contains(t, logBuffer, "soft error 1") + require.Contains(t, logBuffer, "soft error 2") + require.NotContains(t, logBuffer, "hard error 1") + require.NotContains(t, logBuffer, "hard error 2") + require.NotContains(t, logBuffer, "hard error 3") +} + +func TestJoinErrors(t *testing.T) { + errs := []error{ + errors.New("hard error 1"), + errors.New("hard error 2"), + newSoftValidationErr(fmt.Errorf("soft error 1")), + } + + err := JoinErrors(errs) + + require.Error(t, err) + require.ErrorContains(t, err, "hard error 1") + require.ErrorContains(t, err, "hard error 2") + require.False(t, strings.Contains(err.Error(), "soft error 1")) +} diff --git a/rpc/sip.go b/rpc/sip.go index a3276628d..8ce7adb5f 100644 --- a/rpc/sip.go +++ b/rpc/sip.go @@ -72,10 +72,10 @@ func NewCreateSIPParticipantRequest( projectID, callID, fromHostname, wsUrl, token string, req *livekit.CreateSIPParticipantRequest, trunk *livekit.SIPOutboundTrunkInfo, -) (*InternalCreateSIPParticipantRequest, error) { +) (*InternalCreateSIPParticipantRequest, []error) { req.Upgrade() - if err := req.Validate(); err != nil { - return nil, err + if errs := req.Validate(); errs != nil { + return nil, errs } var ( hostname string @@ -116,7 +116,7 @@ func NewCreateSIPParticipantRequest( outboundNumber := req.SipNumber if outboundNumber == "" { if trunk == nil || len(trunk.Numbers) == 0 { - return nil, psrpc.NewErrorf(psrpc.FailedPrecondition, "no numbers on outbound trunk") + return nil, []error{psrpc.NewErrorf(psrpc.FailedPrecondition, "no numbers on outbound trunk")} } outboundNumber = trunk.Numbers[rand.IntN(len(trunk.Numbers))] } diff --git a/rpc/sip_test.go b/rpc/sip_test.go index c47416409..cfd08dcdf 100644 --- a/rpc/sip_test.go +++ b/rpc/sip_test.go @@ -82,8 +82,8 @@ func TestNewCreateSIPParticipantRequest(t *testing.T) { Encryption: new(livekit.SIPMediaEncryption_SIP_MEDIA_ENCRYPT_REQUIRE), }, } - res, err := NewCreateSIPParticipantRequest("p_123", "call-id", "xyz.sip.livekit.cloud", "url", "token", r, tr) - require.NoError(t, err) + res, errs := NewCreateSIPParticipantRequest("p_123", "call-id", "xyz.sip.livekit.cloud", "url", "token", r, tr) + require.Empty(t, errs) require.True(t, proto.Equal(exp, res), "%v\nvs\n%v", exp, res) r.HidePhoneNumber = true @@ -91,8 +91,8 @@ func TestNewCreateSIPParticipantRequest(t *testing.T) { r.Media = &livekit.SIPMediaConfig{ Encryption: new(livekit.SIPMediaEncryption_SIP_MEDIA_ENCRYPT_ALLOW), } - res, err = NewCreateSIPParticipantRequest("p_123", "call-id", "xyz.sip.livekit.cloud", "url", "token", r, tr) - require.NoError(t, err) + res, errs = NewCreateSIPParticipantRequest("p_123", "call-id", "xyz.sip.livekit.cloud", "url", "token", r, tr) + require.Empty(t, errs) exp = &InternalCreateSIPParticipantRequest{ ProjectId: "p_123", SipCallId: "call-id", @@ -149,8 +149,8 @@ func TestNewCreateSIPParticipantRequest(t *testing.T) { } exp.ParticipantAttributes = expAttrs1 exp.ParticipantAttributes[livekit.AttrSIPTrunkID] = "" - res, err = NewCreateSIPParticipantRequest("p_123", "call-id", "xyz.sip.livekit.cloud", "url", "token", r, nil) - require.NoError(t, err) + res, errs = NewCreateSIPParticipantRequest("p_123", "call-id", "xyz.sip.livekit.cloud", "url", "token", r, nil) + require.Empty(t, errs) require.True(t, proto.Equal(exp, res), "%v\nvs\n%v", exp, res) } @@ -174,8 +174,8 @@ func TestNewCreateSIPParticipantRequestMediaConfig(t *testing.T) { r *livekit.CreateSIPParticipantRequest, tr *livekit.SIPOutboundTrunkInfo, media *livekit.SIPMediaConfig, ) { - res, err := NewCreateSIPParticipantRequest("p_123", "call-id", "xyz.sip.livekit.cloud", "url", "token", r, tr) - require.NoError(t, err) + res, errs := NewCreateSIPParticipantRequest("p_123", "call-id", "xyz.sip.livekit.cloud", "url", "token", r, tr) + require.Empty(t, errs) require.Equal(t, *media.Encryption, res.MediaEncryption) require.NotNil(t, res.Media) prototest.Equals(t, media, res.Media)