@sergey-safarov commented on this pull request.

+ if(ret == 1) { + /* match on host:port, but if gruu, then fail */ + if(_puri->gr.s != NULL) + return 0; + } + + return ret; +} + +/*! + * \brief Parse the message and find first occurrence of Route header field. + * \param _m SIP message + * \return -1 or -2 on a parser error, 0 if there is a Route HF and 1 if there is no Route HF + */ +static inline int find_first_route(struct sip_msg *_m)

I'm fine with using the core function. Could core developers create these core functions and then we will use in this PR.

@sergey-safarov commented on this pull request.

• return CHECK_FLOW_ERROR_URI_NOT_MYSELF;
  

+ } + + + LM_DBG("topmost route URI: '%.*s' is me\n", uri.len, ZSW(uri.s)); + ret = decode_flow_token(msg, &rcv, puri.user); + + if(ret == -2) { + LM_DBG("no flow token found\n"); + return CHECK_FLOW_ERROR_NO_FLOW_TOKEN; + } else if(ret == -1) { + LM_DBG("failed to decode flow token\n"); + return CHECK_FLOW_ERROR_DECODE; + } else if(rcv->proto == PROTO_TCP || rcv->proto == PROTO_TLS + || rcv->proto == PROTO_WS || rcv->proto == PROTO_WSS) { + tcp_connection_t *con =

done

@ivanuschak commented on this pull request.

• 	return 1;
  

+ } + } +} + +int check_flow_token(struct sip_msg *msg) +{ + struct hdr_field *hdr; + struct sip_uri puri; + rr_t *rt; + str uri; + int ret; + struct receive_info *rcv = NULL; + tcp_connection_t *con = NULL; + + switch(find_first_route(msg)) {

please check `find_first_route` function implementation. It returns 4 values: -2, -1, 0, 1. The value `1` is returned when no route header found. Service crashes when execution goes over this function and `sip_msg` does not contain Route header. Need to add `1` in the switch. Please fix.

@sergey-safarov commented on this pull request.

+int check_flow_token(struct sip_msg *msg) +{ + struct hdr_field *hdr; + struct sip_uri puri; + rr_t *rt; + str uri; + int ret; + struct receive_info *rcv = NULL; + tcp_connection_t *con = NULL; + + switch(find_first_route(msg)) { + case -1: + LM_DBG("there is no Route HF\n"); + return CHECK_FLOW_NO_ROUTE_HEADER; + break;

done

miconda left a comment (kamailio/kamailio#4449)

I added the functions (with different names) to core with the commits 521c974a200569b39f48b397853f50299d06ae7e and d11a33dc80431b69a3e7f29e1b00071ca24b5f04 . I will update rr module to use them.

@sergey-safarov commented on this pull request.

+ if(ret == 1) { + /* match on host:port, but if gruu, then fail */ + if(_puri->gr.s != NULL) + return 0; + } + + return ret; +} + +/*! + * \brief Parse the message and find first occurrence of Route header field. + * \param _m SIP message + * \return -1 or -2 on a parser error, 0 if there is a Route HF and 1 if there is no Route HF + */ +static inline int find_first_route(struct sip_msg *_m)

done

miconda left a comment (kamailio/kamailio#4449)

The description of the commit lists return 0, which should not be done by a module config function because it stops config execution -- this as a note, because it seems the code is returning negative and positive int values. But I would suggest to add explicit values to the enum fields, not only the -7 and 1, it should be safer in the future and easier to understand when looking at the code, avoiding shifting values when a new field might be added:

``` typedef enum check_flow_retcode { CHECK_FLOW_ERROR_HEADERS_PARSE = -7, CHECK_FLOW_ERROR_ROUTE_PARSE, CHECK_FLOW_EXPIRED, CHECK_FLOW_NO_TCP_CONNECTION, CHECK_FLOW_ERROR_DECODE, CHECK_FLOW_ERROR_NO_FLOW_TOKEN, CHECK_FLOW_ERROR_URI_NOT_MYSELF, CHECK_FLOW_NO_ROUTE_HEADER = 1, CHECK_FLOW_SUCCESS } check_flow_retcode_t; ```

Then, in my opinion CHECK_FLOW_NO_ROUTE_HEADER belongs better to the negative/failure range (false in native scripting), and have only CHECK_FLOW_SUCCESS as positive/success (true).

sergey-safarov left a comment (kamailio/kamailio#4449)

@miconda I have updated the ENUM definition as proposed in the previous message, and updated description the XML documentation. Also fixed the commit description.

sergey-safarov left a comment (kamailio/kamailio#4449)

Maybe the `check_flowtoken` function name is more preferred than `check_flow_token`?

Merged #4449 into master.