Hi,

> Ok, I was sloppy and didn't run the tests before I merged this. It turns out we have test 569
> that verifies that we don't accept spaces as part of the RTSP session id. Like this:
>
> Session: \$extraspaces ignore-this-part-------;foo=bar
>
> So, do you think we should fix the test and allow the entire string to be the session (including
> spaces) or should we fix the code and also stop the parsing at the first space?

The sloppyness is on my side. I didn't look at the tests, and this test has a clear origin in section 3.4 of the RFC so it deserves attention.
You just jump to conclusions very fast :) but you have a process that protects you!

I think the RFC is sloppy on whitespace. For example, it doesn't specify whitespace at all but every header starts with one after the :. But on spaces in the session-id it is explicit.

See section 3.4 in https://www.ietf.org/rfc/rfc2326.txt

 I see a few ways to respect/interpret/deal with this:

- refuse a session-id as soon as any space if found before ; or CRLF whichever comes first
- gracefully accept right-trim the value first because the rfc has vagueness on the subject of whitespace
- ignore the problem, as I made it now all valid session-ids will be accepted, as well as some invalid ones.

There is a philosophy aspect. Does curl strive to accept valid input only, or does it mean to handle as good as possible (but 'no guarantees') flaky input? My style is the latter, I was once responsible for a development where we made a very strict RTSP client and we suffered from that decision because no IP camera manufacturer seemed to implement their server decently. (Well.. nowadays I work for one :)). So I tend to make my code such that as long as it can (at low cost), it will work with flaky input. I do not work in nuclear plants and life saving equipment.

What is the curl philosophy? I think the choice should be made according to it.
- Work as good as possible with incorrect input? -> remove the test.
- Demand proper server behavior to protect from unknown potential serious problems later in the flow? -> I need to change my fix because it is not strict enough

Regards,

Erik

-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
Received on 2016-08-11