Skip to content

Commit

Permalink
Allow 'text/plain' and 'text/html' content types for incoming SIP MES…
Browse files Browse the repository at this point in the history 
…SAGE

Fixed incoming SIP MESSAGE processing when the Content-Type header contains parameters
  • Loading branch information
@jmillan
jmillan committed Nov 19, 2012
1 parent e18ef6b commit 4e70a25
Showing 1 changed file with 22 additions and 3 deletions.
25 changes: 22 additions & 3 deletions src/Message.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Sign in to view

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.

Sign in to view

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.

Sign in to view

Copy link
@ibc

ibc Nov 22, 2012

Member

http://goo.gl/fC2zW

This comment has been minimized.

Sign in to view

Copy link
@jmillan

jmillan Nov 22, 2012

Author Member

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

This comment has been minimized.

Sign in to view

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.

Sign in to view

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.

Sign in to view

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.

Sign in to view

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.