Skip to content
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

Event.prototype.serialize(plain) - single replace of NL with %0A breaks Content-Length #63

Closed
mhmeadows63 opened this issue Mar 7, 2017 · 7 comments

Comments

@mhmeadows63
Copy link

The Content-Length header is calculated before the first \n only is replaced with %0A.

Apart from the content-length then being wrong, is this replace strictly necessary?

In my case, this body:
    <ATM>\n\t<version>1.5</version>\n\t<data>00000</data>\n</ATM>
becomes:
    <ATM>%0A\t<version>1.5</version>\n\t<data>00000</data>\n</ATM>
which is nonsense to the recipient.

@englercj
Copy link
Owner

englercj commented Mar 8, 2017

The content-length being calculated too early is certainly a bug. As for the replace, at first I thought the issue was just that it didn't replace them all (i.e. it should be: .replace(/\n/g, '%0A'). However, reading back through this code (it has been years since I worked on it) it seems like it is not needed.

So the fixes would likely be to calculate content-lengths properly for each of the types as well as not doing that replace on the body.

@mhmeadows63
Copy link
Author

mhmeadows63 commented Mar 8, 2017

I agree that the plain-mode replace should be removed.

The json-mode calculation looks fine because the stringify wraps the whole event, not just the body.

As for the xml-mode calculation, there may be a need for htmlentity encoding before taking the length, however, I'm not clear on the intended use case.

It may be that Content-Length is only necessary in plain-mode, and is otherwise superfluous.

@englercj
Copy link
Owner

englercj commented Mar 9, 2017

The content length is necessary since it tells you how many bytes of the stream to read for the body. Without that you wouldn't know how many bytes to parse for the message, and would have to do contextual parsing (which this lib and fsw don't do).

Since Content-Length is the byte-length of the body to be read form the stream, it should be calculated after the body is completely encoded.

There is a difference between the body and the message as well. A plain message sent over the stream actually looks like this:

Content-Type: text/event-plain
Content-Length: 700

Event-Name: CUSTOM
Core-UUID: 8b192020-7368-4498-9b11-cbe10f48a784
FreeSWITCH-Hostname: smsdev
FreeSWITCH-Switchname: smsdev
FreeSWITCH-IPv4: 10.1.12.115
FreeSWITCH-IPv6: %3A%3A1
Event-Date-Local: 2012-09-25%2014%3A21%3A56
Event-Date-GMT: Tue,%2025%20Sep%202012%2018%3A21%3A56%20GMT
Event-Date-Timestamp: 1348597316736546
Event-Calling-File: switch_cpp.cpp
Event-Calling-Function: Event
Event-Calling-Line-Number: 262
Event-Sequence: 11021
Event-Subclass: SMS%3A%3ASEND_MESSAGE
proto: sip
dest_proto: sip
from: 9515529832
from_full: 9515529832
to: internal/8507585138%40sms-proxy-01.bandwidthclec.com
subject: PATLive%20Testing
type: text/plain
hint: the%20hint
replying: true
Content-Length: 4

Body

First is the headers for the message followed by the event as the body of that message. Within the event are the headers followed by the body of that event.

JSON would look like this:

Content-Type: text/event-json
Content-Length: 920

{
    "Event-Name": "CUSTOM",
    "Core-UUID": "8b192020-7368-4498-9b11-cbe10f48a784",
    "FreeSWITCH-Hostname": "smsdev",
    "FreeSWITCH-Switchname": "smsdev",
    "FreeSWITCH-IPv4": "10.1.12.115",
    "FreeSWITCH-IPv6": "::1",
    "Event-Date-Local": "2012-09-25 14:22:37",
    "Event-Date-GMT": "Tue, 25 Sep 2012 18:22:37 GMT",
    "Event-Date-Timestamp": "1348597357036551",
    "Event-Calling-File": "switch_cpp.cpp",
    "Event-Calling-Function": "Event",
    "Event-Calling-Line-Number": "262",
    "Event-Sequence": "11027",
    "Event-Subclass": "SMS::SEND_MESSAGE",
    "proto": "sip",
    "dest_proto": "sip",
    "from": "9515529832",
    "from_full": "9515529832",
    "to": "internal/8507585138@sms-proxy-01.bandwidthclec.com",
    "subject": "PATLive Testing",
    "type": "text/plain",
    "hint": "the hint",
    "replying": "true",
    "Content-Length": "23",
    "_body": "Hello from Chad Engler!"
}

Even though the content-length in the event for json/xml is superfluous, there is a chance fsw might be using it even in those scenarios because it does send it. For consistency sake if nothing else, I'd rather keep it in.

@englercj
Copy link
Owner

englercj commented Mar 9, 2017

Just published v1.1.9 of modesl. Hopefully that fixes your issue!

@englercj englercj closed this as completed Mar 9, 2017
@mhmeadows63
Copy link
Author

Just reviewed your 1.1.9 changes:

In xml-mode you have omitted to test whether a body is present to encode.

Also, in plain-mode, you have left the comment that talks of urlencoding NLs

@englercj
Copy link
Owner

englercj commented Mar 11, 2017

Xml does check if the body exist, just under the comment that says "add body". The encodeXml function returns empty string if passed an empty body so no need to check twice.

I should remove that comment at some point, but is the change working for you?

@mhmeadows63
Copy link
Author

I can confirm that the change is working for me.

Many thanks.

davehorton pushed a commit to davehorton/node-esl that referenced this issue Sep 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants