Agree, especially on the ground that spaces are such problematic characters.

I appreciate the way you give your attention to all these little things from all over world.

This patch is the 3rd and last one of adaptions that I had to make, now that I am in sync with the mainline very soon the slate will be clean to finally put my attention to the remaining issue of the libcurl RTSP implementation: mix up of callback handlers that occur when having multiple sessions open with a single host in the multi interface. That one is less isolated and requires deeper understanding of the lib. Still climbing that hill :)

Erik

-----Original Message-----
From: Daniel Stenberg [mailto:daniel_at_haxx.se]
Sent: donderdag 11 augustus 2016 13:25
To: Erik Janssen <Erik.Janssen_at_axis.com>
Cc: libcurl development <curl-library_at_cool.haxx.se>
Subject: RE: [Patch] accept any RTSP session id

On Thu, 11 Aug 2016, Erik Janssen wrote:

> 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

We're basically all over the map but we lean towards the later. We follow specs when everyone else does that and as far as possible, but when other players aren't following the rules we are pragmatic and lean towards going with some sort of sensed majority or loosened strictness. Compatibility with the ecosystem is more valuable than a moral high ground. But sometimes we put down our heels into the ground and fight hard for that moral high ground - just saying that we should never just back down needlessly.

> 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

How about a middle ground?

Your patch made us accept *anything* until the semicolon based on the fact that there are servers sending illegal characters in that string and that others accept those.

But are they ever sending whitespaces and assuming the clients to also eat those and use them in the session id? Or are there even servers sending whitespace that _known_ that clients strip them off before proceeding. In that case, this patch breaks compatibility with them.

So, I'm suggeseting that we should make the code accept only non-whitespace as part of the session id string. Simply based on the fact that whitespace is usually always a problem in protocols and it feels unlikely that servers will actually depend on clients sending such back. I don't *know* that, this juch a hunch.

I'm proposing this little change:

- while(*end && *end != ';')
+ while(*end && *end != ';' && !ISSPACE(*end))
          end++;

-- 
  / daniel.haxx.se
-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:  https://curl.haxx.se/mail/etiquette.html