[sr-dev] [kamailio/kamailio] cannot parse From header (#2723)

[sergey-safarov]

### Description

For some reason `From` header cannot be parsed.

#### Reproduction

Tested config
```
listen=udp:127.0.0.1:5060
loadmodule "pv.so"
loadmodule "http_client.so"
loadmodule "lost.so"

modparam("http_client", "httpcon", "http://example.com=>http://example.com");


request_route {
lost_held_query("http://example.com", "$var(pidf)", "$var(geo_url)", "$var(err)");
}
```
Client sipp
```
sipp -m 1 -sn uac -p 2345 127.0.0.1
```

#### Log Messages

Kamailio output
````
1(7643) WARNING: lost [functions.c:252]: lost_held_function(): P-A-I header not found, trying From header ...
1(7643) ERROR: lost [utilities.c:644]: lost_get_from_header(): From header not found
1(7643) ERROR: lost [functions.c:259]: lost_held_function(): no device id found
2(7644) WARNING: lost [functions.c:252]: lost_held_function(): P-A-I header not found, trying From header ...
2(7644) ERROR: lost [utilities.c:644]: lost_get_from_header(): From header not found
2(7644) ERROR: lost [functions.c:259]: lost_held_function(): no device id found
3(7645) WARNING: lost [functions.c:252]: lost_held_function(): P-A-I header not found, trying From header ...
3(7645) ERROR: lost [utilities.c:644]: lost_get_from_header(): From header not found
3(7645) ERROR: lost [functions.c:259]: lost_held_function(): no device id found
````

#### SIP Traffic

```
INVITE sip:***@127.0.0.1:5060 SIP/2.0
Via: SIP/2.0/UDP 127.0.0.1:2345;branch=z9hG4bK-7655-1-0
From: sipp <sip:***@127.0.0.1:2345>;tag=7655SIPpTag001
To: service <sip:***@127.0.0.1:5060>
Call-ID: 1-***@127.0.0.1
CSeq: 1 INVITE
Contact: sip:***@127.0.0.1:2345
Max-Forwards: 70
Subject: Performance Test
Content-Type: application/sdp
Content-Length: 129

v=0
o=user1 53655765 2353687637 IN IP4 127.0.0.1
s=-
c=IN IP4 127.0.0.1
t=0 0
m=audio 6000 RTP/AVP 0
a=rtpmap:0 PCMU/8000
```


### Additional Information
current master

* **Operating System**:

CentOS 8 x86_64

[Wolfgang Kampichler]

lost module uses the standard header parsing function. `lost_get_from_header` is just a wrapper function to return a string pointer and uses `parse_headers`, which in your case returns an error. I did use the sipp example but had no header parsing issues, see below (OS: alpine:latest docker image)
```
6(13) INFO: {1 1 INVITE 1-***@127.0.0.1} <script>: ### ESRP - INVITE request received
Sock: tcp:172.16.16.176:38763
From: sip:***@127.0.0.1:2345
To: sip:***@127.0.0.1:5060
###
6(13) INFO: {1 1 INVITE 1-***@127.0.0.1} <script>: INVITE urn:service:sos SIP/2.0
Via: SIP/2.0/UDP 127.0.0.1:2345;branch=z9hG4bK-7655-1-0
From: sipp <sip:***@127.0.0.1:2345>;tag=7655SIPpTag001
To: service <sip:***@127.0.0.1:5060>
Call-ID: 1-***@127.0.0.1
CSeq: 1 INVITE
Contact: sip:***@127.0.0.1:2345
Max-Forwards: 70
Subject: Performance Test
Content-Type: application/sdp
Content-Length: 129

v=0
o=user1 53655765 2353687637 IN IP4 127.0.0.1
s=-
c=IN IP4 127.0.0.1
t=0 0
m=audio 6000 RTP/AVP 0
a=rtpmap:0 PCMU/8000

6(13) WARNING: {1 1 INVITE 1-***@127.0.0.1} lost [functions.c:257]: lost_held_function(): P-A-I header not found, trying From header ...
6(13) INFO: {1 1 INVITE 1-***@127.0.0.1} lost [functions.c:271]: lost_held_function(): ### HELD id [sip:***@127.0.0.1:2345]
6(13) WARNING: {1 1 INVITE 1-***@127.0.0.1} lost [functions.c:480]: lost_held_function(): locationRequest error response: [locationUnknown]
6(13) INFO: {1 1 INVITE 1-***@127.0.0.1} <script>: ### ESRP - held request failed
6(13) INFO: {1 1 INVITE 1-***@127.0.0.1} <script>: ### ESRP - held request returned err:
locationUnknown
6(13) INFO: {1 1 INVITE 1-***@127.0.0.1} <script>: ### ESRP - service urn received
6(13) INFO: {1 1 INVITE 1-***@127.0.0.1} <script>: INVITE urn:service:sos SIP/2.0
Via: SIP/2.0/UDP 127.0.0.1:2345;branch=z9hG4bK-7655-1-0
From: sipp <sip:***@127.0.0.1:2345>;tag=7655SIPpTag001
To: service <sip:***@127.0.0.1:5060>
Call-ID: 1-***@127.0.0.1
CSeq: 1 INVITE
Contact: sip:***@127.0.0.1:2345
Max-Forwards: 69
Subject: Performance Test
Content-Type: application/sdp
Content-Length: 129

v=0
o=user1 53655765 2353687637 IN IP4 127.0.0.1
s=-
c=IN IP4 127.0.0.1
t=0 0
m=audio 6000 RTP/AVP 0
a=rtpmap:0 PCMU/8000

6(13) INFO: {1 1 INVITE 1-***@127.0.0.1} <script>: ###
6(13) INFO: {1 1 INVITE 1-***@127.0.0.1} lost [functions.c:891]: lost_function(): ### LOST urn [urn:service:sos]
6(13) ERROR: {1 1 INVITE 1-***@127.0.0.1} lost [functions.c:923]: lost_function(): geolocation header not found
```

[sergey-safarov]

My test results for commit 2058676daacc2feded18b872bd53c0514710d4bb inside docker containers
1. Fedora 33 - issue present;
2. CentOS 8 - issue present;
3. CentOS 7 - issue reproduced;
4. Alpine v3.13 - issue reproduced;

Also, make the same test on VM with CentOS 8 - issue reproduced.
Something strange here.

[Wolfgang Kampichler]

Can you share the original xml file used with sipp?

[Daniel-Constantin Mierla]

Try to replace parse_headers() with parse_from_header() inside lost_get_from_header(), the sample config file above is minimal, not many headers are deep-parsed automatically. Default config does lot of other operations, which can result in headers being parsed in detail.

[sergey-safarov]

@wkampich original XML file located here
http://sipp.sourceforge.net/doc/uac.xml.html

RAW format
http://sipp.sourceforge.net/doc/uac.xml

[sergey-safarov]

@miconda yes, `parse_from_header` function resolved this error.

[sergey-safarov]

Yes, I call `lost_held_query` at the start of routing.
This describes by not all headers parsed.
What is connect solution here?

[Wolfgang Kampichler]

@sergey-safarov could you retry with the following change in `lost_get_from_header`
```
if(parse_headers(msg, HDR_EOH_F, 0) == -1) {
LM_ERR("failed to parse SIP headers\n");
return res;
}
```
basically replacing the flag `HDR_FROM_F` with `HDR_EOH_F`

[sergey-safarov]

basically replacing the flag HDR_FROM_F with HDR_EOH_F

This not help and errors still present.

[Wolfgang Kampichler]

Finally, I was able to reproduce the problem by removing a log message to print the From header at the beginning of the routing and calling `lost_held_query` directly. Since `parse_headers()` is also used to get Geolocation and P-A-I header content I did not remove it, but changed the method for parsing the From content. The fix worked at least in my local setup. @sergey-safarov thanks for testing and could you please retry with latest commit.

[Daniel-Constantin Mierla]

@wkampich: why not using the `parse_from_header()` like I suggested in my previous commit here instead of doing the commit fe6923cd8e995c400fc1838304c95142cc00273e? You try to reimplement `parse_from_header()`. The From header must be only one time in the sip message, parse_to() may be changed in the future, adding complexity to maintenance of the code, ... Unless there is a clear reason to do it like in fe6923cd8e995c400fc1838304c95142cc00273e, it should be reverted and use the `parse_from_header()`.

[Wolfgang Kampichler]

@miconda I want to be sure that `parse_headers()` is not the cause of the issue as it is also used to get Geolocation and P-A-I header content. If it is sure that `parse_headers` is not the problem, then it is ok to use `parse_from_header()`.

[Daniel-Constantin Mierla]

`parse_headers()` is mainly breaking in `header name` and `header body`, not doing deep parsing of `header body` part for most of the headers, but only for a few. To get the From header body parsed, you have to use `parse_from_header()`, not parse it internally using parse_to(). You can still use `parse_headers(msg, HDR_EOH_F, 0)` to locate other headers, just do not expect to do deep parsing of every header, there are dedicated functions that have to be used (e.g., for Contact, Route, ...).

[Wolfgang Kampichler]

@miconda thanks for the clarification, I will switch to `parse_from_header()` and also check if `parse_pai_header()` fits.

[Daniel-Constantin Mierla]

I guess this can be closed.

@sergey-safarov: as you seem to make many testing scenarios with sipp to reproduce issues/test features, maybe you can contribute them as units in the kamailio-tests repo:

* https://github.com/kamailio/kamailio-tests

Can be useful to detect regressions/changes in behaviour.

[sergey-safarov]

Just tested the current master.
Issue fixed.

To develop a unit test for this case assign the ticket to me.

[Daniel-Constantin Mierla]

@sergey-safarov closing this one, being aged. If you have time for a test case in the near future, just push it to repo https://github.com/kamailio/kamailio-tests .

[Daniel-Constantin Mierla]

Closed #2723.