search

aboutsip / pkts

Pure java based pcap library capable of reading and writing to/from pcaps.
Other
198 stars 92 forks source link

Thoughts about SipParseExceptions (regarding issue #38 and #39) #42

Open thaeger71 opened 8 years ago

thaeger71 commented 8 years ago

Hello,

I'am not sure if someone still care about this project or not, but just in case, here're my thoughts:

I've posted issue 38 and 39 in the assumption that these maybe are errors. I found out it was errors, but errors caused by not correct header field values which are not corresponding to their specifaction. In my opinion this is not a task for a package parser like pkts. Because to throw Exceptions means that the package never occurs in application and even the Exception handling seems not to be correct, never throw an Execption out to the logs/stderr or another Exception will be thrown caused by a strange problem with the condtructor function of the class SipParseException. I'am not saying that these informations wouldn't be useful they would, if they could be stored in another way, attached somehow to the packet. If these format errors still throwing exceptions this library could not be used for analyzing SIP traffic for example, or just give the application layer a chnace to say to the counterpart that there is something wrong with the header field X. I thought about a way to store these informations from defined exceptions somehow on the package, but in most cases where the exception will be thrown, there is no connection to the packet, only the buffer or a part of the package buffer .... I would appreciate your ideas in that case. Otherwise I would propose that all format errors has just to be commented out, so that ALL SIP packages get into the application layer. Would you guys agree, I would make a fork and a pull request.

aboutsip commented 8 years ago

Hi and thanks for your filed issues. The project is a personal project of mine and as such I only work on it when I find the time, which sometimes is hard. Also, in regards to this particular issue, the sip packet is part of my other project sipstack.io, where it does make sense to throw exceptions for those headers that are mandatory of any SIP message. However, I do see your point as well so let me think about it.

In regards to some of the other issues you have filed, such as the annoying thing that pcaps over 2 gigs will blow up memory, feel free to issue pull requests. That particular one I may fix myself soon because at work I have similar issues (work at twilio.com).

Thanks,

Jonas Borjesson

thaeger71 commented 8 years ago

Hi Jonas,

thanks for your response. I'am glad to see that someone feels responsible for this project ;-). Sorry for the directness, but I wasn't sure that someone cares about my postings because nobody answered to my requests. Even though I appreciate all your work and want to say thank you :-)

Meanwhile I found the reasons for issue 38 and 39 and have just changed the code for myself for the first. But as I wrote, this is a fundamental problem and I think, also for your sipstack project, it would be make more sense to shift this, I will call it "RFC error handling" to the stack, and not to the parser, OR, better just to pin these infos somehow to the packet, so that the application can see them and decide what to do. But it looks like this would be a bigger site. I appriciate that you will think about that and I'am curious to your coming thoughts. If I have any further ideas I will inform you too.

Regarding the 2 GB problem, I just have made a workaround in my app in the way that I splitt the pcap files if they are >= 2 GB, and walk through the splitted files one after one to parse all the data. I tried to figure out what the problem is with the file size, but until now I didn't find the reason ....

Greets,

Thomas.