1. 01 Apr, 2019 2 commits
  2. 24 Mar, 2019 2 commits
    • Jeff King's avatar
      http: use normalize_curl_result() instead of manual conversion · 3d10f72e
      Jeff King authored
      When we switched off CURLOPT_FAILONERROR in 17966c0a (http: avoid
      disconnecting on 404s for loose objects, 2016-07-11), the fetch_object()
      function started manually handling 404's. Since we now have
      normalize_curl_result() for use elsewhere, we can use it here as well,
      shortening the code.
      
      Note that we lose the check for http/https in the URL here. None of the
      other result-normalizing code paths bother with this. Looking at
      missing_target(), which checks specifically for an FTP-specific CURLcode
      and "http" code 550, it seems likely that git-over-ftp has been subtly
      broken since 17966c0a. This patch does nothing to fix that, but nor
      should it make anything worse (in fact, it may be slightly better
      because we'll actually recognize an error as such, rather than assuming
      CURLE_OK means we actually got some data).
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      3d10f72e
    • Jeff King's avatar
      http: normalize curl results for dumb loose and alternates fetches · ccbbd8bf
      Jeff King authored
      If the dumb-http walker encounters a 404 when fetching a loose object,
      it then looks at any http-alternates for the object. The 404 check is
      implemented by missing_target(), which checks not only the http code,
      but also that we got an http error from the CURLcode.
      
      That broke when we stopped using CURLOPT_FAILONERROR in 17966c0a
      (http: avoid disconnecting on 404s for loose objects, 2016-07-11), since
      our CURLcode will now be CURLE_OK. As a result, fetching over dumb-http
      from a repository with alternates could result in Git printing "Unable
      to find abcd1234..." and aborting.
      
      We could probably fix this just by loosening missing_target(). However,
      there's other code which looks at the curl result, and it would have to
      be tweaked as well. Instead, let's just normalize the result the same
      way the smart-http code does.
      
      There's a similar case in processing the alternates (where we failover
      from "info/http-alternates" to "info/alternates"). We'll give it the
      same treatment.
      
      After this patch, we should be hitting all code paths that need this
      normalization (notably absent here is the http_pack_request path, but it
      does not use FAILONERROR, nor missing_target()).
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      ccbbd8bf
  3. 08 Jan, 2019 3 commits
    • Jeff King's avatar
      convert has_sha1_file() callers to has_object_file() · 98374a07
      Jeff King authored
      The only remaining callers of has_sha1_file() actually have an object_id
      already. They can use the "object" variant, rather than dereferencing
      the hash themselves.
      
      The code changes here were completely generated by the included
      coccinelle patch.
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      98374a07
    • Jeff King's avatar
      sha1-file: modernize loose object file functions · 514c5fdd
      Jeff King authored
      The loose object access code in sha1-file.c is some of the oldest in
      Git, and could use some modernizing. It mostly uses "unsigned char *"
      for object ids, which these days should be "struct object_id".
      
      It also uses the term "sha1_file" in many functions, which is confusing.
      The term "loose_objects" is much better. It clearly distinguishes
      them from packed objects (which didn't even exist back when the name
      "sha1_file" came into being). And it also distinguishes it from the
      checksummed-file concept in csum-file.c (which until recently was
      actually called "struct sha1file"!).
      
      This patch converts the functions {open,close,map,stat}_sha1_file() into
      open_loose_object(), etc, and switches their sha1 arguments for
      object_id structs. Similarly, path functions like fill_sha1_path()
      become fill_loose_path() and use object_ids.
      
      The function sha1_loose_object_info() already says "loose", so we can
      just drop the "sha1" (and teach it to use object_id).
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      514c5fdd
    • Jeff King's avatar
      http: use struct object_id instead of bare sha1 · f0be0db1
      Jeff King authored
      The dumb-http walker code still passes around and stores object ids as
      "unsigned char *sha1". Let's modernize it.
      
      There's probably still more work to be done to handle dumb-http fetches
      with a new, larger hash. But that can wait; this is enough that we can
      now convert some of the low-level object routines that we call into from
      here (and in fact, some of the "oid.hash" references added here will be
      further improved in the next patch).
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      f0be0db1
  4. 13 Nov, 2018 1 commit
    • Jeff King's avatar
      sha1_file_name(): overwrite buffer instead of appending · b69fb867
      Jeff King authored
      The sha1_file_name() function is used to generate the path to a loose
      object in the object directory. It doesn't make much sense for it to
      append, since the the path we write may be absolute (i.e., you cannot
      reliably build up a path with it). Because many callers use it with a
      static buffer, they have to strbuf_reset() manually before each call
      (and the other callers always use an empty buffer, so they don't care
      either way). Let's handle this automatically.
      
      Since we're changing the semantics, let's take the opportunity to give
      it a more hash-neutral name (which will also catch any callers from
      topics in flight).
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      b69fb867
  5. 29 Aug, 2018 2 commits
    • Jeff King's avatar
      convert "hashcmp() != 0" to "!hasheq()" · 67947c34
      Jeff King authored
      This rounds out the previous three patches, covering the
      inequality logic for the "hash" variant of the functions.
      
      As with the previous three, the accompanying code changes
      are the mechanical result of applying the coccinelle patch;
      see those patches for more discussion.
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      67947c34
    • Jeff King's avatar
      convert "hashcmp() == 0" to hasheq() · e3ff0683
      Jeff King authored
      This is the partner patch to the previous one, but covering
      the "hash" variants instead of "oid".  Note that our
      coccinelle rule is slightly more complex to avoid triggering
      the call in hasheq().
      
      I didn't bother to add a new rule to convert:
      
        - hasheq(E1->hash, E2->hash)
        + oideq(E1, E2)
      
      Since these are new functions, there won't be any such
      existing callers. And since most of the code is already
      using oideq, we're not likely to introduce new ones.
      
      We might still see "!hashcmp(E1->hash, E2->hash)" from topics
      in flight. But because our new rule comes after the existing
      ones, that should first get converted to "!oidcmp(E1, E2)"
      and then to "oideq(E1, E2)".
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      e3ff0683
  6. 26 Mar, 2018 2 commits
  7. 14 Mar, 2018 1 commit
  8. 17 Jan, 2018 1 commit
  9. 23 Aug, 2017 1 commit
  10. 13 Mar, 2017 1 commit
    • Jeff King's avatar
      http-walker: fix buffer underflow processing remote alternates · d61434ae
      Jeff King authored
      If we parse a remote alternates (or http-alternates), we
      expect relative lines like:
      
        ../../foo.git/objects
      
      which we convert into "$URL/../foo.git/" (and then use that
      as a base for fetching more objects).
      
      But if the remote feeds us nonsense like just:
      
        ../
      
      we will try to blindly strip the last 7 characters, assuming
      they contain the string "objects". Since we don't _have_ 7
      characters at all, this results in feeding a small negative
      value to strbuf_add(), which converts it to a size_t,
      resulting in a big positive value. This should consistently
      fail (since we can't generall allocate the max size_t minus
      7 bytes), so there shouldn't be any security implications.
      
      Let's fix this by using strbuf_strip_suffix() to drop the
      characters we want. If they're not present, we'll ignore the
      alternate (in theory we could use it as-is, but the rest of
      the http-walker code unconditionally tacks "objects/" back
      on, so it is it not prepared to handle such a case).
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      d61434ae
  11. 06 Mar, 2017 2 commits
  12. 15 Dec, 2016 1 commit
    • Jeff King's avatar
      http: respect protocol.*.allow=user for http-alternates · abcbdc03
      Jeff King authored
      The http-walker may fetch the http-alternates (or
      alternates) file from a remote in order to find more
      objects. This should count as a "not from the user" use of
      the protocol. But because we implement the redirection
      ourselves and feed the new URL to curl, it will use the
      CURLOPT_PROTOCOLS rules, not the more restrictive
      CURLOPT_REDIR_PROTOCOLS.
      
      The ideal solution would be for each curl request we make to
      know whether or not is directly from the user or part of an
      alternates redirect, and then set CURLOPT_PROTOCOLS as
      appropriate. However, that would require plumbing that
      information through all of the various layers of the http
      code.
      
      Instead, let's check the protocol at the source: when we are
      parsing the remote http-alternates file. The only downside
      is that if there's any mismatch between what protocol we
      think it is versus what curl thinks it is, it could violate
      the policy.
      
      To address this, we'll make the parsing err on the picky
      side, and only allow protocols that it can parse
      definitively. So for example, you can't elude the "http"
      policy by asking for "HTTP://", even though curl might
      handle it; we would reject it as unknown. The only unsafe
      case would be if you have a URL that starts with "http://"
      but curl interprets as another protocol. That seems like an
      unlikely failure mode (and we are still protected by our
      base CURLOPT_PROTOCOL setting, so the worst you could do is
      trigger one of https, ftp, or ftps).
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarBrandon Williams <bmwill@google.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      abcbdc03
  13. 06 Dec, 2016 2 commits
    • Jeff King's avatar
      http-walker: complain about non-404 loose object errors · 3680f16f
      Jeff King authored
      Since commit 17966c0a (http: avoid disconnecting on 404s
      for loose objects, 2016-07-11), we turn off curl's
      FAILONERROR option and instead manually deal with failing
      HTTP codes.
      
      However, the logic to do so only recognizes HTTP 404 as a
      failure. This is probably the most common result, but if we
      were to get another code, the curl result remains CURLE_OK,
      and we treat it as success. We still end up detecting the
      failure when we try to zlib-inflate the object (which will
      fail), but instead of reporting the HTTP error, we just
      claim that the object is corrupt.
      
      Instead, let's catch anything in the 300's or above as an
      error (300's are redirects which are not an error at the
      HTTP level, but are an indication that we've explicitly
      disabled redirects, so we should treat them as such; we
      certainly don't have the resulting object content).
      
      Note that we also fill in req->errorstr, which we didn't do
      before. Without FAILONERROR, curl will not have filled this
      in, and it will remain a blank string. This never mattered
      for the 404 case, because in the logic below we hit the
      "missing_target()" branch and print nothing. But for other
      errors, we'd want to say _something_, if only to fill in the
      blank slot in the error message.
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      3680f16f
    • Jeff King's avatar
      http: treat http-alternates like redirects · cb4d2d35
      Jeff King authored
      The previous commit made HTTP redirects more obvious and
      tightened up the default behavior. However, there's another
      way for a server to ask a git client to fetch arbitrary
      content: by having an http-alternates file (or a regular
      alternates file, which is used as a backup).
      
      Similar to the HTTP redirect case, a malicious server can
      claim to have refs pointing at object X, return a 404 when
      the client asks for X, but point to some other URL via
      http-alternates, which the client will transparently fetch.
      The end result is that it looks from the user's perspective
      like the objects came from the malicious server, as the
      other URL is not mentioned at all.
      
      Worse, because we feed the new URL to curl ourselves, the
      usual protocol restrictions do not kick in (neither curl's
      default of disallowing file://, nor the protocol
      whitelisting in f4113cac (http: limit redirection to
      protocol-whitelist, 2015-09-22).
      
      Let's apply the same rules here as we do for HTTP redirects.
      Namely:
      
        - unless http.followRedirects is set to "always", we will
          not follow remote redirects from http-alternates (or
          alternates) at all
      
        - set CURLOPT_PROTOCOLS alongside CURLOPT_REDIR_PROTOCOLS
          restrict ourselves to a known-safe set and respect any
          user-provided whitelist.
      
        - mention alternate object stores on stderr so that the
          user is aware another source of objects may be involved
      
      The first item may prove to be too restrictive. The most
      common use of alternates is to point to another path on the
      same server. While it's possible for a single-server
      redirect to be an attack, it takes a fairly obscure setup
      (victim and evil repository on the same host, host speaks
      dumb http, and evil repository has access to edit its own
      http-alternates file).
      
      So we could make the checks more specific, and only cover
      cross-server redirects. But that means parsing the URLs
      ourselves, rather than letting curl handle them. This patch
      goes for the simpler approach. Given that they are only used
      with dumb http, http-alternates are probably pretty rare.
      And there's an escape hatch: the user can allow redirects on
      a specific server by setting http.<url>.followRedirects to
      "always".
      Reported-by: 's avatarJann Horn <jannh@google.com>
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      cb4d2d35
  14. 12 Jul, 2016 3 commits
  15. 25 Sep, 2015 1 commit
    • Jeff King's avatar
      http-walker: store url in a strbuf · 54ba4c5f
      Jeff King authored
      We do an unchecked sprintf directly into our url buffer.
      This doesn't overflow because we know that it was sized for
      "$base/objects/info/http-alternates", and we are writing
      "$base/objects/info/alternates", which must be smaller. But
      that is not immediately obvious to a reader who is looking
      for buffer overflows. Let's switch to a strbuf, so that we
      do not have to think about this issue at all.
      Signed-off-by: 's avatarJeff King <peff@peff.net>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      54ba4c5f
  16. 02 Sep, 2014 1 commit
  17. 19 Jun, 2014 2 commits
  18. 12 Sep, 2012 1 commit
  19. 04 May, 2011 1 commit
  20. 16 Mar, 2011 1 commit
    • Jonathan Nieder's avatar
      standardize brace placement in struct definitions · 9cba13ca
      Jonathan Nieder authored
      In a struct definitions, unlike functions, the prevailing style is for
      the opening brace to go on the same line as the struct name, like so:
      
       struct foo {
      	int bar;
      	char *baz;
       };
      
      Indeed, grepping for 'struct [a-z_]* {$' yields about 5 times as many
      matches as 'struct [a-z_]*$'.
      
      Linus sayeth:
      
       Heretic people all over the world have claimed that this inconsistency
       is ...  well ...  inconsistent, but all right-thinking people know that
       (a) K&R are _right_ and (b) K&R are right.
      Signed-off-by: 's avatarJonathan Nieder <jrnieder@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      9cba13ca
  21. 31 May, 2010 1 commit
    • Gary V. Vaughan's avatar
      enums: omit trailing comma for portability · 4b05548f
      Gary V. Vaughan authored
      Without this patch at least IBM VisualAge C 5.0 (I have 5.0.2) on AIX
      5.1 fails to compile git.
      
      enum style is inconsistent already, with some enums declared on one
      line, some over 3 lines with the enum values all on the middle line,
      sometimes with 1 enum value per line... and independently of that the
      trailing comma is sometimes present and other times absent, often
      mixing with/without trailing comma styles in a single file, and
      sometimes in consecutive enum declarations.
      
      Clearly, omitting the comma is the more portable style, and this patch
      changes all enum declarations to use the portable omitted dangling
      comma style consistently.
      Signed-off-by: 's avatarGary V. Vaughan <gary@thewrittenword.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      4b05548f
  22. 17 Apr, 2010 1 commit
  23. 02 Mar, 2010 2 commits
  24. 06 Jun, 2009 5 commits
    • Ray's avatar
      http*: add helper methods for fetching objects (loose) · 5424bc55
      Ray authored
      The code handling the fetching of loose objects in http-push.c and
      http-walker.c have been refactored into new methods and a new struct
      (object_http_request) in http.c. They are not meant to be invoked
      elsewhere.
      
      The new methods in http.c are
       - new_http_object_request
       - process_http_object_request
       - finish_http_object_request
       - abort_http_object_request
       - release_http_object_request
      
      and the new struct is http_object_request.
      
      RANGER_HEADER_SIZE and no_pragma_header is no longer made available
      outside of http.c, since after the above changes, there are no other
      instances of usage outside of http.c.
      
      Remove members of the transfer_request struct in http-push.c and
      http-walker.c, including filename, real_sha1 and zret, as they are used
      no longer used.
      
      Move the methods append_remote_object_url() and get_remote_object_url()
      from http-push.c to http.c. Additionally, get_remote_object_url() is no
      longer defined only when USE_CURL_MULTI is defined, since
      non-USE_CURL_MULTI code in http.c uses it (namely, in
      new_http_object_request()).
      
      Refactor code from http-push.c::start_fetch_loose() and
      http-walker.c::start_object_fetch_request() that deals with the details
      of coming up with the filename to store the retrieved object, resuming
      a previously aborted request, and making a new curl request, into a new
      function, new_http_object_request().
      
      Refactor code from http-walker.c::process_object_request() into the
      function, process_http_object_request().
      
      Refactor code from http-push.c::finish_request() and
      http-walker.c::finish_object_request() into a new function,
      finish_http_object_request(). It returns the result of the
      move_temp_to_file() invocation.
      
      Add a function, release_http_object_request(), which cleans up object
      request data. http-push.c and http-walker.c invoke this function
      separately; http-push.c::release_request() and
      http-walker.c::release_object_request() do not invoke this function.
      
      Add a function, abort_http_object_request(), which unlink()s the object
      file and invokes release_http_object_request(). Update
      http-walker.c::abort_object_request() to use this.
      Signed-off-by: Ray's avatarTay Ray Chuan <rctay89@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      5424bc55
    • Ray's avatar
      http*: add helper methods for fetching packs · 2264dfa5
      Ray authored
      The code handling the fetching of packs in http-push.c and
      http-walker.c have been refactored into new methods and a new struct
      (http_pack_request) in http.c. They are not meant to be invoked
      elsewhere.
      
      The new methods in http.c are
       - new_http_pack_request
       - finish_http_pack_request
       - release_http_pack_request
      
      and the new struct is http_pack_request.
      
      Add a function, new_http_pack_request(), that deals with the details of
      coming up with the filename to store the retrieved packfile, resuming a
      previously aborted request, and making a new curl request. Update
      http-push.c::start_fetch_packed() and http-walker.c::fetch_pack() to
      use this.
      
      Add a function, finish_http_pack_request(), that deals with renaming
      the pack, advancing the pack list, and installing the pack. Update
      http-push.c::finish_request() and http-walker.c::fetch_pack to use
      this.
      
      Update release_request() in http-push.c and http-walker.c to invoke
      release_http_pack_request() to clean up pack request helper data.
      
      The local_stream member of the transfer_request struct in http-push.c
      has been removed, as the packfile pointer will be managed in the struct
      http_pack_request.
      Signed-off-by: Ray's avatarTay Ray Chuan <rctay89@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      2264dfa5
    • Ray's avatar
      http*: add http_get_info_packs · b8caac2b
      Ray authored
      http-push.c and http-walker.c no longer have to use fetch_index or
      setup_index; they simply need to use http_get_info_packs, a new http
      method, in their fetch_indices implementations.
      
      Move fetch_index() and rename to fetch_pack_index() in http.c; this
      method is not meant to be used outside of http.c. It invokes
      end_url_with_slash with base_url; apart from that change, the code is
      identical.
      
      Move setup_index() and rename to fetch_and_setup_pack_index() in
      http.c; this method is not meant to be used outside of http.c.
      
      Do not immediately set ret to 0 in http-walker.c::fetch_indices();
      instead do it in the HTTP_MISSING_TARGET case, to make it clear that
      the HTTP_OK and HTTP_MISSING_TARGET cases both return 0.
      Signed-off-by: Ray's avatarTay Ray Chuan <rctay89@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      b8caac2b
    • Ray's avatar
      http*: move common variables and macros to http.[ch] · e9176745
      Ray authored
      Move RANGE_HEADER_SIZE to http.h.
      
      Create no_pragma_header, the curl header list containing the header
      "Pragma:" in http.[ch]. It is allocated in http_init, and freed in
      http_cleanup. This replaces the no_pragma_header in http-push.c, and
      the no_pragma_header member in walker_data in http-walker.c.
      
      Create http_is_verbose. It is to be used by methods in http.c, and is
      modified at the entry points of http.c's users, namely http-push.c
      (when parsing options) and http-walker.c (in get_http_walker).
      Signed-off-by: Ray's avatarTay Ray Chuan <rctay89@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      e9176745
    • Ray's avatar
      http*: copy string returned by sha1_to_hex · 20cfb3aa
      Ray authored
      In the fetch_index implementations in http-push.c and http-walker.c,
      the string returned by sha1_to_hex is assumed to stay immutable.
      
      This patch ensures that hex stays immutable by copying the string
      returned by sha1_to_hex (via xstrdup) and frees it subsequently. It
      also refactors free()'s and fclose()'s with labels.
      Signed-off-by: Ray's avatarTay Ray Chuan <rctay89@gmail.com>
      Signed-off-by: 's avatarJunio C Hamano <gitster@pobox.com>
      20cfb3aa