Skip to content

Build with patched libcurl#4317

Merged
ethomson merged 4 commits intomasterfrom
ethomson/libcurl_build
Jul 26, 2017
Merged

Build with patched libcurl#4317
ethomson merged 4 commits intomasterfrom
ethomson/libcurl_build

Conversation

@ethomson
Copy link
Copy Markdown
Member

@ethomson ethomson commented Jul 23, 2017 •
edited
Loading

This builds with a custom version of libcurl, which includes a fix for proxies and NTLM credentials. It comes as quite a shock to curl 7.35, but you do not need to necessarily use proxy credentials when you are using a proxy with NTLM credentials (on the overall connection) and curl's connection pooling mechanism will segfault if you in fact do not.

This fix skips a strcmp against NULL proxy username/password:

--- curl-7.35.0.orig/lib/url.c
+++ curl-7.35.0/lib/url.c
@@ -3135,6 +3135,11 @@ ConnectionExists(struct SessionHandle *d

         /* Same for Proxy NTLM authentication */
         if(wantProxyNTLMhttp) {
+          /* Both check->http_proxy.user and check->http_proxy.passwd can be
+           * NULL */
+          if(!check->proxyuser || !check->proxypasswd)
+            continue;
+
           if(strcmp(needle->proxyuser, check->proxyuser) ||
              strcmp(needle->proxypasswd, check->proxypasswd))
             continue;

Regrettably, we need to move away from the container-based build system so that we can install custom packages that aren't actually part of the ubuntu distribution.

@ethomson ethomson force-pushed the ethomson/libcurl_build branch 6 times, most recently from ec00a3d to b3da977 Compare July 24, 2017 14:11
ethomson added 2 commits July 24, 2017 16:56
@ethomson
travis: build with patched libcurl
f031e20 
Ubuntu trusty has a bug in curl when using NTLM credentials in a proxy,
dereferencing a null pointer and causing segmentation faults.  Use a
custom-patched version of libcurl that avoids this issue.
@ethomson
travis: use trusty
4da3819
@ethomson ethomson force-pushed the ethomson/libcurl_build branch from b5b66cd to 6d59ed2 Compare July 24, 2017 15:56
@ethomson ethomson force-pushed the ethomson/libcurl_build branch from 6d59ed2 to 697583e Compare July 24, 2017 16:39
@ethomson ethomson changed the title WIP: Build with patched libcurl Build with patched libcurl Jul 24, 2017
@ethomson
Copy link
Copy Markdown
Member Author

ethomson commented Jul 24, 2017

It's not ideal to move back to the VM-based build infra and away from the container-based system, but at least our unit tests should now be consistent.

@pks-t @carlosmn any thoughts?

@ethomson ethomson merged commit 8f9d2bb into master Jul 26, 2017
@pks-t
Copy link
Copy Markdown
Member

pks-t commented Jul 28, 2017

I'm rather opposed to reverting to VM-based infra again. And luckily, we don't even have to do this, as the apt addon supports adding additional sources. E.g. like follows:

addons:
    apt:
        sources:
            - sourceline: 'deb https://repo.example.com/custom_curl'
              key_url: 'https://repo.example.com/gpg.key'

It forces us to create an actual repository, though. If you don't want to create the repo itself, I can do so, but I have no means to host these files afterwards.

@ethomson
Copy link
Copy Markdown
Member Author

ethomson commented Jul 28, 2017

Cool! I looked for that but didn't see it. I have an actual repository created at http://libgit2deps.edwardthomson.com. I'll give this a shot. Would definitely prefer to go back to the container style builds.

@ethomson ethomson mentioned this pull request Jul 28, 2017
Merged
@ethomson ethomson mentioned this pull request Sep 27, 2017
Merged
@carlosmn carlosmn mentioned this pull request Sep 28, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ethomson @pks-t