summaryrefslogtreecommitdiff
path: root/remote-curl.c
AgeCommit message (Collapse)AuthorFilesLines
2016-06-13remote-curl.c: convert fetch_git() to use argv_arrayLibravatar Nguyễn Thái Ngọc Duy1-28/+18
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-01-15strbuf: introduce strbuf_getline_{lf,nul}()Libravatar Junio C Hamano1-3/+3
The strbuf_getline() interface allows a byte other than LF or NUL as the line terminator, but this is only because I wrote these codepaths anticipating that there might be a value other than NUL and LF that could be useful when I introduced line_termination long time ago. No useful caller that uses other value has emerged. By now, it is clear that the interface is overly broad without a good reason. Many codepaths have hardcoded preference to read either LF terminated or NUL terminated records from their input, and then call strbuf_getline() with LF or NUL as the third parameter. This step introduces two thin wrappers around strbuf_getline(), namely, strbuf_getline_lf() and strbuf_getline_nul(), and mechanically rewrites these call sites to call either one of them. The changes contained in this patch are: * introduction of these two functions in strbuf.[ch] * mechanical conversion of all callers to strbuf_getline() with either '\n' or '\0' as the third parameter to instead call the respective thin wrapper. After this step, output from "git grep 'strbuf_getline('" would become a lot smaller. An interim goal of this series is to make this an empty set, so that we can have strbuf_getline_crlf() take over the shorter name strbuf_getline(). Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-11-20parse_fetch: convert to use struct object_idLibravatar brian m. carlson1-6/+6
Convert the parse_fetch function to use struct object_id. Remove the strlen check as get_oid_hex will fail safely on receiving a too-short NUL-terminated string. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
2015-11-20Convert struct ref to use object_id.Libravatar brian m. carlson1-5/+5
Use struct object_id in three fields in struct ref and convert all the necessary places that use it. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
2015-10-05use alloc_ref rather than hand-allocating "struct ref"Libravatar Jeff King1-4/+1
This saves us some manual computation, and eliminates a call to strcpy. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-19push: support signing pushes iff the server supports itLibravatar Dave Borowitz1-5/+11
Add a new flag --sign=true (or --sign=false), which means the same thing as the original --signed (or --no-signed). Give it a third value --sign=if-asked to tell push and send-pack to send a push certificate if and only if the server advertised a push cert nonce. If not, warn the user that their push may not be as secure as they thought. Signed-off-by: Dave Borowitz <dborowitz@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-17Merge branch 'rs/deflate-init-cleanup'Libravatar Junio C Hamano1-1/+0
Code simplification. * rs/deflate-init-cleanup: zlib: initialize git_zstream in git_deflate_init{,_gzip,_raw}
2015-03-05zlib: initialize git_zstream in git_deflate_init{,_gzip,_raw}Libravatar René Scharfe1-1/+0
Clear the git_zstream variable at the start of git_deflate_init() etc. so that callers don't have to do that. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-02-18Merge branch 'ye/http-accept-language'Libravatar Junio C Hamano1-0/+2
Using environment variable LANGUAGE and friends on the client side, HTTP-based transports now send Accept-Language when making requests. * ye/http-accept-language: http: add Accept-Language header if possible
2015-02-17Merge branch 'jk/remote-curl-an-array-in-struct-cannot-be-null'Libravatar Junio C Hamano1-1/+1
Fix a misspelled conditional that is always true. * jk/remote-curl-an-array-in-struct-cannot-be-null: do not check truth value of flex arrays
2015-01-28do not check truth value of flex arraysLibravatar Jeff King1-1/+1
There is no point in checking "!ref->name" when ref is a "struct ref". The name field is a flex-array, and there always has a non-zero address. This is almost certainly not hurting anything, but it does cause clang-3.6 to complain. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-01-28http: add Accept-Language header if possibleLibravatar Yi EungJun1-0/+2
Add an Accept-Language header which indicates the user's preferred languages defined by $LANGUAGE, $LC_ALL, $LC_MESSAGES and $LANG. Examples: LANGUAGE= -> "" LANGUAGE=ko:en -> "Accept-Language: ko, en;q=0.9, *;q=0.1" LANGUAGE=ko LANG=en_US.UTF-8 -> "Accept-Language: ko, *;q=0.1" LANGUAGE= LANG=en_US.UTF-8 -> "Accept-Language: en-US, *;q=0.1" This gives git servers a chance to display remote error messages in the user's preferred language. Limit the number of languages to 1,000 because q-value must not be smaller than 0.001, and limit the length of Accept-Language header to 4,000 bytes for some HTTP servers which cannot accept such long header. Signed-off-by: Yi EungJun <eungjun.yi@navercorp.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-08Merge branch 'jc/push-cert'Libravatar Junio C Hamano1-1/+12
Allow "git push" request to be signed, so that it can be verified and audited, using the GPG signature of the person who pushed, that the tips of branches at a public repository really point the commits the pusher wanted to, without having to "trust" the server. * jc/push-cert: (24 commits) receive-pack::hmac_sha1(): copy the entire SHA-1 hash out signed push: allow stale nonce in stateless mode signed push: teach smart-HTTP to pass "git push --signed" around signed push: fortify against replay attacks signed push: add "pushee" header to push certificate signed push: remove duplicated protocol info send-pack: send feature request on push-cert packet receive-pack: GPG-validate push certificates push: the beginning of "git push --signed" pack-protocol doc: typofix for PKT-LINE gpg-interface: move parse_signature() to where it should be gpg-interface: move parse_gpg_output() to where it should be send-pack: clarify that cmds_sent is a boolean send-pack: refactor inspecting and resetting status and sending commands send-pack: rename "new_refs" to "need_pack_data" receive-pack: factor out capability string generation send-pack: factor out capability string generation send-pack: always send capabilities send-pack: refactor decision to send update per ref send-pack: move REF_STATUS_REJECT_NODELETE logic a bit higher ...
2014-09-19Merge branch 'da/styles'Libravatar Junio C Hamano1-1/+1
* da/styles: stylefix: asterisks stick to the variable, not the type
2014-09-19Merge branch 'jk/send-pack-many-refspecs'Libravatar Junio C Hamano1-1/+7
The number of refs that can be pushed at once over smart HTTP was limited by the command line length. The limitation has been lifted by passing these refs from the standard input of send-pack. * jk/send-pack-many-refspecs: send-pack: take refspecs over stdin
2014-09-17signed push: teach smart-HTTP to pass "git push --signed" aroundLibravatar Junio C Hamano1-1/+12
The "--signed" option received by "git push" is first passed to the transport layer, which the native transport directly uses to notice that a push certificate needs to be sent. When the transport-helper is involved, however, the option needs to be told to the helper with set_helper_option(), and the helper needs to take necessary action. For the smart-HTTP helper, the "necessary action" involves spawning the "git send-pack" subprocess with the "--signed" option. Once the above all gets wired in, the smart-HTTP transport now can use the push certificate mechanism to authenticate its pushes. Add a test that is modeled after tests for the native transport in t5534-push-signed.sh to t5541-http-push-smart.sh. Update the test Apache configuration to pass GNUPGHOME environment variable through. As PassEnv would trigger warnings for an environment variable that is not set, export it from test-lib.sh set to a harmless value when GnuPG is not being used in the tests. Note that the added test is deliberately loose and does not check the nonce in this step. This is because the stateless RPC mode is inevitably flaky and a nonce that comes back in the actual push processing is one issued by a different process; if the two interactions with the server crossed a second boundary, the nonces will not match and such a check will fail. A later patch in the series will work around this shortcoming. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-09-02stylefix: asterisks stick to the variable, not the typeLibravatar David Aguilar1-1/+1
Signed-off-by: David Aguilar <davvid@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-26send-pack: take refspecs over stdinLibravatar Jeff King1-1/+7
Pushing a large number of refs works over most transports, because we implement send-pack as an internal function. However, it can sometimes fail when pushing over http, because we have to spawn "git send-pack --stateless-rpc" to do the heavy lifting, and we pass each refspec on the command line. This can cause us to overflow the OS limits on the size of the command line for a large push. We can solve this by giving send-pack a --stdin option and using it from remote-curl. We already dealt with this on the fetch-pack side in 078b895 (fetch-pack: new --stdin option to read refs from stdin, 2012-04-02). The stdin option (and in particular, its use of packet-lines for stateless-rpc input) is modeled after that solution. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-20run-command: introduce CHILD_PROCESS_INITLibravatar René Scharfe1-2/+1
Most struct child_process variables are cleared using memset first after declaration. Provide a macro, CHILD_PROCESS_INIT, that can be used to initialize them statically instead. That's shorter, doesn't require a function call and is slightly more readable (especially given that we already have STRBUF_INIT, ARGV_ARRAY_INIT etc.). Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-07-21Merge branch 'jk/remote-curl-squelch-extra-errors'Libravatar Junio C Hamano1-8/+6
* jk/remote-curl-squelch-extra-errors: remote-curl: mark helper-protocol errors more clearly remote-curl: use error instead of fprintf(stderr) remote-curl: do not complain on EOF from parent git
2014-07-10remote-curl: mark helper-protocol errors more clearlyLibravatar Jeff King1-4/+4
When we encounter an error in remote-curl, we generally just report it to stderr. There is no need for the user to care that the "could not connect to server" error was generated by git-remote-https rather than a function in the parent git-fetch process. However, when the error is in the protocol between git and the helper, it makes sense to clearly identify which side is complaining. These cases shouldn't ever happen, but when they do, we can make them less confusing by being more verbose. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-07-10remote-curl: use error instead of fprintf(stderr)Libravatar Jeff King1-5/+5
We usually prefix our error messages with "error: ", but many error messages from remote-curl are simply printed with fprintf. This can make the output a little harder to read (especially because such message may be intermingled with errors from the parent git process). There is no reason to avoid error(), as we are already calling it many places (in addition to libgit.a functions which use it). While we're adjusting the messages, we can also drop the capitalization which makes them unlike other git error messages. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-07-10remote-curl: do not complain on EOF from parent gitLibravatar Jeff King1-2/+0
The parent git process is supposed to send us an empty line to indicate that the conversation is over. However, the parent process may die() if there is a problem with the operation (e.g., we try to fetch a ref that does not exist). In this case, it produces a useful message, but then remote-curl _also_ produces an unhelpful message: $ git pull origin matser fatal: couldn't find remote ref matser Unexpected end of command stream The "right" way to fix this is to teach the parent git to always cleanly close the connection to the helper, letting it know that we are done. Implementing that is rather clunky, though, as it would involve either replacing die() operations with returning errors up the stack (until we disconnect the transport), or adding an atexit handler to clean up any transport helpers left open. It's much simpler to just suppress the EOF message in remote-curl. It was not added to address any real-world situation in the first place, but rather a "we should probably report unexpected things" suggestion[1]. It is the parent git which drives the operation, and whose exit value actually matters. If the parent dies, then the helper has no need to complain (except as a debugging aid). In the off chance that the pipe is closed without the parent dying, it can still notice the non-zero exit code. [1] http://article.gmane.org/gmane.comp.version-control.git/176036 Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-20use skip_prefix to avoid repeating stringsLibravatar Jeff King1-7/+8
It's a common idiom to match a prefix and then skip past it with strlen, like: if (starts_with(foo, "bar")) foo += strlen("bar"); This avoids magic numbers, but means we have to repeat the string (and there is no compiler check that we didn't make a typo in one of the strings). We can use skip_prefix to handle this case without repeating ourselves. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-05-27remote-curl: reencode http error messagesLibravatar Jeff King1-7/+10
We currently recognize an error message with a content-type "text/plain; charset=utf-16" as text, but we ignore the charset parameter entirely. Let's encode it to log_output_encoding, which is presumably something the user's terminal can handle. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-05-27http: extract type/subtype portion of content-typeLibravatar Jeff King1-1/+1
When we get a content-type from curl, we get the whole header line, including any parameters, and without any normalization (like downcasing or whitespace) applied. If we later try to match it with strcmp() or even strcasecmp(), we may get false negatives. This could cause two visible behaviors: 1. We might fail to recognize a smart-http server by its content-type. 2. We might fail to relay text/plain error messages to users (especially if they contain a charset parameter). This patch teaches the http code to extract and normalize just the type/subtype portion of the string. This is technically passing out less information to the callers, who can no longer see the parameters. But none of the current callers cares, and a future patch will add back an easier-to-use method for accessing those parameters. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-02-18http: never use curl_easy_performLibravatar Jeff King1-4/+1
We currently don't reuse http connections when fetching via the smart-http protocol. This is bad because the TCP handshake introduces latency, and especially because SSL connection setup may be non-trivial. We can fix it by consistently using curl's "multi" interface. The reason is rather complicated: Our http code has two ways of being used: queuing many "slots" to be fetched in parallel, or fetching a single request in a blocking manner. The parallel code is built on curl's "multi" interface. Most of the single-request code uses http_request, which is built on top of the parallel code (we just feed it one slot, and wait until it finishes). However, one could also accomplish the single-request scheme by avoiding curl's multi interface entirely and just using curl_easy_perform. This is simpler, and is used by post_rpc in the smart-http protocol. It does work to use the same curl handle in both contexts, as long as it is not at the same time. However, internally curl may not share all of the cached resources between both contexts. In particular, a connection formed using the "multi" code will go into a reuse pool connected to the "multi" object. Further requests using the "easy" interface will not be able to reuse that connection. The smart http protocol does ref discovery via http_request, which uses the "multi" interface, and then follows up with the "easy" interface for its rpc calls. As a result, we make two HTTP connections rather than reusing a single one. We could teach the ref discovery to use the "easy" interface. But it is only once we have done this discovery that we know whether the protocol will be smart or dumb. If it is dumb, then our further requests, which want to fetch objects in parallel, will not be able to reuse the same connection. Instead, this patch switches post_rpc to build on the parallel interface, which means that we use it consistently everywhere. It's a little more complicated to use, but since we have the infrastructure already, it doesn't add any code; we can just factor out the relevant bits from http_request. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-01-17Merge branch 'nd/shallow-clone'Libravatar Junio C Hamano1-5/+30
Fetching from a shallow-cloned repository used to be forbidden, primarily because the codepaths involved were not carefully vetted and we did not bother supporting such usage. This attempts to allow object transfer out of a shallow-cloned repository in a controlled way (i.e. the receiver become a shallow repository with truncated history). * nd/shallow-clone: (31 commits) t5537: fix incorrect expectation in test case 10 shallow: remove unused code send-pack.c: mark a file-local function static git-clone.txt: remove shallow clone limitations prune: clean .git/shallow after pruning objects clone: use git protocol for cloning shallow repo locally send-pack: support pushing from a shallow clone via http receive-pack: support pushing to a shallow clone via http smart-http: support shallow fetch/clone remote-curl: pass ref SHA-1 to fetch-pack as well send-pack: support pushing to a shallow clone receive-pack: allow pushes that update .git/shallow connected.c: add new variant that runs with --shallow-file add GIT_SHALLOW_FILE to propagate --shallow-file to subprocesses receive/send-pack: support pushing from a shallow clone receive-pack: reorder some code in unpack() fetch: add --update-shallow to accept refs that update .git/shallow upload-pack: make sure deepening preserves shallow roots fetch: support fetching from a shallow repository clone: support remote shallow repository ...
2013-12-17Merge branch 'cc/starts-n-ends-with'Libravatar Junio C Hamano1-7/+7
Remove a few duplicate implementations of prefix/suffix comparison functions, and rename them to starts_with and ends_with. * cc/starts-n-ends-with: replace {pre,suf}fixcmp() with {starts,ends}_with() strbuf: introduce starts_with() and ends_with() builtin/remote: remove postfixcmp() and use suffixcmp() instead environment: normalize use of prefixcmp() by removing " != 0"
2013-12-10smart-http: support shallow fetch/cloneLibravatar Nguyễn Thái Ngọc Duy1-4/+28
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-12-10remote-curl: pass ref SHA-1 to fetch-pack as wellLibravatar Nguyễn Thái Ngọc Duy1-1/+2
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-12-10connect.c: teach get_remote_heads to parse "shallow" linesLibravatar Nguyễn Thái Ngọc Duy1-1/+1
No callers pass a non-empty pointer as shallow_points at this stage. As a result, all clients still refuse to talk to shallow repository on the other end. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-12-05replace {pre,suf}fixcmp() with {starts,ends}_with()Libravatar Christian Couder1-7/+7
Leaving only the function definitions and declarations so that any new topic in flight can still make use of the old functions, replace existing uses of the prefixcmp() and suffixcmp() with new API functions. The change can be recreated by mechanically applying this: $ git grep -l -e prefixcmp -e suffixcmp -- \*.c | grep -v strbuf\\.c | xargs perl -pi -e ' s|!prefixcmp\(|starts_with\(|g; s|prefixcmp\(|!starts_with\(|g; s|!suffixcmp\(|ends_with\(|g; s|suffixcmp\(|!ends_with\(|g; ' on the result of preparatory changes in this series. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-12-05Merge branch 'bc/http-100-continue'Libravatar Junio C Hamano1-10/+21
Issue "100 Continue" responses to help use of GSS-Negotiate authentication scheme over HTTP transport when needed. * bc/http-100-continue: remote-curl: fix large pushes with GSSAPI remote-curl: pass curl slot_results back through run_slot http: return curl's AUTHAVAIL via slot_results
2013-10-31remote-curl: fix large pushes with GSSAPILibravatar Brian M. Carlson1-2/+9
Due to an interaction between the way libcurl handles GSSAPI authentication over HTTP and the way git uses libcurl, large pushes (those over http.postBuffer bytes) would fail due to an authentication failure requiring a rewind of the curl buffer. Such a rewind was not possible because the data did not fit into the entire buffer. Enable the use of the Expect: 100-continue header for large requests where the server offers GSSAPI authentication to avoid this issue, since the request would otherwise fail. This allows git to get the authentication data right before sending the pack contents. Existing cases where pushes would succeed, including small requests using GSSAPI, still disable the use of 100 Continue, as it causes problems for some remote HTTP implementations (servers and proxies). Signed-off-by: Brian M. Carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net>
2013-10-31remote-curl: pass curl slot_results back through run_slotLibravatar Jeff King1-9/+13
Some callers may want to know more than just the integer error code we return. Let them optionally pass a slot_results struct to fill in (or NULL if they do not care). In either case we continue to return the integer code. We can also give probe_rpc the same treatment (since it builds directly on run_slot). Signed-off-by: Jeff King <peff@peff.net>
2013-10-30Merge branch 'jk/http-auth-redirects'Libravatar Junio C Hamano1-28/+41
Handle the case where http transport gets redirected during the authorization request better. * jk/http-auth-redirects: http.c: Spell the null pointer as NULL remote-curl: rewrite base url from info/refs redirects remote-curl: store url as a strbuf remote-curl: make refs_url a strbuf http: update base URLs when we see redirects http: provide effective url to callers http: hoist credential request out of handle_curl_result http: refactor options to http_get_* http_request: factor out curlinfo_strbuf http_get_file: style fixes
2013-10-14remote-curl: rewrite base url from info/refs redirectsLibravatar Jeff King1-0/+4
For efficiency and security reasons, an earlier commit in this series taught http_get_* to re-write the base url based on redirections we saw while making a specific request. This commit wires that option into the info/refs request, meaning that a redirect from http://example.com/foo.git/info/refs to https://example.com/bar.git/info/refs will behave as if "https://example.com/bar.git" had been provided to git in the first place. The tests bear some explanation. We introduce two new hierearchies into the httpd test config: 1. Requests to /smart-redir-limited will work only for the initial info/refs request, but not any subsequent requests. As a result, we can confirm whether the client is re-rooting its requests after the initial contact, since otherwise it will fail (it will ask for "repo.git/git-upload-pack", which is not redirected). 2. Requests to smart-redir-auth will redirect, and require auth after the redirection. Since we are using the redirected base for further requests, we also update the credential struct, in order not to mislead the user (or credential helpers) about which credential is needed. We can therefore check the GIT_ASKPASS prompts to make sure we are prompting for the new location. Because we have neither multiple servers nor https support in our test setup, we can only redirect between paths, meaning we need to turn on credential.useHttpPath to see the difference. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-10-14remote-curl: store url as a strbufLibravatar Jeff King1-19/+19
We use a strbuf to generate the string containing the remote URL, but then detach it to a bare pointer. This makes it harder to later manipulate the URL, as we have forgotten the length (and the allocation semantics are not as clear). Let's instead keep the strbuf around. As a bonus, this eliminates a confusing double-use of the "buf" strbuf in main(). Prior to this, it was used both for constructing the url, and for reading commands from stdin. The downside is that we have to update each call site to refer to "url.buf" rather than just "url" when they want the C string. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-10-14remote-curl: make refs_url a strbufLibravatar Jeff King1-8/+7
In the discover_refs function, we use a strbuf named "buffer" for multiple purposes. First we build the info/refs URL in it, and then detach that to a bare pointer. Then, we use the same strbuf to store the result of fetching the refs. Let's instead keep a separate refs_url strbuf. This is less confusing, as the "buffer" strbuf is now used for only one thing. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-10-14http: hoist credential request out of handle_curl_resultLibravatar Jeff King1-1/+6
When we are handling a curl response code in http_request or in the remote-curl RPC code, we use the handle_curl_result helper to translate curl's response into an easy-to-use code. When we see an HTTP 401, we do one of two things: 1. If we already had a filled-in credential, we mark it as rejected, and then return HTTP_NOAUTH to indicate to the caller that we failed. 2. If we didn't, then we ask for a new credential and tell the caller HTTP_REAUTH to indicate that they may want to try again. Rejecting in the first case makes sense; it is the natural result of the request we just made. However, prompting for more credentials in the second step does not always make sense. We do not know for sure that the caller is going to make a second request, and nor are we sure that it will be to the same URL. Logically, the prompt belongs not to the request we just finished, but to the request we are (maybe) about to make. In practice, it is very hard to trigger any bad behavior. Currently, if we make a second request, it will always be to the same URL (even in the face of redirects, because curl handles the redirects internally). And we almost always retry on HTTP_REAUTH these days. The one exception is if we are streaming a large RPC request to the server (e.g., a pushed packfile), in which case we cannot restart. It's extremely unlikely to see a 401 response at this stage, though, as we would typically have seen it when we sent a probe request, before streaming the data. This patch drops the automatic prompt out of case 2, and instead requires the caller to do it. This is a few extra lines of code, and the bug it fixes is unlikely to come up in practice. But it is conceptually cleaner, and paves the way for better handling of credentials across redirects. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-09-30http: refactor options to http_get_*Libravatar Jeff King1-2/+7
Over time, the http_get_strbuf function has grown several optional parameters. We now have a bitfield with multiple boolean options, as well as an optional strbuf for returning the content-type of the response. And a future patch in this series is going to add another strbuf option. Treating these as separate arguments has a few downsides: 1. Most call sites need to add extra NULLs and 0s for the options they aren't interested in. 2. The http_get_* functions are actually wrappers around 2 layers of low-level implementation functions. We have to pass these options through individually. 3. The http_get_strbuf wrapper learned these options, but nobody bothered to do so for http_get_file, even though it is backed by the same function that does understand the options. Let's consolidate the options into a single struct. For the common case of the default options, we'll allow callers to simply pass a NULL for the options struct. The resulting code is often a few lines longer, but it ends up being easier to read (and to change as we add new options, since we do not need to update each call site). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-09-09Merge branch 'jc/push-cas'Libravatar Junio C Hamano1-0/+12
Allow a safer "rewind of the remote tip" push than blind "--force", by requiring that the overwritten remote ref to be unchanged since the new history to replace it was prepared. The machinery is more or less ready. The "--force" option is again the big red button to override any safety, thanks to J6t's sanity (the original round allowed --lockref to defeat --force). The logic to choose the default implemented here is fragile (e.g. "git fetch" after seeing a failure will update the remote-tracking branch and will make the next "push" pass, defeating the safety pretty easily). It is suitable only for the simplest workflows, and it may hurt users more than it helps them. * jc/push-cas: push: teach --force-with-lease to smart-http transport send-pack: fix parsing of --force-with-lease option t5540/5541: smart-http does not support "--force-with-lease" t5533: test "push --force-with-lease" push --force-with-lease: tie it all together push --force-with-lease: implement logic to populate old_sha1_expect[] remote.c: add command line option parser for "--force-with-lease" builtin/push.c: use OPT_BOOL, not OPT_BOOLEAN cache.h: move remote/connect API out of it
2013-09-09Merge branch 'nd/clone-connectivity-shortcut'Libravatar Junio C Hamano1-1/+14
* nd/clone-connectivity-shortcut: smart http: use the same connectivity check on cloning
2013-08-02push: teach --force-with-lease to smart-http transportLibravatar Junio C Hamano1-1/+15
We have been passing enough information to enable the compare-and-swap logic down to the transport layer, but the transport helper was not passing it to smart-http transport. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-07-23smart http: use the same connectivity check on cloningLibravatar Nguyễn Thái Ngọc Duy1-1/+14
This is an extension of c6807a4 (clone: open a shortcut for connectivity check - 2013-05-26) to reduce the cost of connectivity check at clone time, this time with smart http protocol. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-07-09remote-http: use argv-arrayLibravatar Junio C Hamano1-16/+16
Instead of using a hand-managed argument array, use argv-array API to manage dynamically formulated command line. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-06remote-curl: die directly with http error messagesLibravatar Jeff King1-2/+1
When we encounter an unknown http error (e.g., a 403), we hand the error code to http_error, which then prints it with error(). After that we die with the redundant message "HTTP request failed". Instead, let's just drop http_error entirely, which does nothing but pass arguments to error(), and instead die directly with a useful message. So before: $ git clone https://example.com/repo.git Cloning into 'repo'... error: unable to access 'https://example.com/repo.git': The requested URL returned error: 403 Forbidden fatal: HTTP request failed and after: $ git clone https://example.com/repo.git Cloning into 'repo'... fatal: unable to access 'https://example.com/repo.git': The requested URL returned error: 403 Forbidden Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-06http: simplify http_error helper functionLibravatar Jeff King1-1/+1
This helper function should really be a one-liner that prints an error message, but it has ended up unnecessarily complicated: 1. We call error() directly when we fail to start the curl request, so we must later avoid printing a duplicate error in http_error(). It would be much simpler in this case to just stuff the error message into our usual curl_errorstr buffer rather than printing it ourselves. This means that http_error does not even have to care about curl's exit value (the interesting part is in the errorstr buffer already). 2. We return the "ret" value passed in to us, but none of the callers actually cares about our return value. We can just drop this entirely. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-06remote-curl: consistently report repo url for http errorsLibravatar Jeff King1-2/+2
When we report http errors in fetching the initial ref advertisement, we show the full URL we attempted to use, including "info/refs?service=git-upload-pack". While this may be useful for debugging a broken server, it is unnecessarily verbose and confusing for most cases, in which the client user is not even the same person as the owner of the repository. Let's just show the repository URL; debugging can happen with GIT_CURL_VERBOSE, which shows way more useful information, anyway. At the same time, let's also make sure to mention the repository URL when we report failed authentication (previously we said only "Authentication failed"). Knowing the URL can help the user realize why authentication failed (e.g., they meant to push to remote A, not remote B). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>