cURL / Mailing Lists / curl-library / Single Mail

curl-library

RE: SF bug 1456 and associated commits

From: Steve Holme <steve_holme_at_hotmail.com>
Date: Tue, 16 Dec 2014 02:04:20 +0000

On Mon, 15 Dec 2014, Patrick Monnerat wrote:

> > > What do we do ?
>
> > As there are only two weeks until release I think we should:
>
> > * Hold off on automatic conversion until after the release of 7.40.0 -
> > this gives others time to contribute and assist in the debate about
> > whether this should or shouldn't be done

I'm going to try and keep this brief as its gone midnight. I've only just eaten but I guess that's what I get for a long day at work (after a couple of days off - I returned to mayhem!), a visit to the gym and delayed trains. As such please excuse me I sound a little blunt - it's not my intention :(

> An implementation that requires a special (and incorrect) option to be
> set as an OS dependence to make the protocol working is a BUG. I think
> we should include the correct end of line processing immediately, whatever
> other protocols will require.

I disagree. The original bug report (1456) was:

a) That --crlf didn't work for SMTP - In summary this was implemented as per the user's request, as a feature for 7.40, before the feature freeze.

b) That dot stuffing didn't work as expected - This has now been fixed, after the feature freeze as a bug fix, and with thanks to yourself. Cheers again - the hint was all I needed to fix that.

What you're suggesting, whilst related, is against how SMTP has worked since it was first introduced in 7.20.0. I appreciate you may disagree with the feature I implemented but a) that is what the user requested, b) I agree with him as it is in keeping with how curl works elsewhere and c) that is how it was documented to work for SMTP prior to my changes.

I appreciate that it is annoying to have to pass --crlf on a Linux machine or any other machine (inc. Windows) that happens to use a file using LF or even CR line endings for that matter. I use files on Windows where the EOL characters are the normal CRLF but also files that use LF - the conversion is just something I have got used to. For example: If I push a patch that someone else has created and sent to the mailing list, sent to me privately, or I downloaded a merge request patch from github I have to make sure that the patch uses LF line endings before I push - otherwise I end up sending CRLF to the repo :( Obviously I don't for my own commits due to the nature I have TortoiseGit (msysgit underneath) set up.

The auto conversion that you are taking about hasn't been present in the last 31 releases - either SMTP is not used from Linux (which I doubt) or everyone that uses it understands that the content of the data is RAW and has to use CRLF line endings (as per the RFC).

Not only that but what you are proposing also takes away the ability and opportunity to send content to a server using LF line endings (as can be done at present - and again this is something that has been present for 31 releases). Whilst this is against the RFC and curl is true in spirit to most RFCs, it doesn't always stick to them per se. Instead it and libcurl allow a user or programmer to sometimes do what they want with a server. For example: Just look at what can be achieved with --request and how wrongly that can be used!

I have some ideas about a) how we can achieve backwards compatibility with the current usage, b) move to automatic conversion and c) implement this for other protocols, but I need to think about some of this a little more before I discuss it with the list as and I will because I a) Value your opinion and want to get you involved because I know you worked on SMTP before I came on board in 7.22.0, b) Would like to get Dan's opinion because (and I could be wrong here) I believe he implemented the initial --crlf and has a lot of experience with FTP, c) Would like the opportunity for others to comment and d) Would like Daniel's sign off as project lead for curl.

What I have implemented is working as intended - it may be ideal and you could view it as a work around.

Given all of this I don't think it can be classed as a bug but instead should be classified as a feature enhancement. If there are still bugs with what I have implemented then please feel free to let me know and I will endeavour to fix them ASAP.

As such, I think January would be a good time to flush this out properly especially when you bear in mind that we are trying fix some of the outstanding bugs from Source Forge before the release (As well as personally I need to spend some time fixing issues in my GSS-API changes and work with Bill to fix some of the SMB issues).

> > And please note: the CURLOPT_CRLF already mentions:
> > "This is a legacy option of questionable use.".

Yes it does. I don't know why that has been written - I can only guess it is because we don't have a full understanding as to which protocols use it or even should use it. Technically it is implemented for every protocol apart from SMTP (and prior to my changes the documentation incorrectly included SMTP as I mentioned earlier). However, as I think you mentioned in a previous email (possibly in passing - I can't quite remember) this isn't really applicable for non-textual protocols. As such, and I could again be wrong here, but it may be more applicable to the "Ping Pong" protocols that we support.

> For the time being, I won't commit anything: I've worked 10hours on
> this.

Whilst you're work is much appreciated on this, please don't quote how much time you have spent on something - everyone that is involved with the project spends an amount of time (mostly voluntary) on the project and in my opinion it doesn't matter how great or small it is - whether it is coding, fixing bugs, reporting issues, testing, writing documentation, updating the website etc... it is all useful in one way or another. In the 3 and half years I have been involved with the project I have 1400+ commits under my belt and I hate to think of the amount of time I have spent on the project - I typically spend most of my free time doing something for curl as I found myself single again 4 years ago (at the age of 38 - am now 42) and I'm not in a rush to get back get into another one - but that's my choice - and as such I am fortunate enough to dedicate the amount of time I do to the project, socialise with team members where the opportunity allows and attend things like FOSDEM ;-) You'll note I'm less involved
during the summer months as I'm normally out on the motorcycle at weekends and one day due to unforeseen circumstances I may not be involved at all - I can't imagine that at the moment but you never know what's round the corner ;-)

Not only that, but also please bear in mind that, the feature / bug report was already underway when you started your own work on it, when it was already assigned to me in Source Forge - in that respect sorry if you feel I have trodden on your toes so to speak.

> The tests I've written assume the data is compliant while they currently
> are not. And I won't restart the work for corollary bugs on an incorrect
> implementation.

I will leave that decision with you but I would I would really value your feedback on the questions I raised about the initial dot and empty email issues you found, so if you could find your way to answer those that would be fantastic - many thanks in advance.

Sorry that ended up being a long email (and I've just realised is now gone 1:30) so time for bed - but I didn't want to keep you waiting for a response especially when I don't think I will get time to respond later today as I have another hectic day ahead of me :(

Also please feel free to email me off list if you want / need to discuss anything.

Kind Regards

Steve

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2014-12-16