httpd/carddav from git sources

Johan Hattne johan at hattne.se
Wed Jun 29 08:55:54 EDT 2016 

Hi Ellie & Ken;

> On Jun 28, 2016, at 20:47, ellie timoney <ellie at fastmail.com> wrote:
> 
> Hi Johan,
> 
>> In the unpatched code, a sasl_http_request_t structure is created on the
>> stack and a pointer to it is copied to httpd_saslconn using
>> sasl_setprop().  When the structure goes out of scope, there is no
>> guarantee that its members will be preserved.  A diff against the code I
>> just cloned (cyrus-imapd-3.0.0-beta2-294-ga42b500) is attached.
> 
> I think you're onto something here (though the attachment is missing):
> 
> Looking only at the cyrus-imapd code, I would have supposed that
> sasl_setprop() promised to take a copy of the provided value, not just
> the pointer, and that it was therefore fine for the original
> sasl_http_request_t structure to go out of scope.
> 
> But looking at cyrus-sasl code, it looks like sasl_setprop() *generally*
> takes a copy of the provided value, but for a few cases -- one of which
> is SASL_HTTP_REQUEST -- it just copies the pointer.
> 
> I'm not sure if this is a bug in cyrus-sasl, or in cyrus-imapd, but one
> of them is clearly wrong.  The man page for sasl_setprop() makes no
> promises in either direction, but also doesn't document
> SASL_HTTP_REQUEST at all, so the intent remains unclear.
> 
> Ken, what do you think?  IMO, sasl_setprop() should strictly and
> consistently make its own copy of the provided value, and promise thus,
> in which case the cyrus-imapd code is good.  But there might be API
> change ramifications here to be wary of?
> 
> I'm curious, how did you come across this?

I’m not sure how to best deal with it; hence a patch (regenerated and attached this time) instead of a pull request.

I ran into this while playing around with digest authentication in carddav.  Authentication relies on the method, which is passed to libsasl in the sasl_http_request_t structure.

// Cheers; Johan 


> On Tue, Jun 28, 2016, at 05:31 PM, Johan Hattne wrote:
>> 
>>> On Jun 26, 2016, at 20:02, ellie timoney <ellie at fastmail.com> wrote:
>>> 
>>> Hi Johan,
>>> 
>>> I don't know how your git repository got into that state...
>>> 
>>>> $ git describe
>>>> cyrus-imapd-2.5-snapshot-autoconf-and-automake-3857-g4049dd2
>>> 
>>> This says that the nearest tag your git can find is
>>> "cyrus-imapd-2.5-snapshot-autoconf-and-automake"(which is over four
>>> years old), and that your branch is 3857 commits ahead of that tag (but
>>> the "cyrus-imapd-2.5.0" release tag is only 1796 commits ahead, so why
>>> didn't describe find that, instead?), and that the commit id of your
>>> branch tip is "4049dd2" (which I don't have, presumably because it's
>>> your local commit containing your changes).
>>> 
>>> You probably want to refetch entirely I suspect [but keep reading first]
>>> 
>>>> * remote origin
>>>>   Fetch URL: git://git.cyrusimap.org/cyrus-imapd
>>>>   Push  URL: git://git.cyrusimap.org/cyrus-imapd
>>>>   HEAD branch: master
>>> 
>>> I think these URLs are wrong/old.  If you can even fetch from them
>>> anymore, do they update?  Look at the dates on the recent commits -- are
>>> they ancient?  ('git log --format=fuller' to see commit dates).
>>> 
>>> But anyway, we just moved our repositories to github last week, so
>>> anything you get from your old origin is going to be stale now
>>> regardless.  https://github.com/cyrusimap/cyrus-imapd is the github page
>>> with the clone/fork/etc links.
>> 
>> Thanks a lot, Ellie;
>> 
>> I’m confused about the repositories; I didn’t commit anything and the
>> last log entries are from August 2015.  Nevertheless, I cloned from
>> GitHub as per above and imap-makefile.patch is indeed obsolete.  However,
>> I find that the httpd patch, or something addressing the same symptom, is
>> still necessary.
>> 
>> In the unpatched code, a sasl_http_request_t structure is created on the
>> stack and a pointer to it is copied to httpd_saslconn using
>> sasl_setprop().  When the structure goes out of scope, there is no
>> guarantee that its members will be preserved.  A diff against the code I
>> just cloned (cyrus-imapd-3.0.0-beta2-294-ga42b500) is attached.
>> 
>>>> I got this all from
>>>> https://cyrusimap.org/mediawiki/index.php/Contribute#Anonymous_GIT_Access;
>>>> the text indicates that development is actually happening elsewhere, but
>>>> it has never been clear to me what the relationship between the
>>>> repositories is.
>>> 
>>> Okay, that mediawiki is ancient -- I actually thought it was gone
>>> already -- so that explains the old info.   I guess you got there from
>>> Google, because it hasn't been linked from cyrusimap.org for ages.
>>> 
>>> Anyway, I suspect once you set up the correct git remote, and fetch from
>>> that, your build troubles will go away.
>> 
>> Yes, I think I got to the mediawiki via Google.  
>> 
>> // Best wishes; Johan
>> 
>> 
>>> On Fri, Jun 24, 2016, at 11:33 PM, Johan Hattne wrote:
>>>> Hi Ellie;
>>>> 
>>>> $ git remote show origin | head -n 4
>>>> * remote origin
>>>>   Fetch URL: git://git.cyrusimap.org/cyrus-imapd
>>>>   Push  URL: git://git.cyrusimap.org/cyrus-imapd
>>>>   HEAD branch: master
>>>> $ git describe
>>>> cyrus-imapd-2.5-snapshot-autoconf-and-automake-3857-g4049dd2
>>>> 
>>>> I got this all from
>>>> https://cyrusimap.org/mediawiki/index.php/Contribute#Anonymous_GIT_Access;
>>>> the text indicates that development is actually happening elsewhere, but
>>>> it has never been clear to me what the relationship between the
>>>> repositories is.
>>>> 
>>>> // Best wishes; Johan
>>>> 
>>>>> On Jun 23, 2016, at 19:08, ellie timoney via Info-cyrus <info-cyrus at lists.andrew.cmu.edu> wrote:
>>>>> 
>>>>> Hi Johan,
>>>>> 
>>>>> What revision are these patches against?
>>>>> 
>>>>> The Makefile.am patch is unnecessary, and doesn't apply cleanly anyway,
>>>>> so I suspect you're looking at an old revision.
>>>>> 
>>>>> I haven't studied the httpd patch in depth yet.
>>>>> 
>>>>> Cheers,
>>>>> 
>>>>> ellie
>>>>> 
>>>>> On Thu, Jun 23, 2016, at 12:59 AM, Johan Hattne via Info-cyrus wrote:
>>>>>> Dear all;
>>>>>> 
>>>>>> To make carddav run from git sources
>>>>>> (git://git.cyrusimap.org/cyrus-imapd) I had to apply attached tiny
>>>>>> patches; the Makefile.am patch addresses a use-before-definition issue
>>>>>> which causes automake-1.14.1 to fall over, the httpd patch ensures that
>>>>>> the sasl_http_request_t structure which is set as a property of
>>>>>> httpd_saslconn does not go out of scope before it is used (I suppose this
>>>>>> behavior is a tad system-dependent).  Apologies if both these issues have
>>>>>> already been addressed elsewhere.
>>>>>> 
>>>>>> // Best wishes; Johan
>>>>>> 
>>>>>> ----
>>>>>> Cyrus Home Page: http://www.cyrusimap.org/
>>>>>> List Archives/Info: http://lists.andrew.cmu.edu/pipermail/info-cyrus/
>>>>>> To Unsubscribe:
>>>>>> https://lists.andrew.cmu.edu/mailman/listinfo/info-cyrus
>>>>>> Email had 2 attachments:
>>>>>> + imap-httpd.patch
>>>>>> 1k (text/plain)
>>>>>> + imap-makefile.patch
>>>>>> 1k (text/plain)
>>>>> ----
>>>>> Cyrus Home Page: http://www.cyrusimap.org/
>>>>> List Archives/Info: http://lists.andrew.cmu.edu/pipermail/info-cyrus/
>>>>> To Unsubscribe:
>>>>> https://lists.andrew.cmu.edu/mailman/listinfo/info-cyrus
>>>> 
>> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: imap-httpd.patch
Type: application/octet-stream
Size: 818 bytes
Desc: not available
URL: <http://lists.andrew.cmu.edu/pipermail/info-cyrus/attachments/20160629/193e5cef/attachment.obj>

More information about the Info-cyrus mailing list