Skip to content
Permalink
Browse files

Allow 'text/plain' and 'text/html' content types for incoming SIP MES…

…SAGE

Fixed incoming SIP MESSAGE processing when the Content-Type header contains parameters
  • Loading branch information
jmillan committed Nov 19, 2012
1 parent e18ef6b commit 4e70a25f51352a1a187e4374581b71cc5c81cf52
Showing with 22 additions and 3 deletions.
  1. +22 −3 src/Message.js
@@ -159,19 +159,38 @@ JsSIP.Message.prototype.close = function() {
* @private
*/
JsSIP.Message.prototype.init_incoming = function(request) {
var contentType = request.getHeader('content-type');
var transaction,
contentType = request.getHeader('content-type');

this.direction = 'incoming';
this.local_identity = request.s('to').uri;
this.remote_identity = request.s('from').uri;

request.reply(200, JsSIP.c.REASON_200);
this.accept = function() {
request.reply(200, JsSIP.c.REASON_200);
};

if (contentType && contentType === "text/plain") {
this.reject = function(status_code, reason_phrase) {
if (status_code && reason_phrase) {
request.reply(status_code, reason_phrase);
} else {
request.reply(480, JsSIP.c.REASON_480);
}
};

This comment has been minimized.

Copy link
@ibc

ibc Nov 20, 2012

Member

I don't fully like the above. The user should be allowed to just pass the status code without reason and the above approach does not allow it (which I consider dangerous since it could generate an empty reason in the response or raise some error when converting null to string).

Proposal (pseudo-code):

this.reject = function(status_code, reason_phrase) {
  if (status_code && (status_code < 300 || status_code >= 700) {
    BUMPP("invalid status code, die now");
  }
  request.reply(status_code || 480, reason_phrase || JsSIP.c.REASON_480);
}

I hope this should be the approach in every function allowing passing status and reason as optional params.

BTW: Why aren't those accept() and reject() functions defined as prototype methods?

This comment has been minimized.

Copy link
@jmillan

jmillan Nov 22, 2012

Author Member

@ibc

My approach already verifies that the user inserts a reason_phrase. If not, default response (480) is sent. Documentation needed.

if (status_code && reason_phrase) {
     request.reply(status_code, reason_phrase);
} else {
     request.reply(480, JsSIP.c.REASON_480);
}

Yes, the status_code must be checked against split personality users (ej: reject(200, 'OK')) and other type of inconsistencies reject('asdfd').

About accept() and reply() in the prototype. You are rigtht: will be set in the prototype. I forgot for a moment that JsSIP must be perfect SIP stack.

Thanks.

This comment has been minimized.

Copy link
@ibc

This comment has been minimized.

Copy link
@jmillan

jmillan Nov 22, 2012

Author Member

Reason Pharse will be optional Mrs. Happy? :-)

This comment has been minimized.

Copy link
@ibc

ibc Nov 22, 2012

Member

Happy again!

if (contentType && (contentType.match(/^text\/plain(\s*;\s*.+)*$/i) || contentType.match(/^text\/html(\s*;\s*.+)*$/i))) {
this.ua.emit('newMessage', this.ua, {
originator: 'remote',
message: this,
request: request
});

transaction = this.ua.transactions.nist[request.via_branch];

if (transaction && (transaction.state === JsSIP.c.TRANSACTION_TRYING || transaction.state === JsSIP.c.TRANSACTION_PROCEEDING)) {
request.reply(200, JsSIP.c.REASON_200);

This comment has been minimized.

Copy link
@saghul

saghul Nov 20, 2012

Contributor

Why is this here and how is it related to the accept method? What happens if I call accept twice? Isn't adding accept and reject and API change?

This comment has been minimized.

Copy link
@jmillan

jmillan Nov 20, 2012

Author Member

In case the message is not accepted by the user, it is done by JsSIP by default.
If the user accepted or rejected the request, the transaction state won't match and no reply will be sent.
Regardless the user accepts or rejects many times, the request is replied only once because of the transaction state machine:. Once a final response is sent, its state is updated to 'completed' ; in such state, the following responses passed by the TU are discarded.

Yes, adding accept and reject is a minimal, backward compatible API change. I must review versioning rules. Perhaps these new methods should not be here.

This comment has been minimized.

Copy link
@saghul

saghul Nov 20, 2012

Contributor

Awesome :-) Yeah, if we plan to extend the 0.2.0 branch they don't belong. But don't be afraid of bumping the version, it's just numbers :-)

}
} else {
request.reply(415, JsSIP.c.REASON_415, ["Accept: text/plain, text/html"]);
}
};

0 comments on commit 4e70a25

Please sign in to comment.
You can’t perform that action at this time.