-
-
Notifications
You must be signed in to change notification settings - Fork 147
Improve SIP message parsers #271
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| package sip | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "fmt" | ||
| "strconv" | ||
| "strings" | ||
|
|
@@ -10,9 +11,9 @@ import ( | |
| // Some of headers parsing are moved to different files for better maintance | ||
|
|
||
| // A HeaderParser is any function that turns raw header data into one or more Header objects. | ||
| type HeaderParser func(headerName string, headerData string) (Header, error) | ||
| type HeaderParser func(headerName []byte, headerData string) (Header, error) | ||
|
|
||
| type mapHeadersParser map[string]HeaderParser | ||
| type HeadersParser map[string]HeaderParser | ||
|
|
||
| type errComaDetected int | ||
|
|
||
|
|
@@ -37,7 +38,7 @@ func (e errComaDetected) Error() string { | |
| // t To RFC 3261 | ||
| // u Allow-Events -events- "understand" | ||
| // v Via RFC 3261 | ||
| var headersParsers = mapHeadersParser{ | ||
| var headersParsers = HeadersParser{ | ||
| "c": headerParserContentType, | ||
| "content-type": headerParserContentType, | ||
| "f": headerParserFrom, | ||
|
|
@@ -66,56 +67,49 @@ func DefaultHeadersParser() map[string]HeaderParser { | |
| return headersParsers | ||
| } | ||
|
|
||
| // parseMsgHeader will append any parsed header | ||
| // In case comma seperated values it will add them as new in case comma is detected | ||
| func (headersParser mapHeadersParser) parseMsgHeader(msg Message, headerText string) (err error) { | ||
| // p.log.Tracef("parsing header \"%s\"", headerText) | ||
|
|
||
| colonIdx := strings.Index(headerText, ":") | ||
| // ParseHeader parses a SIP header from the line and appends it to out. | ||
| func (headersParser HeadersParser) ParseHeader(out []Header, line []byte) ([]Header, error) { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was avoiding to expose this, but I guess there is use case?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm quite happy with the current API having the |
||
| colonIdx := bytes.IndexByte(line, ':') | ||
| if colonIdx == -1 { | ||
| err = fmt.Errorf("field name with no value in header: %s", headerText) | ||
| return | ||
| return out, fmt.Errorf("field name with no value in header: %q", line) | ||
| } | ||
|
|
||
| fieldName := strings.TrimSpace(headerText[:colonIdx]) | ||
| lowerFieldName := HeaderToLower(fieldName) | ||
| fieldText := strings.TrimSpace(headerText[colonIdx+1:]) | ||
| fieldName := bytes.TrimSpace(line[:colonIdx]) | ||
| lowerFieldName := headerToLower(fieldName) | ||
| fieldValue := bytes.TrimSpace(line[colonIdx+1:]) | ||
|
|
||
| headerParser, ok := headersParser[lowerFieldName] | ||
| headerParser, ok := headersParser[string(lowerFieldName)] | ||
| if !ok { | ||
| // We have no registered parser for this header type, | ||
| // so we encapsulate the header data in a GenericHeader struct. | ||
| // We do only forwarding on this with trimmed space. Validation and parsing is required by user | ||
|
|
||
| header := NewHeader(fieldName, fieldText) | ||
| msg.AppendHeader(header) | ||
| return nil | ||
| h := NewHeader(string(fieldName), string(fieldValue)) | ||
| out = append(out, h) | ||
| return out, nil | ||
| } | ||
|
|
||
| // Support comma seperated value | ||
| fieldText := string(fieldValue) | ||
| // Support comma separated values | ||
| for { | ||
| // We have a registered parser for this header type - use it. | ||
| // headerParser should detect comma (,) and return as error | ||
| header, err := headerParser(lowerFieldName, fieldText) | ||
|
|
||
| // Mostly we will run with no error | ||
| h, err := headerParser(lowerFieldName, fieldText) | ||
| if err == nil { | ||
| msg.AppendHeader(header) | ||
| return nil | ||
| out = append(out, h) | ||
| return out, nil | ||
| } | ||
|
|
||
| commaErr, ok := err.(errComaDetected) | ||
| if !ok { | ||
| return err | ||
| return out, err | ||
| } | ||
|
|
||
| // Ok we detected we have comma in header value | ||
| msg.AppendHeader(header) | ||
| out = append(out, h) | ||
| fieldText = fieldText[commaErr+1:] | ||
| } | ||
| } | ||
|
|
||
| func headerParserCallId(headerName string, headerText string) (header Header, err error) { | ||
| func headerParserCallId(headerName []byte, headerText string) (header Header, err error) { | ||
| var callId CallIDHeader | ||
| return &callId, parseCallIdHeader(headerText, &callId) | ||
| } | ||
|
|
@@ -131,7 +125,7 @@ func parseCallIdHeader(headerText string, callId *CallIDHeader) error { | |
| return nil | ||
| } | ||
|
|
||
| func headerParserMaxForwards(headerName string, headerText string) (header Header, err error) { | ||
| func headerParserMaxForwards(headerName []byte, headerText string) (header Header, err error) { | ||
| var maxfwd MaxForwardsHeader | ||
| return &maxfwd, parseMaxForwardsHeader(headerText, &maxfwd) | ||
| } | ||
|
|
@@ -143,7 +137,7 @@ func parseMaxForwardsHeader(headerText string, maxfwd *MaxForwardsHeader) error | |
| return err | ||
| } | ||
|
|
||
| func headerParserCSeq(headerName string, headerText string) (headers Header, err error) { | ||
| func headerParserCSeq(headerName []byte, headerText string) (headers Header, err error) { | ||
| var cseq CSeqHeader | ||
| return &cseq, parseCSeqHeader(headerText, &cseq) | ||
| } | ||
|
|
@@ -169,7 +163,7 @@ func parseCSeqHeader(headerText string, cseq *CSeqHeader) error { | |
| return nil | ||
| } | ||
|
|
||
| func headerParserContentLength(headerName string, headerText string) (header Header, err error) { | ||
| func headerParserContentLength(headerName []byte, headerText string) (header Header, err error) { | ||
| var contentLength ContentLengthHeader | ||
| return &contentLength, parseContentLengthHeader(headerText, &contentLength) | ||
| } | ||
|
|
@@ -182,7 +176,7 @@ func parseContentLengthHeader(headerText string, contentLength *ContentLengthHea | |
| } | ||
|
|
||
| // headerParserContentType parses ContentType header | ||
| func headerParserContentType(headerName string, headerText string) (headers Header, err error) { | ||
| func headerParserContentType(headerName []byte, headerText string) (headers Header, err error) { | ||
| var contentType ContentTypeHeader | ||
| return &contentType, parseContentTypeHeader(headerText, &contentType) | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could understand this comes from line buffer, but value is mostly not used, so not sure do we need this change anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally it would be great to switch values to byte slices - that should give even better performance. But that would be a very large change. So I only did header names for now.
Answering your question, tbh I'm not sure why it helps here specifically. As you mentioned, the string here is unused and is mostly generated from static strings in the lower case header names switch. So it should not matter if we do bytes here. But it does affect performance in the end.